From 3ce3ac5350f30e8ab949ead22a992b29948abb17 Mon Sep 17 00:00:00 2001 From: nossr50 Date: Wed, 5 Jun 2019 21:11:44 -0700 Subject: [PATCH 1/4] mcMMO should no longer lose a few minutes of player data from shutting down the server --- Changelog.txt | 3 + pom.xml | 2 +- .../com/gmail/nossr50/api/ExperienceAPI.java | 2 +- .../nossr50/datatypes/player/McMMOPlayer.java | 9 ++- .../datatypes/player/PlayerProfile.java | 55 +++++++++++++------ src/main/java/com/gmail/nossr50/mcMMO.java | 2 +- .../nossr50/runnables/SaveTimerTask.java | 2 +- .../player/PlayerProfileSaveTask.java | 6 +- .../nossr50/util/player/UserManager.java | 30 ++++++++-- 9 files changed, 80 insertions(+), 31 deletions(-) diff --git a/Changelog.txt b/Changelog.txt index 645674540..1f18f1fa6 100644 --- a/Changelog.txt +++ b/Changelog.txt @@ -1,3 +1,6 @@ +Version 2.1.69 + Fixed a few places where mcMMO would not save player data immediately which may cause players to lose a few minutes of progress + Version 2.1.68 Updated Japanese locale (thanks Snake) Fixed a bug where consuming food in the off hand did not trigger the Diet abilities diff --git a/pom.xml b/pom.xml index 1e704dd90..6e46fabb5 100755 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.gmail.nossr50.mcMMO mcMMO - 2.1.68 + 2.1.69-SNAPSHOT mcMMO https://github.com/mcMMO-Dev/mcMMO diff --git a/src/main/java/com/gmail/nossr50/api/ExperienceAPI.java b/src/main/java/com/gmail/nossr50/api/ExperienceAPI.java index e018a4cc2..6a800fb23 100644 --- a/src/main/java/com/gmail/nossr50/api/ExperienceAPI.java +++ b/src/main/java/com/gmail/nossr50/api/ExperienceAPI.java @@ -1102,7 +1102,7 @@ public final class ExperienceAPI { PlayerProfile profile = getOfflineProfile(playerUniqueId); profile.addXp(skill, XP); - profile.save(); + profile.save(true); } @Deprecated diff --git a/src/main/java/com/gmail/nossr50/datatypes/player/McMMOPlayer.java b/src/main/java/com/gmail/nossr50/datatypes/player/McMMOPlayer.java index 548536fc2..6b1a9bc2f 100644 --- a/src/main/java/com/gmail/nossr50/datatypes/player/McMMOPlayer.java +++ b/src/main/java/com/gmail/nossr50/datatypes/player/McMMOPlayer.java @@ -100,9 +100,10 @@ public class McMMOPlayer { private boolean isUsingUnarmed; private final FixedMetadataValue playerMetadata; + private String playerName; public McMMOPlayer(Player player, PlayerProfile profile) { - String playerName = player.getName(); + this.playerName = player.getName(); UUID uuid = player.getUniqueId(); this.player = player; @@ -140,6 +141,10 @@ public class McMMOPlayer { experienceBarManager = new ExperienceBarManager(this); } + public String getPlayerName() { + return playerName; + } + /*public void hideXpBar(PrimarySkillType primarySkillType) { experienceBarManager.hideExperienceBar(primarySkillType); @@ -995,7 +1000,7 @@ public class McMMOPlayer { BleedTimerTask.bleedOut(thisPlayer); if (syncSave) { - getProfile().save(); + getProfile().save(true); } else { getProfile().scheduleAsyncSave(); } diff --git a/src/main/java/com/gmail/nossr50/datatypes/player/PlayerProfile.java b/src/main/java/com/gmail/nossr50/datatypes/player/PlayerProfile.java index b2b0f7803..ba3567363 100644 --- a/src/main/java/com/gmail/nossr50/datatypes/player/PlayerProfile.java +++ b/src/main/java/com/gmail/nossr50/datatypes/player/PlayerProfile.java @@ -92,14 +92,23 @@ public class PlayerProfile { } public void scheduleAsyncSave() { - new PlayerProfileSaveTask(this).runTaskAsynchronously(mcMMO.p); + new PlayerProfileSaveTask(this, false).runTaskAsynchronously(mcMMO.p); + } + + public void scheduleSyncSave() { + new PlayerProfileSaveTask(this, true).runTask(mcMMO.p); } public void scheduleAsyncSaveDelay() { - new PlayerProfileSaveTask(this).runTaskLaterAsynchronously(mcMMO.p, 20); + new PlayerProfileSaveTask(this, false).runTaskLaterAsynchronously(mcMMO.p, 20); } - public void save() { + @Deprecated + public void scheduleSyncSaveDelay() { + new PlayerProfileSaveTask(this, true).runTaskLater(mcMMO.p, 20); + } + + public void save(boolean useSync) { if (!changed || !loaded) { saveAttempts = 0; return; @@ -121,7 +130,12 @@ public class PlayerProfile { if(saveAttempts < 10) { saveAttempts++; - scheduleAsyncSaveDelay(); + + if(useSync) + scheduleSyncSave(); //Execute sync saves immediately + else + scheduleAsyncSaveDelay(); + return; } else { mcMMO.p.getLogger().severe("mcMMO has failed to save the profile for " @@ -144,7 +158,7 @@ public class PlayerProfile { } public void setUniqueId(UUID uuid) { - changed = true; + markProfileDirty(); this.uuid = uuid; } @@ -162,17 +176,24 @@ public class PlayerProfile { } public void setMobHealthbarType(MobHealthbarType mobHealthbarType) { - changed = true; + markProfileDirty(); this.mobHealthbarType = mobHealthbarType; } + /** + * Marks the profile as "dirty" which flags a profile to be saved in the next save operation + */ + public void markProfileDirty() { + changed = true; + } + public int getScoreboardTipsShown() { return scoreboardTipsShown; } public void setScoreboardTipsShown(int scoreboardTipsShown) { - changed = true; + markProfileDirty(); this.scoreboardTipsShown = scoreboardTipsShown; } @@ -188,12 +209,12 @@ public class PlayerProfile { public int getChimaerWingDATS() { return uniquePlayerData.get(UniqueDataType.CHIMAERA_WING_DATS);} protected void setChimaeraWingDATS(int DATS) { - changed = true; + markProfileDirty(); uniquePlayerData.put(UniqueDataType.CHIMAERA_WING_DATS, DATS); } public void setUniqueData(UniqueDataType uniqueDataType, int newData) { - changed = true; + markProfileDirty(); uniquePlayerData.put(uniqueDataType, newData); } @@ -216,7 +237,7 @@ public class PlayerProfile { * @param DATS the DATS of the ability */ protected void setAbilityDATS(SuperAbilityType ability, long DATS) { - changed = true; + markProfileDirty(); abilityDATS.put(ability, (int) (DATS * .001D)); } @@ -225,7 +246,7 @@ public class PlayerProfile { * Reset all ability cooldowns. */ protected void resetCooldowns() { - changed = true; + markProfileDirty(); for (SuperAbilityType ability : abilityDATS.keySet()) { abilityDATS.put(ability, 0); @@ -253,7 +274,7 @@ public class PlayerProfile { return; } - changed = true; + markProfileDirty(); skillsXp.put(skill, xpLevel); } @@ -261,7 +282,7 @@ public class PlayerProfile { protected float levelUp(PrimarySkillType skill) { float xpRemoved = getXpToLevel(skill); - changed = true; + markProfileDirty(); skills.put(skill, skills.get(skill) + 1); skillsXp.put(skill, skillsXp.get(skill) - xpRemoved); @@ -280,7 +301,7 @@ public class PlayerProfile { return; } - changed = true; + markProfileDirty(); skillsXp.put(skill, skillsXp.get(skill) - xp); } @@ -290,7 +311,7 @@ public class PlayerProfile { return; } - changed = true; + markProfileDirty(); skillsXp.put(skill, skillsXp.get(skill) - xp); } @@ -306,7 +327,7 @@ public class PlayerProfile { return; } - changed = true; + markProfileDirty(); //Don't allow levels to be negative if(level < 0) @@ -333,7 +354,7 @@ public class PlayerProfile { * @param xp Number of experience to add */ public void addXp(PrimarySkillType skill, float xp) { - changed = true; + markProfileDirty(); if (skill.isChildSkill()) { Set parentSkills = FamilyTree.getParents(skill); diff --git a/src/main/java/com/gmail/nossr50/mcMMO.java b/src/main/java/com/gmail/nossr50/mcMMO.java index 530db4281..6aa845ae7 100644 --- a/src/main/java/com/gmail/nossr50/mcMMO.java +++ b/src/main/java/com/gmail/nossr50/mcMMO.java @@ -318,9 +318,9 @@ public class mcMMO extends JavaPlugin { @Override public void onDisable() { try { - Alchemy.finishAllBrews(); // Finish all partially complete AlchemyBrewTasks to prevent vanilla brewing continuation on restart UserManager.saveAll(); // Make sure to save player information if the server shuts down UserManager.clearAll(); + Alchemy.finishAllBrews(); // Finish all partially complete AlchemyBrewTasks to prevent vanilla brewing continuation on restart PartyManager.saveParties(); // Save our parties //TODO: Needed? diff --git a/src/main/java/com/gmail/nossr50/runnables/SaveTimerTask.java b/src/main/java/com/gmail/nossr50/runnables/SaveTimerTask.java index 240c52178..2e679b824 100644 --- a/src/main/java/com/gmail/nossr50/runnables/SaveTimerTask.java +++ b/src/main/java/com/gmail/nossr50/runnables/SaveTimerTask.java @@ -14,7 +14,7 @@ public class SaveTimerTask extends BukkitRunnable { int count = 1; for (McMMOPlayer mcMMOPlayer : UserManager.getPlayers()) { - new PlayerProfileSaveTask(mcMMOPlayer.getProfile()).runTaskLaterAsynchronously(mcMMO.p, count); + new PlayerProfileSaveTask(mcMMOPlayer.getProfile(), false).runTaskLaterAsynchronously(mcMMO.p, count); count++; } diff --git a/src/main/java/com/gmail/nossr50/runnables/player/PlayerProfileSaveTask.java b/src/main/java/com/gmail/nossr50/runnables/player/PlayerProfileSaveTask.java index 414896983..593d785ca 100644 --- a/src/main/java/com/gmail/nossr50/runnables/player/PlayerProfileSaveTask.java +++ b/src/main/java/com/gmail/nossr50/runnables/player/PlayerProfileSaveTask.java @@ -5,13 +5,15 @@ import org.bukkit.scheduler.BukkitRunnable; public class PlayerProfileSaveTask extends BukkitRunnable { private PlayerProfile playerProfile; + private boolean isSync; - public PlayerProfileSaveTask(PlayerProfile playerProfile) { + public PlayerProfileSaveTask(PlayerProfile playerProfile, boolean isSync) { this.playerProfile = playerProfile; + this.isSync = isSync; } @Override public void run() { - playerProfile.save(); + playerProfile.save(isSync); } } diff --git a/src/main/java/com/gmail/nossr50/util/player/UserManager.java b/src/main/java/com/gmail/nossr50/util/player/UserManager.java index 39044998e..b66a9d59c 100644 --- a/src/main/java/com/gmail/nossr50/util/player/UserManager.java +++ b/src/main/java/com/gmail/nossr50/util/player/UserManager.java @@ -10,10 +10,15 @@ import org.bukkit.metadata.FixedMetadataValue; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; public final class UserManager { - private UserManager() {} + private static HashSet playerDataSet; //Used to track players for sync saves on shutdown + + private UserManager() { + playerDataSet = new HashSet<>(); + } /** * Track a new user. @@ -22,6 +27,11 @@ public final class UserManager { */ public static void track(McMMOPlayer mcMMOPlayer) { mcMMOPlayer.getPlayer().setMetadata(mcMMO.playerDataKey, new FixedMetadataValue(mcMMO.p, mcMMOPlayer)); + playerDataSet.add(mcMMOPlayer); //for sync saves on shutdown + } + + public static void cleanupPlayer(McMMOPlayer mcMMOPlayer) { + playerDataSet.remove(mcMMOPlayer); } /** @@ -30,7 +40,9 @@ public final class UserManager { * @param player The Player object */ public static void remove(Player player) { + McMMOPlayer mcMMOPlayer = getPlayer(player); player.removeMetadata(mcMMO.playerDataKey, mcMMO.p); + playerDataSet.remove(mcMMOPlayer); //Clear sync save tracking } /** @@ -40,25 +52,31 @@ public final class UserManager { for (Player player : mcMMO.p.getServer().getOnlinePlayers()) { remove(player); } + + playerDataSet.clear(); //Clear sync save tracking } /** * Save all users ON THIS THREAD. */ public static void saveAll() { - ImmutableList onlinePlayers = ImmutableList.copyOf(mcMMO.p.getServer().getOnlinePlayers()); - mcMMO.p.debug("Saving mcMMOPlayers... (" + onlinePlayers.size() + ")"); + ImmutableList trackedSyncData = ImmutableList.copyOf(playerDataSet); - for (Player player : onlinePlayers) { + mcMMO.p.getLogger().info("Saving mcMMOPlayers... (" + trackedSyncData.size() + ")"); + + for (McMMOPlayer playerData : trackedSyncData) { try { - getPlayer(player).getProfile().save(); + mcMMO.p.getLogger().info("Saving data for player: "+playerData.getPlayerName()); + playerData.getProfile().save(true); } catch (Exception e) { - mcMMO.p.getLogger().warning("Could not save mcMMO player data for player: " + player.getName()); + mcMMO.p.getLogger().warning("Could not save mcMMO player data for player: " + playerData.getPlayerName()); } } + + mcMMO.p.getLogger().info("Finished save operation for "+trackedSyncData.size()+" players!"); } public static Collection getPlayers() { From 426b2d27e76eaf7b276f206be5270af7165caeed Mon Sep 17 00:00:00 2001 From: nossr50 Date: Wed, 5 Jun 2019 21:43:07 -0700 Subject: [PATCH 2/4] Fix NPE for new data tracker --- .../gmail/nossr50/util/player/UserManager.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/gmail/nossr50/util/player/UserManager.java b/src/main/java/com/gmail/nossr50/util/player/UserManager.java index b66a9d59c..d20f39356 100644 --- a/src/main/java/com/gmail/nossr50/util/player/UserManager.java +++ b/src/main/java/com/gmail/nossr50/util/player/UserManager.java @@ -16,9 +16,7 @@ public final class UserManager { private static HashSet playerDataSet; //Used to track players for sync saves on shutdown - private UserManager() { - playerDataSet = new HashSet<>(); - } + private UserManager() {} /** * Track a new user. @@ -27,6 +25,10 @@ public final class UserManager { */ public static void track(McMMOPlayer mcMMOPlayer) { mcMMOPlayer.getPlayer().setMetadata(mcMMO.playerDataKey, new FixedMetadataValue(mcMMO.p, mcMMOPlayer)); + + if(playerDataSet == null) + playerDataSet = new HashSet<>(); + playerDataSet.add(mcMMOPlayer); //for sync saves on shutdown } @@ -42,7 +44,9 @@ public final class UserManager { public static void remove(Player player) { McMMOPlayer mcMMOPlayer = getPlayer(player); player.removeMetadata(mcMMO.playerDataKey, mcMMO.p); - playerDataSet.remove(mcMMOPlayer); //Clear sync save tracking + + if(playerDataSet != null && playerDataSet.contains(mcMMOPlayer)) + playerDataSet.remove(mcMMOPlayer); //Clear sync save tracking } /** @@ -53,7 +57,8 @@ public final class UserManager { remove(player); } - playerDataSet.clear(); //Clear sync save tracking + if(playerDataSet != null) + playerDataSet.clear(); //Clear sync save tracking } /** From 0638f4c437e1a56a0261907c369aee65747708da Mon Sep 17 00:00:00 2001 From: nossr50 Date: Wed, 5 Jun 2019 23:50:39 -0700 Subject: [PATCH 3/4] 2.1.69 --- pom.xml | 2 +- .../com/gmail/nossr50/datatypes/player/PlayerProfile.java | 4 ++-- src/main/java/com/gmail/nossr50/listeners/PlayerListener.java | 4 +++- src/main/java/com/gmail/nossr50/mcMMO.java | 2 +- src/main/java/com/gmail/nossr50/util/player/UserManager.java | 3 ++- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 6e46fabb5..313aa80ed 100755 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.gmail.nossr50.mcMMO mcMMO - 2.1.69-SNAPSHOT + 2.1.69 mcMMO https://github.com/mcMMO-Dev/mcMMO diff --git a/src/main/java/com/gmail/nossr50/datatypes/player/PlayerProfile.java b/src/main/java/com/gmail/nossr50/datatypes/player/PlayerProfile.java index ba3567363..a9582c095 100644 --- a/src/main/java/com/gmail/nossr50/datatypes/player/PlayerProfile.java +++ b/src/main/java/com/gmail/nossr50/datatypes/player/PlayerProfile.java @@ -144,9 +144,9 @@ public class PlayerProfile { " Check your console for errors and inspect your DB for issues."); } + } else { + saveAttempts = 0; } - - saveAttempts = 0; } public String getPlayerName() { diff --git a/src/main/java/com/gmail/nossr50/listeners/PlayerListener.java b/src/main/java/com/gmail/nossr50/listeners/PlayerListener.java index ff9c6cd46..6b29f41e8 100644 --- a/src/main/java/com/gmail/nossr50/listeners/PlayerListener.java +++ b/src/main/java/com/gmail/nossr50/listeners/PlayerListener.java @@ -502,7 +502,9 @@ public class PlayerListener implements Listener { } McMMOPlayer mcMMOPlayer = UserManager.getPlayer(player); - mcMMOPlayer.logout(false); + //There's an issue with using Async saves on player quit + //Basically there are conditions in which an async task does not execute fast enough to save the data if the server shutdown shortly after this task was scheduled + mcMMOPlayer.logout(true); } /** diff --git a/src/main/java/com/gmail/nossr50/mcMMO.java b/src/main/java/com/gmail/nossr50/mcMMO.java index 6aa845ae7..3e20e8f15 100644 --- a/src/main/java/com/gmail/nossr50/mcMMO.java +++ b/src/main/java/com/gmail/nossr50/mcMMO.java @@ -332,7 +332,7 @@ public class mcMMO extends JavaPlugin { placeStore.saveAll(); // Save our metadata placeStore.cleanUp(); // Cleanup empty metadata stores } - catch (NullPointerException e) { e.printStackTrace(); } + catch (Exception e) { e.printStackTrace(); } debug("Canceling all tasks..."); getServer().getScheduler().cancelTasks(this); // This removes our tasks diff --git a/src/main/java/com/gmail/nossr50/util/player/UserManager.java b/src/main/java/com/gmail/nossr50/util/player/UserManager.java index d20f39356..c635300c5 100644 --- a/src/main/java/com/gmail/nossr50/util/player/UserManager.java +++ b/src/main/java/com/gmail/nossr50/util/player/UserManager.java @@ -33,7 +33,8 @@ public final class UserManager { } public static void cleanupPlayer(McMMOPlayer mcMMOPlayer) { - playerDataSet.remove(mcMMOPlayer); + if(playerDataSet != null && playerDataSet.contains(mcMMOPlayer)) + playerDataSet.remove(mcMMOPlayer); } /** From 58cde322f508aed25628acf9ccba4d0f6a8ca98c Mon Sep 17 00:00:00 2001 From: nossr50 Date: Thu, 6 Jun 2019 01:53:59 -0700 Subject: [PATCH 4/4] Add shoutout to changelog --- Changelog.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changelog.txt b/Changelog.txt index 1f18f1fa6..7d8858c51 100644 --- a/Changelog.txt +++ b/Changelog.txt @@ -1,6 +1,8 @@ Version 2.1.69 Fixed a few places where mcMMO would not save player data immediately which may cause players to lose a few minutes of progress + A big thanks to Sleepyflea for helping test this patch and report this bug. + Version 2.1.68 Updated Japanese locale (thanks Snake) Fixed a bug where consuming food in the off hand did not trigger the Diet abilities