Finally fix autosaves after regression caused by https://github.com/LMBishop/Quests/commit/e5c0237bcb45c5d308e0a2b763f50ac97135c768

2 files changed, 129 insertions, 36 deletions

diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/BukkitQuestsPlugin.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/BukkitQuestsPlugin.java
index 2cbe540a..e01c1d25 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/BukkitQuestsPlugin.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/BukkitQuestsPlugin.java
@@ -155,6 +155,7 @@ import org.bukkit.entity.Player;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.plugin.Plugin;
 import org.bukkit.plugin.java.JavaPlugin;
+import org.bukkit.scheduler.BukkitRunnable;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -170,6 +171,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.function.BiFunction;
+import java.util.logging.Level;
 
 public class BukkitQuestsPlugin extends JavaPlugin implements Quests {
 
@@ -271,7 +273,7 @@ public class BukkitQuestsPlugin extends JavaPlugin implements Quests {
         questsLogger.info("Running server scheduler: " + serverScheduler.getServerSchedulerName());
 
         // Load base configuration for use during rest of startup procedure
-        if (!this.reloadBaseConfiguration()) {
+        if (!this.reloadBaseConfiguration(true)) {
             questsLogger.severe("Plugin cannot start into a stable state as the configuration is broken!");
             super.getServer().getPluginManager().disablePlugin(this);
             return;
@@ -615,7 +617,7 @@ public class BukkitQuestsPlugin extends JavaPlugin implements Quests {
 
     @Override
     public void reloadQuests() {
-        if (this.reloadBaseConfiguration()) {
+        if (this.reloadBaseConfiguration(false)) {
             BukkitQuestsLoader questsLoader = new BukkitQuestsLoader(this);
             questsLoader.loadQuestItems(new File(super.getDataFolder() + File.separator + "items"));
             configProblems = questsLoader.loadQuests(new File(super.getDataFolder() + File.separator + "quests"));
@@ -645,7 +647,7 @@ public class BukkitQuestsPlugin extends JavaPlugin implements Quests {
         return itemGetter.getItem(path, config, excludes);
     }
 
-    private boolean reloadBaseConfiguration() {
+    private boolean reloadBaseConfiguration(final boolean initialLoad) {
         this.validConfiguration = this.questsConfig.loadConfig();
 
         if (this.validConfiguration) {
@@ -664,24 +666,29 @@ public class BukkitQuestsPlugin extends JavaPlugin implements Quests {
                     // TODO the other one
             }
 
-            final long autoSaveInterval = this.getConfig().getLong("options.performance-tweaking.quest-autosave-interval", 12000);
-            try {
-                if (this.questAutoSaveTask != null) {
-                    this.questAutoSaveTask.cancel();
+            // Don't do that on first load as the plugin will later call reloadQuests()
+            // in the onEnable() lambda which calls this method and makes it try to cancel
+            // task that has not been scheduled yet
+            if (!initialLoad) {
+                final long autoSaveInterval = this.getConfig().getLong("options.performance-tweaking.quest-autosave-interval", 12000);
+                try {
+                    if (this.questAutoSaveTask != null && !this.questAutoSaveTask.isCancelled()) {
+                        this.questAutoSaveTask.cancel();
+                    }
+                    this.questAutoSaveTask = this.serverScheduler.runTaskTimer(new QuestsAutoSaveRunnable(this), autoSaveInterval, autoSaveInterval);
+                } catch (final Exception e) {
+                    this.getLogger().log(Level.SEVERE, "Cannot cancel and restart quest autosave task", e);
                 }
-                this.questAutoSaveTask = this.serverScheduler.runTaskTimer(() -> new QuestsAutoSaveRunnable(this), autoSaveInterval, autoSaveInterval);
-            } catch (final Exception e) {
-                this.questsLogger.debug("Cannot cancel and restart quest autosave task");
-            }
 
-            final long queueExecuteInterval = this.getConfig().getLong("options.performance-tweaking.quest-queue-executor-interval", 1);
-            try {
-                if (this.questQueuePollTask != null) {
-                    this.questQueuePollTask.cancel();
+                final long queueExecuteInterval = this.getConfig().getLong("options.performance-tweaking.quest-queue-executor-interval", 1);
+                try {
+                    if (this.questQueuePollTask != null && !this.questQueuePollTask.isCancelled()) {
+                        this.questQueuePollTask.cancel();
+                    }
+                    this.questQueuePollTask = this.serverScheduler.runTaskTimer(this.questCompleter, queueExecuteInterval, queueExecuteInterval);
+                } catch (final Exception e) {
+                    this.getLogger().log(Level.SEVERE, "Could not cancel and restart queue executor task", e);
                 }
-                this.questQueuePollTask = this.serverScheduler.runTaskTimer(this.questCompleter, queueExecuteInterval, queueExecuteInterval);
-            } catch (final Exception e) {
-                this.questsLogger.debug("Cannot cancel and restart queue executor task");
             }
 
             // Set number formats to be used
@@ -922,6 +929,6 @@ public class BukkitQuestsPlugin extends JavaPlugin implements Quests {
 
     @Override
     public void reloadConfig() {
-        this.reloadBaseConfiguration();
+        this.reloadBaseConfiguration(false);
     }
 }
diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/runnable/QuestsAutoSaveRunnable.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/runnable/QuestsAutoSaveRunnable.java
index 3ffc1876..ebc55581 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/runnable/QuestsAutoSaveRunnable.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/runnable/QuestsAutoSaveRunnable.java
@@ -2,39 +2,125 @@ package com.leonardobishop.quests.bukkit.runnable;
 
 import com.leonardobishop.quests.bukkit.BukkitQuestsPlugin;
 import com.leonardobishop.quests.bukkit.scheduler.WrappedRunnable;
-import org.bukkit.Bukkit;
+import com.leonardobishop.quests.bukkit.scheduler.WrappedTask;
 import org.bukkit.entity.Player;
+import org.jetbrains.annotations.NotNull;
 
-import java.util.LinkedList;
-import java.util.Queue;
+import java.util.ArrayDeque;
+import java.util.Deque;
 import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
 
-public class QuestsAutoSaveRunnable extends WrappedRunnable {
+public final class QuestsAutoSaveRunnable extends WrappedRunnable {
 
-    private final Queue<UUID> queue = new LinkedList<>();
     private final BukkitQuestsPlugin plugin;
+    private final Deque<UUID> saveDeque;
+    private int lastSaveSize;
+    private WrappedTask task;
 
-    public QuestsAutoSaveRunnable(BukkitQuestsPlugin plugin) {
-        for (Player player : Bukkit.getOnlinePlayers()) {
-            queue.add(player.getUniqueId());
-        }
-
+    public QuestsAutoSaveRunnable(final @NotNull BukkitQuestsPlugin plugin) {
         this.plugin = plugin;
+        this.saveDeque = new ArrayDeque<>();
+        this.lastSaveSize = 0;
     }
 
     @Override
     public void run() {
-        UUID player = queue.poll();
-        if (player == null) {
-            try {
-                super.cancel();
-            } catch (Exception ignored) {}
+        // log it just to be sure
+        QuestsAutoSaveRunnable.this.plugin.getQuestsLogger().debug("Ran outer autosave runnable");
+
+        // do not call force save if there were no players
+        if (this.lastSaveSize != 0) {
+            // we need to force save for remaining elements in case
+            // user set the interval option to a way too low value
+            this.forceSave(false);
+        }
+
+        // queue all the current online players
+        // it updates lastSaveSize field
+        this.updateSaveDeque();
+
+        // do not schedule the timer if there is no players
+        if (this.lastSaveSize == 0) {
             return;
         }
 
-        if (Bukkit.getPlayer(player) != null) {
-            plugin.getPlayerManager().savePlayer(player);
+        // log it just to be sure
+        QuestsAutoSaveRunnable.this.plugin.getQuestsLogger().debug("Scheduled inner autosave runnable");
+
+        // run autosave task timer again
+        this.task = new WrappedRunnable() {
+            @Override
+            public void run() {
+                final UUID playerId = QuestsAutoSaveRunnable.this.saveDeque.poll();
+
+                // cancel the task as there is nothing to save
+                if (playerId == null) {
+                    this.cancel();
+                    return;
+                }
+
+                // don't do that for offline as they will be saved on quit
+                final Player player = QuestsAutoSaveRunnable.this.plugin.getServer().getPlayer(playerId);
+                if (player != null && player.isOnline()) {
+                    QuestsAutoSaveRunnable.this.save(playerId);
+                }
+            }
+
+            @Override
+            public synchronized void cancel() throws IllegalStateException {
+                QuestsAutoSaveRunnable.this.plugin.getQuestsLogger().debug("Cancelled inner autosave runnable");
+                super.cancel();
+            }
+        }.runTaskTimer(this.plugin.getScheduler(), 2L, 2L);
+    }
+
+    private void forceSave(final boolean cancel) {
+        this.plugin.getQuestsLogger().debug("Called forceSave with cancel: " + cancel);
+
+        // cancel the task so it can be rescheduled
+        if (this.task != null && !this.task.isCancelled()) {
+            this.task.cancel();
         }
+
+        // log about needed config change
+        final int size = this.saveDeque.size();
+        if (!cancel && size > 0) {
+            this.plugin.getLogger().warning("Forced autosave for " + size + " of " + this.lastSaveSize + " players scheduled. "
+                    + "To avoid forced saves in the future and maintain smooth server performance, please consider increasing the "
+                    + "options.performance-tweaking.quest-autosave-interval setting.");
+        }
+
+        // force save leftovers
+        for (int i = 0; i < size; i++) {
+            final UUID uniqueId = this.saveDeque.removeFirst();
+
+            // joining is evil, but we can assume now that all was saved properly
+            // which makes us able to just continue and schedule another autosave
+            this.save(uniqueId).join();
+        }
+    }
+
+    private @NotNull CompletableFuture<Void> save(final @NotNull UUID uniqueId) {
+        if (this.plugin.getServer().getPlayer(uniqueId) != null) {
+            return this.plugin.getPlayerManager().savePlayer(uniqueId);
+        } else {
+            return CompletableFuture.completedFuture(null);
+        }
+    }
+
+    private void updateSaveDeque() {
+        for (final Player player : this.plugin.getServer().getOnlinePlayers()) {
+            this.saveDeque.add(player.getUniqueId());
+        }
+        this.lastSaveSize = this.saveDeque.size();
     }
 
+    @Override
+    public synchronized void cancel() throws IllegalStateException {
+        this.forceSave(true); // we need to force autosave
+
+        this.plugin.getQuestsLogger().debug("Cancelled outer autosave runnable");
+        super.cancel();
+    }
 }