Conversation
…taken and what is just a condition at all
…ues before the other server node got saved async
# Conflicts: # pom.xml # src/main/java/studio/magemonkey/fusion/data/player/FusionPlayer.java # src/main/java/studio/magemonkey/fusion/data/queue/CraftingQueue.java # src/main/java/studio/magemonkey/fusion/gui/RecipeGui.java # src/main/java/studio/magemonkey/fusion/gui/recipe/RecipeGuiEventRouter.java
Travja
left a comment
There was a problem hiding this comment.
Thanks for the extensive bugfixes and performance improvements! This is a great step forward for Fusion's stability.
I have a few concerns regarding cross-database compatibility and the new locking mechanism:
1. SQLite Compatibility
The use of BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY in FusionProfessionsSQL.java and FusionRecipeLimitsSQL.java will cause issues for users on the LOCAL (SQLite) storage type. SQLite uses AUTOINCREMENT (and only with INTEGER PRIMARY KEY). We should ensure these statements are compatible with both SQLite and MySQL/MariaDB, or handle the dialect difference in SQLManager.
2. Potential NullPointerExceptions from Locking
The change in PlayerLoader.getPlayer(uuid) to return null when a player is locked during an async save is risky. Many call sites (like Placeholders, commands, and GUI logic) do not expect a null return from this method. This will likely lead to NPEs during the save window.
Recommendation: Instead of returning null, we should consider handling the "locked" state inside the FusionPlayer object or ensuring all call sites can handle a null response gracefully.
3. Async Cache Clearing
In FusionPlayer.save(), cachedQueues.clear() and cachedRecipeLimits.clear() are called inside the asynchronous task. If a player interacts with the plugin and triggers a cache repopulation while the save is still in progress, that new data might be lost when the clear command eventually runs.
Recommendation: Clear the caches before passing the data to the async task, or pass a snapshot of the data to the thread.
Otherwise, the GUI optimization and the fix for InventoryClickEvent look excellent!
|
Those are good points to consider, I will adress them soon and get back to you. Thanks! |
|
I wrote fixing prompts for you requests and checked it on functionility. I think this looks fine from my side |
- Clear all player locks on startup in SQLManager to prevent permanent locks after a crash. - Modify FusionPlayer.save() to only clear caches when explicitly requested (e.g., during unload), preventing data loss during periodic saves. - Fix race conditions in Fusion.onPlayerJoin by waiting for pending save locks asynchronously before loading player data. - Optimize PlayerLoader.getPlayer by removing frequent database queries for lock status, relying on initial load instead. - Fix redundant save and reload logic in PlayerManager.savePlayer to ensure consistency.
This branch contains some bugfixes that were done over time on a production server