Move save operation handling to its own class

src/main/java/com/gmail/nossr50/commands/experience/AddlevelsCommand.java

@@ -1,6 +1,5 @@
 package com.gmail.nossr50.commands.experience;
 
-import com.gmail.nossr50.datatypes.experience.XPGainReason;
 import com.gmail.nossr50.datatypes.player.McMMOPlayer;
 import com.gmail.nossr50.datatypes.player.PlayerProfile;
 import com.gmail.nossr50.datatypes.skills.PrimarySkillType;
@@ -28,7 +27,7 @@ public class AddlevelsCommand extends ExperienceCommand {
         profile.addLevels(skill, value);
 
         if (player == null) {
-            profile.scheduleAsyncSave();
+            UserManager.getPlayerSaveHandler().scheduleAsyncSave(profile.getPlayerData());
             return;
         }
 

src/main/java/com/gmail/nossr50/commands/experience/AddxpCommand.java

@@ -32,7 +32,7 @@ public class AddxpCommand extends ExperienceCommand {
         }
         else {
             profile.addXp(skill, value);
-            profile.scheduleAsyncSave();
+            UserManager.getPlayerSaveHandler().scheduleAsyncSave(profile.getPlayerData());
         }
     }
 

src/main/java/com/gmail/nossr50/commands/experience/MmoeditCommand.java

@@ -30,7 +30,7 @@ public class MmoeditCommand extends ExperienceCommand {
         profile.modifySkill(skill, value);
 
         if (player == null) {
-            profile.scheduleAsyncSave();
+            UserManager.getPlayerSaveHandler().scheduleAsyncSave(profile.getPlayerData());
             return;
         }
 

src/main/java/com/gmail/nossr50/commands/experience/SkillresetCommand.java

@@ -129,7 +129,7 @@ public class SkillresetCommand implements TabExecutor {
         profile.modifySkill(skill, 0);
 
         if (player == null) {
-            profile.scheduleAsyncSave();
+            UserManager.getPlayerSaveHandler().scheduleAsyncSave(profile.getPlayerData());
             return;
         }
 

src/main/java/com/gmail/nossr50/database/DatabaseManager.java

@@ -4,6 +4,7 @@ import com.gmail.nossr50.api.exceptions.InvalidSkillException;
 import com.gmail.nossr50.config.Config;
 import com.gmail.nossr50.datatypes.database.DatabaseType;
 import com.gmail.nossr50.datatypes.database.PlayerStat;
+import com.gmail.nossr50.datatypes.player.MMODataSnapshot;
 import com.gmail.nossr50.datatypes.player.PlayerData;
 import com.gmail.nossr50.datatypes.player.PlayerProfile;
 import com.gmail.nossr50.datatypes.skills.PrimarySkillType;
@@ -55,6 +56,13 @@ public interface DatabaseManager {
      */
     boolean saveUser(@NotNull PlayerData playerData);
 
+    /**
+     *
+     * @param dataSnapshot target data snapshot
+     * @return true if successful, false on failure
+     */
+    boolean saveUser(@NotNull MMODataSnapshot dataSnapshot);
+
     /**
     * Retrieve leaderboard info.
      * Will never be null but it may be empty

src/main/java/com/gmail/nossr50/database/FlatFileDatabaseManager.java

@@ -258,9 +258,12 @@ public final class FlatFileDatabaseManager implements DatabaseManager {
         //Not used in FlatFile
     }
 
-
     public boolean saveUser(@NotNull PlayerData playerData) {
-        MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData); //Clone data into Immutable data
+        MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData);
+        return saveUser(mmoDataSnapshot); //Clone data into Immutable data
+    }
+
+    public boolean saveUser(@NotNull MMODataSnapshot mmoDataSnapshot) {
         String playerName = mmoDataSnapshot.getPlayerName();
         UUID uuid = mmoDataSnapshot.getPlayerUUID();
 
@@ -305,8 +308,8 @@ public final class FlatFileDatabaseManager implements DatabaseManager {
                     }
 
                     if (!(uuid != null
-                                    && splitData[FlatFileMappings.UUID_INDEX].equalsIgnoreCase(uuid.toString()))
+                            && splitData[FlatFileMappings.UUID_INDEX].equalsIgnoreCase(uuid.toString()))
-                                    && !splitData[FlatFileMappings.USERNAME].equalsIgnoreCase(playerName)) {
+                            && !splitData[FlatFileMappings.USERNAME].equalsIgnoreCase(playerName)) {
                         writer.append(line).append("\r\n"); //Not the user so write it to file and move on
                     } else {
                         //User found

src/main/java/com/gmail/nossr50/datatypes/player/MMODataSnapshot.java

@@ -10,6 +10,9 @@ import org.jetbrains.annotations.NotNull;
 import java.util.UUID;
 
 public class MMODataSnapshot {
+    /* Save Attempts */
+    private int saveAttempts = 0;
+
     /* Player Stuff */
     private final @NotNull String playerName;
     private final @NotNull UUID playerUUID;
@@ -109,4 +112,17 @@ public class MMODataSnapshot {
     public boolean isLeaderBoardExcluded() {
         return leaderBoardExclusion;
     }
+
+    public int getSaveAttempts() {
+        return saveAttempts;
+    }
+
+    public void setSaveAttempts(int saveAttempts) {
+        this.saveAttempts = saveAttempts;
+    }
+
+    public void incrementSaveAttempts() {
+        this.saveAttempts += 1;
+    }
+
 }

src/main/java/com/gmail/nossr50/runnables/SaveTimerTask.java

@@ -1,6 +1,8 @@
 package com.gmail.nossr50.runnables;
 
+import com.gmail.nossr50.datatypes.player.McMMOPlayer;
 import com.gmail.nossr50.mcMMO;
+import com.gmail.nossr50.util.player.UserManager;
 import com.neetgames.mcmmo.player.OnlineMMOPlayer;
 import org.bukkit.scheduler.BukkitRunnable;
 
@@ -12,8 +14,8 @@ public class SaveTimerTask extends BukkitRunnable {
         int count = 1;
 
         //TODO: write a more efficient bulk save
-        for (OnlineMMOPlayer mmoPlayer : UserManager.getPlayers()) {
+        for (McMMOPlayer mmoPlayer : UserManager.getPlayers()) {
-            UserManager.saveUserWithDelay(mmoPlayer.getPersistentPlayerData(), false, count);
+            UserManager.getPlayerSaveHandler().scheduleAsyncSaveDelay(mmoPlayer.getPlayerData());
             count++;
         }
 

src/main/java/com/gmail/nossr50/runnables/database/FormulaConversionTask.java

@@ -41,9 +41,8 @@ public class FormulaConversionTask extends BukkitRunnable {
 
                 editValues(profile);
                 // Since this is a temporary profile, we save it here.
-                profile.scheduleAsyncSave();
+                UserManager.getPlayerSaveHandler().scheduleAsyncSave(profile.getPlayerData());
-            }
+            } else {
-            else {
                 profile = mcMMOPlayer.getProfile();
                 editValues(profile);
             }

src/main/java/com/gmail/nossr50/util/player/PlayerSaveHandler.java

@@ -0,0 +1,81 @@
+package com.gmail.nossr50.util.player;
+
+import com.gmail.nossr50.datatypes.player.MMODataSnapshot;
+import com.gmail.nossr50.datatypes.player.PlayerData;
+import com.gmail.nossr50.mcMMO;
+import org.bukkit.Bukkit;
+import org.jetbrains.annotations.NotNull;
+
+//TODO: Low priority - Track pending Async saves to avoid data loss during server shutdown
+//TODO: T&C Javadocs
+public class PlayerSaveHandler {
+
+    private void save(@NotNull MMODataSnapshot mmoDataSnapshot, boolean useSync) {
+        boolean saveSuccessful = mcMMO.getDatabaseManager().saveUser(mmoDataSnapshot);
+
+        //Check for failure to save
+        if (!saveSuccessful) {
+            String playerName = mmoDataSnapshot.getPlayerName();
+            String uuidStr = mmoDataSnapshot.getPlayerUUID().toString();
+            mcMMO.p.getLogger().severe("PlayerProfile saving failed for player name: " + playerName + " UUID: " + uuidStr);
+
+            if(mmoDataSnapshot.getSaveAttempts() > 0) {
+                mcMMO.p.getLogger().severe("Attempted to save profile for player "+playerName
+                        + " resulted in failure. " + mmoDataSnapshot.getSaveAttempts() + " have been made so far.");
+            }
+
+            if(mmoDataSnapshot.getSaveAttempts() < 10) {
+                mmoDataSnapshot.incrementSaveAttempts();
+
+                //Back out of async saving if we detect a server shutdown, this is not always going to be caught
+                if(mcMMO.isServerShutdownExecuted() || useSync)
+                    scheduleSyncSave(mmoDataSnapshot); //Execute sync saves immediately
+                else
+                    scheduleAsyncSave(mmoDataSnapshot);
+
+            } else {
+                mcMMO.p.getLogger().severe("mcMMO has failed to save the profile for "
+                        + playerName + " numerous times." +
+                        " mcMMO will now stop attempting to save this profile." +
+                        " Check your console for errors and inspect your DB for issues.");
+            }
+        }
+    }
+
+    public void save(@NotNull PlayerData playerData, boolean useSync) {
+        //TODO: We no longer check if a profile is loaded or not as it should never be unloaded if a save operation is being called, need to double check this to be true
+        if(!playerData.isDirtyProfile()) {
+            return; //Don't save data that hasn't changed
+        }
+
+        MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData);
+        save(mmoDataSnapshot, useSync);
+    }
+
+    public void scheduleAsyncSave(@NotNull PlayerData playerData) {
+        MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData);
+        scheduleAsyncSave(mmoDataSnapshot);
+    }
+
+    public void scheduleAsyncSaveDelay(@NotNull PlayerData playerData) {
+        MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData);
+        scheduleAsyncSaveDelay(mmoDataSnapshot);
+    }
+
+    public void scheduleSyncSave(@NotNull PlayerData playerData) {
+        MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData);
+        scheduleSyncSave(mmoDataSnapshot);
+    }
+
+    private void scheduleAsyncSave(@NotNull MMODataSnapshot mmoDataSnapshot) {
+        Bukkit.getScheduler().runTaskAsynchronously(mcMMO.p, () ->  save(mmoDataSnapshot, false));
+    }
+
+    private void scheduleAsyncSaveDelay(@NotNull MMODataSnapshot mmoDataSnapshot) {
+        Bukkit.getScheduler().runTaskLaterAsynchronously(mcMMO.p, () -> save(mmoDataSnapshot, false), 20L);
+    }
+
+    private void scheduleSyncSave(@NotNull MMODataSnapshot mmoDataSnapshot) {
+        Bukkit.getScheduler().runTask(mcMMO.p, () -> save(mmoDataSnapshot, true));
+    }
+}

src/main/java/com/gmail/nossr50/util/player/UserManager.java

@@ -19,6 +19,7 @@ import java.util.HashSet;
 public final class UserManager {
 
     private static HashSet<McMMOPlayer> playerDataSet; //Used to track players for sync saves on shutdown
+    private @NotNull static final PlayerSaveHandler playerSaveHandler = new PlayerSaveHandler();
 
     private UserManager() {}
 
@@ -69,7 +70,7 @@ public final class UserManager {
     }
 
     /**
-     * Save all users ON THIS THREAD.
+     * Save all users on main thread
      */
     public static void saveAll() {
         if(playerDataSet == null)
@@ -79,15 +80,12 @@ public final class UserManager {
 
         mcMMO.p.getLogger().info("Saving mcMMOPlayers... (" + trackedSyncData.size() + ")");
 
-        for (McMMOPlayer playerData : trackedSyncData) {
+        for (McMMOPlayer mmoPlayer : trackedSyncData) {
-            try
+            try {
-            {
+                mcMMO.p.getLogger().info("Saving data for player: "+mmoPlayer.getPlayerName());
-                mcMMO.p.getLogger().info("Saving data for player: "+playerData.getPlayerName());
+                getPlayerSaveHandler().save(mmoPlayer.getPlayerData(), true);
-                playerData.getProfile().save(true);
+            } catch (Exception e) {
-            }
+                mcMMO.p.getLogger().severe("Could not save mcMMO player data for player: " + mmoPlayer.getPlayerName());
-            catch (Exception e)
-            {
-                mcMMO.p.getLogger().warning("Could not save mcMMO player data for player: " + playerData.getPlayerName());
             }
         }
 
@@ -120,9 +118,9 @@ public final class UserManager {
         mmoPlayer.getTamingManager().cleanupAllSummons();
 
         if (syncSave) {
-            getProfile().save(true); //TODO: T&C Wire this up, see master branch com.gmail.nossr50.datatypes.player.PlayerProfile#save
+            getPlayerSaveHandler().save(mmoPlayer.getPlayerData(), true); //TODO: T&C Wire this up, see master branch com.gmail.nossr50.datatypes.player.PlayerProfile#save
         } else {
-            getProfile().scheduleAsyncSave(); //TODO: T&C Wire this up, see master branch com.gmail.nossr50.datatypes.player.PlayerProfile#scheduleAsyncSave
+            getPlayerSaveHandler().scheduleAsyncSave(mmoPlayer.getPlayerData()); //TODO: T&C Wire this up, see master branch com.gmail.nossr50.datatypes.player.PlayerProfile#scheduleAsyncSave
         }
 
         UserManager.remove(targetPlayer);
@@ -190,4 +188,9 @@ public final class UserManager {
     public static boolean hasPlayerDataKey(Entity entity) {
         return entity != null && entity.hasMetadata(mcMMO.playerDataKey);
     }
+
+    public static @NotNull PlayerSaveHandler getPlayerSaveHandler() {
+        return playerSaveHandler;
+    }
+
 }