Fix EcoProfile read method to be asynchronous#390
Fix EcoProfile read method to be asynchronous#390MillionthOdin16 wants to merge 1 commit intoAuxilor:masterfrom
Conversation
Fixes Auxilor#389 Update the `read` method in `PersistentDataHandler` to use `CompletableFuture` for asynchronous operations. * Change the `read` method in `PersistentDataHandler.java` to return `CompletableFuture<T>` instead of `T`. * Remove the blocking call `future.get()` from the `read` method. * Update the `read` method in `EcoProfile.kt` to handle the new `CompletableFuture` return type. * Use `thenApply` to process the result of the `CompletableFuture` in the `read` method in `EcoProfile.kt`. * Ensure the `read` method in `EcoProfile.kt` returns the default value if the `CompletableFuture` is null.
| } ?: key.defaultValue | ||
| } | ||
|
|
||
| this.data[key] = future.thenApply { it ?: key.defaultValue }.join() |
There was a problem hiding this comment.
But here you are joining again. Is this intentional? Because when another plugin calls the read method on an EcoProfile (such as EcoSkills), it is still blocking, isn’t it?
|
This would break API compatibility surely |
|
Okay, but what are the alternatives? Just do nothing and accept that every server will lag when it has database storage and many players joining? |
|
Ignoring the problem or refusing to fix it isn’t really a solution. The server lagging every time a player joins is a serious issue that directly affects performance and usability. I get that changing the API might have some downsides, but just leaving things as they are right now isn’t really an option for server owners who rely on this plugin. |
|
Any updates on fixing? |
|
any updates? |
|
Hey! As someone who uses the eco api, i feel like there should be separate async methods & support for callbacks. There are some instances where you would want something to be blocking the thread (very few albeit). Once that would be done, other plugins can be PR'd and updated accordingly. |
|
@WillFP Any updates on this PR / Issue? |
|
Any update refering to this? |
|
@WillFP Any update on this? It’s only been... what, forever? |
|
This fixes absolutely nothing. The main thread is still blocking, waiting for the completable future to complete |
8 participants
Fixes #389
Update the
readmethod inPersistentDataHandlerto useCompletableFuturefor asynchronous operations.readmethod inPersistentDataHandler.javato returnCompletableFuture<T>instead ofT.future.get()from thereadmethod.readmethod inEcoProfile.ktto handle the newCompletableFuturereturn type.thenApplyto process the result of theCompletableFuturein thereadmethod inEcoProfile.kt.readmethod inEcoProfile.ktreturns the default value if theCompletableFutureis null.