perf: rewrite scanned food only when it would look different

pom.xml

@@ -15,7 +15,7 @@
         <interactiblefurniture.version>0.3.0</interactiblefurniture.version>
         <rpcharacters.version>2.16.0</rpcharacters.version>
         <simplefactions.version>4.0.0</simplefactions.version>
-        <tlibs.version>2.6.0</tlibs.version>
+        <tlibs.version>2.7.0</tlibs.version>
     </properties>
 
     <dependencyManagement>

src/main/java/net/tfminecraft/cooking/Cooking.java

@@ -310,6 +310,10 @@ public void loadConfigs() {
     public void reloadAll() {
         ItemRef.clearCache();
         loadConfigs();
+        if (ItemScanService.get() != null) {
+            // Food skipped as unchanged must be looked at again with the new config.
+            ItemScanService.get().rescanAll();
+        }
         HusbandryLifecycleListener.applyStatsRevision();
         craftingManager.rebuildStations();
         NutritionDrainTask.stop();

src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java

@@ -26,6 +26,16 @@ public boolean matches(ItemStack stack) {
         return isFood(data) ? SeafoodWholeItems.carriesCatchData(data) : stack.getType() == Material.COD;
     }
 
+    /** A slot that doesn't match never will until its item changes; a matching one is checked every scan, as before. */
+    @Override
+    public long scan(Player player, Inventory inventory, int slot, ItemStack stack) {
+        if (!matches(stack)) {
+            return WHEN_CHANGED;
+        }
+        update(player, inventory, slot, stack);
+        return EVERY_SCAN;
+    }
+
     @Override
     public void update(Player player, Inventory inventory, int slot, ItemStack stack) {
         if (inventory == null || slot < 0) {

src/main/java/net/tfminecraft/cooking/item/FoodItem.java

@@ -733,8 +733,12 @@ private static double roundToTenth(double value) {
     // --------------------------------------------------------------
     public static FoodItem fromItem(ItemStack stack) {
         if (stack == null || !stack.hasItemMeta()) return null;
+        return fromItem(stack, stack.getItemMeta());
+    }
 
-        ItemMeta meta = stack.getItemMeta();
+    /** As {@link #fromItem(ItemStack)}, reading the stack's meta the caller already has, so it isn't copied again. */
+    public static FoodItem fromItem(ItemStack stack, ItemMeta meta) {
+        if (stack == null || meta == null) return null;
         var pdc = meta.getPersistentDataContainer();
 
         String id = pdc.get(Keys.FOOD_ID, PersistentDataType.STRING);

src/main/java/net/tfminecraft/cooking/manager/TagManager.java

@@ -9,15 +9,15 @@
 import org.bukkit.persistence.PersistentDataType;
 
 import net.tfminecraft.cooking.item.FoodItem;
-import net.tfminecraft.cooking.utils.ItemUpdater;
+import net.tfminecraft.cooking.utils.FoodScan;
 import net.tfminecraft.cooking.utils.Keys;
 import net.tfminecraft.tlibs.itemscan.ItemScanHandler;
 
 public class TagManager implements ItemScanHandler {
 
     private static final int OFF_HAND_SLOT = 40;
 
-    // Runs on every scanned slot: read the key through the stack's data view instead of copying its whole meta.
+    // Runs on every new or changed slot: read the key through the stack's data view instead of copying its whole meta.
     @Override
     public boolean matches(ItemStack stack) {
         if (stack == null || stack.getType() == Material.AIR) {
@@ -26,27 +26,33 @@ public boolean matches(ItemStack stack) {
         return stack.getPersistentDataContainer().has(Keys.FOOD_ID, PersistentDataType.STRING);
     }
 
-    // Keep the existing legacy text representation, formatting, and exact-string comparisons.
-    @SuppressWarnings("deprecation")
     @Override
     public void update(Player player, Inventory inventory, int slot, ItemStack stack) {
-        FoodItem food = FoodItem.fromItem(stack);
-        if (food == null) {
-            return;
+        scan(player, inventory, slot, stack);
+    }
+
+    /**
+     * Ages a food and rewrites its slot only when it would look different (FoodScan), reading its meta once. Answers
+     * when TLibs must show it the unchanged item again: when it can next look different, or every scan while it is
+     * held, since held food waits for a visible change.
+     */
+    @Override
+    public long scan(Player player, Inventory inventory, int slot, ItemStack stack) {
+        if (!matches(stack)) {
+            return WHEN_CHANGED;
         }
-        if (!food.shouldUpdate()) {
-            ItemMeta meta = stack.getItemMeta();
-            if (meta != null && !ItemUpdater.needsLoreRebuild(
-                    meta.getLore(),
-                    meta.getPersistentDataContainer().has(Keys.LORE_INDEX_MAP, PersistentDataType.STRING))) {
-                return;
-            }
+        ItemMeta meta = stack.getItemMeta();
+        FoodItem food = FoodItem.fromItem(stack, meta);
+        if (food == null) {
+            // An unknown food id: only a reload can change that, and Cooking's reload rescans every slot.
+            return WHEN_CHANGED;
         }
-        boolean held = isHeldSlot(player, inventory, slot);
-        ItemStack updated = ItemUpdater.updateItem(stack, food, null, held);
+        FoodScan.Result result = FoodScan.update(stack, meta, food, isHeldSlot(player, inventory, slot));
+        ItemStack updated = result.item();
         if (updated != null && !updated.getType().isAir() && inventory != null && slot >= 0) {
             inventory.setItem(slot, updated);
         }
+        return result.recheckAt();
     }
 
     public static boolean isHeldSlot(Player player, Inventory inventory, int slot) {

src/main/java/net/tfminecraft/cooking/utils/FoodScan.java

@@ -0,0 +1,144 @@
+package net.tfminecraft.cooking.utils;
+
+import java.util.List;
+
+import org.bukkit.NamespacedKey;
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.meta.ItemMeta;
+import org.bukkit.persistence.PersistentDataContainer;
+import org.bukkit.persistence.PersistentDataType;
+
+import net.tfminecraft.cooking.item.FoodItem;
+import net.tfminecraft.cooking.item.tag.TagStep;
+import net.tfminecraft.cooking.item.tag.TagTrack;
+import net.tfminecraft.tlibs.itemscan.ItemScanHandler;
+
+/**
+ * The inventory scan's update of a food (TagManager). A food ages by the wall clock from its stored clock, value and
+ * remainder, so writing the aged numbers back changes nothing a player sees and the food ages the same either way.
+ * The scan therefore only rewrites a food that would look different (a new tag step, warmth gone, lore or model built
+ * differently, as after a reload or an update), whose clock must be set, or that has a sauce (below). It also says
+ * when the unchanged food could next look different, so TLibs can skip the slot until then.
+ */
+public final class FoodScan {
+
+    // Further than this the next step never comes: a food aging at a huge multiplier, or a track with no next step.
+    private static final double MAX_SECONDS = 1e12;
+
+    private FoodScan() {}
+
+    /**
+     * What the scan puts in the slot (null leaves it as it is) and the {@link ItemScanHandler#scan} answer: when the
+     * unchanged food needs the scan again.
+     */
+    public record Result(ItemStack item, long recheckAt) {}
+
+    /**
+     * Ages the food (parsed from stack, whose meta is meta) to now. A held food is only rewritten when today's held
+     * rule says so (a new step, warmth gone or broken lore), so eating it isn't interrupted.
+     */
+    // Keep the existing legacy text representation, formatting, and exact-string comparisons.
+    @SuppressWarnings("deprecation")
+    public static Result update(ItemStack stack, ItemMeta meta, FoodItem fi, boolean held) {
+        PersistentDataContainer pdc = meta.getPersistentDataContainer();
+        boolean broken = ItemUpdater.needsLoreRebuild(meta.getLore(), pdc.has(Keys.LORE_INDEX_MAP, PersistentDataType.STRING));
+        if (!fi.shouldUpdate()) {
+            return new Result(broken ? ItemUpdater.applyItemUpdate(stack, fi, null) : null, ItemScanHandler.WHEN_CHANGED);
+        }
+        if (fi.hasSauce()) {
+            // A sauce keeps no clock or remainder of its own, so it only ages by whole steps each time the food is
+            // rewritten. Rewrite it on every scan as before, so sauces age exactly as they always have.
+            return new Result(ItemUpdater.updateItem(stack, fi, null, held), ItemScanHandler.EVERY_SCAN);
+        }
+        long now = StackNormalizer.quantizedNow();
+        Long stored = pdc.get(Keys.LAST_UPDATE, PersistentDataType.LONG);
+        long deltaSeconds = stored == null ? 0 : (now - stored) / 1000;
+        boolean expired = WarmthUtils.expireIfRoomTemp(fi);
+        boolean changed = deltaSeconds > 0 && ItemUpdater.applyAging(fi, deltaSeconds);
+        fi.setLastUpdate(now);
+        long recheckAt = held ? ItemScanHandler.EVERY_SCAN : nextChange(fi, now);
+        if (changed || expired || broken || (!held && clockNeedsWriting(stored, now))) {
+            return new Result(ItemUpdater.applyItemUpdate(stack, fi, null), recheckAt);
+        }
+        if (held) {
+            return new Result(null, recheckAt);
+        }
+        ItemStack rebuilt = ItemUpdater.applyItemUpdate(stack.clone(), fi, null);
+        return new Result(looksTheSame(stack, meta, rebuilt) ? null : rebuilt, recheckAt);
+    }
+
+    /**
+     * True when the clock must be written: missing, ahead of now, or not on a whole second (as a newly built food's
+     * is). The scan used to set it to the second on every pass; once it is, leaving it gives the same aging.
+     */
+    static boolean clockNeedsWriting(Long stored, long now) {
+        return stored == null || stored > now || Math.floorMod(stored, 1000L) != 0;
+    }
+
+    /**
+     * The time from which this food, aged to now and left unchanged, can show its next tag step or lose its warmth.
+     * It is a second early, so rounding never makes the scan late; a check that comes early just finds nothing.
+     */
+    static long nextChange(FoodItem fi, long now) {
+        double seconds = Double.POSITIVE_INFINITY;
+        for (TagTrack track : fi.getTagTracks()) {
+            if (!track.isAgeable()) continue;
+            long target = nextStepValue(track);
+            if (target == Long.MAX_VALUE) continue;
+            double units = target - track.getValue() - fi.getAgeRemainder(track.getId());
+            seconds = Math.min(seconds, units * fi.resolveAgeMultiplier(track.getId()));
+        }
+        if (!(seconds <= MAX_SECONDS)) {
+            return ItemScanHandler.WHEN_CHANGED;
+        }
+        long whole = Math.max(0L, (long) Math.ceil(seconds) - 1L);
+        return now + whole * 1000L;
+    }
+
+    /**
+     * The track value at which its shown step next changes, or Long.MAX_VALUE if it never does: the first step it
+     * hasn't reached (TagTrack.getCurrentStep), and room temperature for warmth. Other tracks stop aging once they
+     * reach their last step (TagTrack.setValue).
+     */
+    static long nextStepValue(TagTrack track) {
+        boolean warmth = "warmth".equals(track.getId());
+        long next = warmth ? WarmthUtils.ROOM_TEMP_AGE : Long.MAX_VALUE;
+        List<TagStep> steps = track.getSteps();
+        int value = track.getValue();
+        if (steps.isEmpty() || (!warmth && value >= steps.get(steps.size() - 1).getRequiredValue())) {
+            return next;
+        }
+        for (TagStep step : steps) {
+            if (value < step.getRequiredValue()) {
+                return Math.min(next, step.getRequiredValue());
+            }
+        }
+        return next;
+    }
+
+    /** True when rebuilt differs from the current stack only in its aging clock, values and remainder. */
+    static boolean looksTheSame(ItemStack current, ItemMeta currentMeta, ItemStack rebuilt) {
+        ItemStack probe = rebuilt.clone();
+        ItemMeta probeMeta = probe.getItemMeta();
+        if (probeMeta == null) {
+            return false;
+        }
+        PersistentDataContainer from = currentMeta.getPersistentDataContainer();
+        PersistentDataContainer to = probeMeta.getPersistentDataContainer();
+        copy(from, to, Keys.LAST_UPDATE, PersistentDataType.LONG);
+        copy(from, to, Keys.AGE_REMAINDER, PersistentDataType.STRING);
+        copy(from, to, Keys.TAGS, PersistentDataType.STRING);
+        probe.setItemMeta(probeMeta);
+        return current.isSimilar(probe);
+    }
+
+    private static <T> void copy(PersistentDataContainer from, PersistentDataContainer to, NamespacedKey key,
+            PersistentDataType<?, T> type) {
+        T value = from.get(key, type);
+        if (value == null) {
+            to.remove(key);
+        } else {
+            to.set(key, type, value);
+        }
+    }
+}

src/test/java/net/tfminecraft/cooking/CookingLifecycleTest.java

@@ -260,9 +260,14 @@ class CookingLifecycleTest {
     @Test void reloadRefreshesConfigStationsAndPeriodicNutritionAndHusbandryTasks() {
         Cooking plugin = loadPlugin();
         server.getScheduler().performOneTick();
+        ItemScanService scan = mock(ItemScanService.class);
+        scans.when(ItemScanService::get).thenReturn(scan);
 
         plugin.reloadAll();
 
+        // Food the scan skipped as unchanged is looked at again with the new config.
+        verify(scan).rescanAll();
+
         verify(built(ConfigLoader.class), times(2)).loadConfig(new File(plugin.getDataFolder(), "config.yml"));
         verify(built(CraftingManager.class)).rebuildStations();
         husbandry.verify(HusbandryLifecycleListener::applyStatsRevision, times(2));

src/test/java/net/tfminecraft/cooking/fishing/LegacyFishScanSkipTest.java

@@ -0,0 +1,32 @@
+package net.tfminecraft.cooking.fishing;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.*;
+
+import org.bukkit.entity.Player;
+import org.bukkit.inventory.Inventory;
+import org.bukkit.inventory.ItemStack;
+import org.junit.jupiter.api.Test;
+
+import net.tfminecraft.tlibs.itemscan.ItemScanHandler;
+
+class LegacyFishScanSkipTest {
+
+    @Test
+    void onlyMatchingFishAreCheckedOnEveryScan() {
+        LegacyFishScan scan = spy(new LegacyFishScan());
+        Player player = mock(Player.class);
+        Inventory inventory = mock(Inventory.class);
+        ItemStack stack = mock(ItemStack.class);
+        doNothing().when(scan).update(any(), any(), anyInt(), any());
+
+        doReturn(false).when(scan).matches(stack);
+        assertEquals(ItemScanHandler.WHEN_CHANGED, scan.scan(player, inventory, 3, stack));
+        verify(scan, never()).update(any(), any(), anyInt(), any());
+
+        doReturn(true).when(scan).matches(stack);
+        assertEquals(ItemScanHandler.EVERY_SCAN, scan.scan(player, inventory, 3, stack));
+        verify(scan).update(player, inventory, 3, stack);
+    }
+}

src/test/java/net/tfminecraft/cooking/manager/CookingAndTagManagersCoverageTest.java

@@ -12,7 +12,6 @@
 import org.bukkit.Material;
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockFace;
-import org.bukkit.configuration.MemoryConfiguration;
 import org.bukkit.event.block.Action;
 import org.bukkit.event.player.PlayerInteractEvent;
 import org.bukkit.event.world.ChunkLoadEvent;
@@ -228,34 +227,4 @@ void scanMatchingRequiresFoodMetadataAndRecognizesTheOwningPlayersHeldSlots() {
         assertFalse(TagManager.isHeldSlot(env.player, null, 3)); assertFalse(TagManager.isHeldSlot(env.player, env.server.createInventory(null, 9), 3));
         assertFalse(TagManager.isHeldSlot(env.player, env.server.addPlayer().getInventory(), 3));
     }
-
-    @Test
-    void unchangedFoodAndNonFoodAreNotRewrittenButMissingLoreIsRebuilt() {
-        Inventory inventory = env.server.createInventory(null, 9); ItemStack item = env.stack(kitchen.food("steak", "meat"), Material.BEEF, 2);
-        inventory.setItem(0, item); tags.update(env.player, inventory, 0, new ItemStack(Material.STONE)); tags.update(env.player, inventory, 0, item);
-        kitchen.updater.verify(() -> ItemUpdater.updateItem(any(), any(), any(), anyBoolean()), never()); assertEquals(item, inventory.getItem(0));
-        kitchen.updater.when(() -> ItemUpdater.needsLoreRebuild(any(), anyBoolean())).thenReturn(true);
-        tags.update(env.player, inventory, 0, item); kitchen.updater.verify(() -> ItemUpdater.updateItem(item, env.resolve(item), null, false));
-        assertEquals(2, inventory.getItem(0).getAmount()); assertNotEquals(item, inventory.getItem(0));
-    }
-
-    @Test
-    void changingFoodUsesHeldContextOnlyForThePlayersSelectedOrOffhandSlot() {
-        MemoryConfiguration config = new MemoryConfiguration(); config.set("name", "Steak"); config.set("update", true);
-        FoodItem food = new FoodItem("steak", config); ItemStack stack = env.stack(food, Material.BEEF, 2); var inventory = env.player.getInventory();
-        inventory.setItem(0, stack); tags.update(env.player, inventory, 0, stack);
-        kitchen.updater.verify(() -> ItemUpdater.updateItem(stack, food, null, true)); assertEquals(2, inventory.getItem(0).getAmount());
-        tags.update(env.player, inventory, 1, stack); kitchen.updater.verify(() -> ItemUpdater.updateItem(stack, food, null, false));
-        tags.update(env.player, inventory, 40, stack); assertEquals(2, inventory.getItemInOffHand().getAmount());
-        tags.update(env.player, null, 0, stack); tags.update(env.player, inventory, -1, stack);
-    }
-
-    @ParameterizedTest
-    @ValueSource(strings = {"null", "air"})
-    void failedTagUpdatesNeverDeleteTheExistingFood(String failure) {
-        ItemStack source = env.stack(kitchen.food("steak", "meat"), Material.BEEF, 2); Inventory inventory = env.server.createInventory(null, 9); inventory.setItem(0, source);
-        kitchen.updater.when(() -> ItemUpdater.needsLoreRebuild(any(), anyBoolean())).thenReturn(true);
-        kitchen.updater.when(() -> ItemUpdater.updateItem(any(), any(), any(), anyBoolean())).thenReturn(failure.equals("null") ? null : new ItemStack(Material.AIR));
-        tags.update(env.player, inventory, 0, source); assertEquals(source, inventory.getItem(0));
-    }
 }