feat: improve unlock state sync backlog handling - thanks @Xaikii

Closes #237

common/src/main/java/com/klikli_dev/modonomicon/bookstate/BookUnlockStateManager.java

@@ -16,23 +16,25 @@
 import com.klikli_dev.modonomicon.networking.RequestSyncBookStatesMessage;
 import com.klikli_dev.modonomicon.networking.SyncBookUnlockStatesMessage;
 import com.klikli_dev.modonomicon.platform.Services;
+import it.unimi.dsi.fastutil.objects.ObjectLinkedOpenHashSet;
 import net.minecraft.resources.ResourceLocation;
+import net.minecraft.server.MinecraftServer;
 import net.minecraft.server.level.ServerPlayer;
+import net.minecraft.server.players.PlayerList;
 import net.minecraft.util.datafix.DataFixTypes;
 import net.minecraft.world.entity.player.Player;
 import net.minecraft.world.level.saveddata.SavedData;
 
 import java.util.List;
-import java.util.TimerTask;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
+import java.util.Set;
+import java.util.UUID;
 
 public class BookUnlockStateManager {
 
     private static final BookUnlockStateManager instance = new BookUnlockStateManager();
+    private final Set<UUID> syncRequestedPlayers = new ObjectLinkedOpenHashSet<>();
     public BookStatesSaveData saveData;
-    protected ScheduledExecutorService updateAndSyncTimer;
+    private boolean wasLoaded = false;
 
     public static BookUnlockStateManager get() {
         return instance;
@@ -66,33 +68,8 @@ public void updateAndSyncFor(ServerPlayer player) {
             this.saveData.setDirty();
             this.syncFor(player);
         } else {
-            //we have some edge cases where RecipesUpdatedEvent is fired after EntityJoinLevelEvent.
-            //in SP this means that books are not built yet when updateAndSyncFor is called for the first time.
-            //so we poll until it is available.
-
-            //if timer already shut down, set to null so a new one will be created
-            if (this.updateAndSyncTimer != null && this.updateAndSyncTimer.isShutdown()) {
-                this.updateAndSyncTimer = null;
-            }
-
-            //if we don't have a timer yet, create one
-            if (this.updateAndSyncTimer == null) {
-                this.updateAndSyncTimer = Executors.newSingleThreadScheduledExecutor();
-            }
-
-            final var currentTimer = this.updateAndSyncTimer;
-            //then schedule a task to run in 5 seconds
-            this.updateAndSyncTimer.schedule(new TimerTask() {
-                @Override
-                public void run() {
-                    player.server.execute(() -> {
-                        BookUnlockStateManager.this.updateAndSyncFor(player);
-
-                        //we also shut down the timer to free the thread, as this is only rarely used.
-                        currentTimer.shutdown();
-                    });
-                }
-            }, 5, TimeUnit.SECONDS);
+            this.syncRequestedPlayers.add(player.getUUID());
+            this.wasLoaded = false;
         }
     }
 
@@ -171,4 +148,20 @@ private void getSaveDataIfNecessary(Player player) {
             }
         }
     }
+
+    public void onServerTickEnd(MinecraftServer server) {
+        if (server.getTickCount() % 100 != 0) return; //We only update every 5 seconds (100 ticks)
+        boolean newState = BookDataManager.get().areBooksBuilt();
+        if (newState != this.wasLoaded) { // we only check for things if the state changed for some reason.
+            if (!this.wasLoaded && !this.syncRequestedPlayers.isEmpty()) { //we only process players if we have any and the state was correct.
+                PlayerList list = server.getPlayerList();
+                for (UUID id : this.syncRequestedPlayers) {
+                    ServerPlayer player = list.getPlayer(id);
+                    if (player != null) this.updateAndSyncFor(player);
+                }
+                this.syncRequestedPlayers.clear();
+            }
+            this.wasLoaded = newState;
+        }
+    }
 }

fabric/src/main/java/com/klikli_dev/modonomicon/ModonomiconFabric.java

@@ -20,6 +20,7 @@
 import net.fabricmc.api.ModInitializer;
 import net.fabricmc.fabric.api.command.v2.CommandRegistrationCallback;
 import net.fabricmc.fabric.api.event.lifecycle.v1.ServerLifecycleEvents;
+import net.fabricmc.fabric.api.event.lifecycle.v1.ServerTickEvents;
 import net.fabricmc.fabric.api.event.lifecycle.v1.ServerWorldEvents;
 import net.fabricmc.fabric.api.event.player.UseBlockCallback;
 import net.fabricmc.fabric.api.itemgroup.v1.ItemGroupEvents;
@@ -92,6 +93,11 @@ public void onInitialize() {
 
         UseBlockCallback.EVENT.register(LecternIntegration::rightClick);
 
+        //We use server tick to flush the queue of players that need a book state sync
+        ServerTickEvents.END_SERVER_TICK.register((server) -> {
+            BookUnlockStateManager.get().onServerTickEnd(server);
+        });
+
         //Advancement event handling for condition/unlock system
         //done in MixinPlayerAdvancements, because we have no event in Fabric
 

forge/src/main/java/com/klikli_dev/modonomicon/ModonomiconForge.java

@@ -114,6 +114,11 @@ public ModonomiconForge() {
         //Advancement event handling for condition/unlock system
         MinecraftForge.EVENT_BUS.addListener((AdvancementEvent.AdvancementEarnEvent e) -> BookUnlockStateManager.get().onAdvancement((ServerPlayer) e.getEntity()));
 
+        //We use server tick to flush the queue of players that need a book state sync
+        MinecraftForge.EVENT_BUS.addListener(((TickEvent.ServerTickEvent.Post e) -> {
+            BookUnlockStateManager.get().onServerTickEnd(e.getServer());
+        }));
+
         //Datagen
         modEventBus.addListener(DataGenerators::gatherData);
 

neo/src/main/java/com/klikli_dev/modonomicon/ModonomiconNeo.java

@@ -45,6 +45,7 @@
 import net.neoforged.neoforge.event.entity.player.AdvancementEvent;
 import net.neoforged.neoforge.event.entity.player.PlayerInteractEvent;
 import net.neoforged.neoforge.event.level.LevelEvent;
+import net.neoforged.neoforge.event.tick.ServerTickEvent;
 
 @Mod(Modonomicon.MOD_ID)
 public class ModonomiconNeo {
@@ -114,6 +115,11 @@ public ModonomiconNeo(IEventBus modEventBus, ModContainer modContainer) {
         //Advancement event handling for condition/unlock system
         NeoForge.EVENT_BUS.addListener((AdvancementEvent.AdvancementEarnEvent e) -> BookUnlockStateManager.get().onAdvancement((ServerPlayer) e.getEntity()));
 
+        //We use server tick to flush the queue of players that need a book state sync
+        NeoForge.EVENT_BUS.addListener(((ServerTickEvent.Post e) -> {
+            BookUnlockStateManager.get().onServerTickEnd(e.getServer());
+        }));
+
         //Datagen
         modEventBus.addListener(DataGenerators::gatherData);