[XxFran10xX]

Companion PR: TF-Minecraft/Magic#72 (same scan path, Magic's artifact key check).

Why

TLibs ItemScanService scans one player's inventory (plus any open container) every 2 ticks. It calls every registered handler's matches() on every non-air slot, and update() when that returns true. Cooking registers two handlers:

• TagManager.matches called hasItemMeta() and then getItemMeta() to test for the food_id key.
• LegacyFishScan.matches called getItemMeta() on every fish.

Inventory.getItem returns a mirror, so each getItemMeta() builds a full CraftMetaItem. That copies every component plus the whole custom-data tag, which is large on MMOItems and ItemsAdder items. This happened at least once per slot per handler, 10 scans a second, only to read one key.

For food that matches, the update can rebuild the item. ModelData.apply then goes through ItemRef.apply/resolve, which asked TLibs for a fresh template every time. For ia.* refs that means CustomStack.getInstance(...).getItemStack(). Separately, ItemUpdater.needsLoreRebuild compiled the §. regex once per lore line through String.replaceAll.

Changes

• TagManager.matches and LegacyFishScan.matches now read the key through ItemStack#getPersistentDataContainer(), the read-only PersistentDataContainerView in Paper 1.21.10 API (confirmed in the paper-api-1.21.10-R0.1-SNAPSHOT jar these poms compile against). They check the same keys with the same types. SeafoodWholeItems.carriesCatchData now takes the view interface; PersistentDataContainer extends it, so existing callers are unchanged.
• ItemRef keeps ItemsAdder templates. It keeps one built template per ref string and gives every caller its own clone(). apply clones the kept template once and merges into the clone, so the kept template is never handed out or changed.
  • Only refs that TLibs routes to ItemsAdder are kept (the prefix rules mirror ItemCreator.getItemFromPath).
  • v. and modeled. are not kept: they are cheap to build.
  • m. is not kept: MMOItems builds can roll random stats or unique ids, so one kept copy would change what players get.
  • magic. is not kept: it has no creation path.
  • A prefix with a registered TLibs path handler is not kept: Cooking's own c. builds food with state, and handlers may build something different each time. This is checked on every call, so a prefix that gains a handler later stops being served from the cache.
  • Failed builds are never kept: null, air, and TLibs' dirt fallback (ItemsAdder missing, or the item not loaded yet). They are rebuilt on every call, as now, including TLibs' log lines.
• Cache invalidation:
  • ItemsAdder: the new ItemsAdderReloadHook listens for ItemsAdderLoadDataEvent (first load and /iareload) and clears the cache on each one. Cooking does not compile against ItemsAdder, so the event class is loaded from ItemsAdder's own class loader and registered with registerEvent; no pom, CI or plugin.yml change is needed. Templates are only switched on once this hook registers. Without ItemsAdder, or if the event is missing or refused, nothing is cached.
  • /cooking reload: reloadAll() clears the cache first.
  • TLibs: if TLibs hands out a new ItemCreator, the cache starts again from empty.
  • onDisable: turns caching off.
  • MMOItems: MMOItems does have MMOItemsReloadEvent, but it fires only on /mi reload, not on /mi edit changes, and m. refs are not cached at all (see above). So no MMOItems hook is needed.
• needsLoreRebuild uses a precompiled Pattern.compile("§."). String.replaceAll(regex, r) is defined as Pattern.compile(regex).matcher(this).replaceAll(r), so the output is identical.

Why behaviour is identical

• Key checks: the view and getItemMeta().getPersistentDataContainer() read the same PublicBukkitValues compound in custom_data, with the same type check. The dropped hasItemMeta() guard could only ever reject items whose PDC is empty, and those already answer false.
• Templates: an ItemsAdder template depends only on ItemsAdder's loaded data, which only changes on a load event, and that clears the cache. The first build of each ref is still returned as-is, and later uses get equal clones. Nothing changes when the scan runs or how often, and the once-a-second food write is untouched.
• Threads: the scan and every ItemRef caller run on the server thread. The cache is a ConcurrentHashMap behind a volatile reference, so a clear from any thread is safe, and a build that finishes after a clear writes into the discarded map.

Testing

• ScanMatchParityTest compares TagManager.matches and LegacyFishScan.matches against copies of the origin/main implementations. It covers null, air, items with and without meta, and the food id as text, as a number, or under another namespace. It also covers whole fish with and without catch data and the clean mark, on fish and non-fish materials.
• ItemRefTemplateCacheTest:
  • one build per ItemsAdder ref, with independent clones;
  • callers mutating their copy never reach later copies, and apply merges never reach the template;
  • failures are rebuilt every time;
  • non-ItemsAdder prefixes are never kept, including mixed case and a prefix that gains a path handler;
  • clear, caching off and a new TLibs creator each rebuild.
• ItemsAdderReloadHookTest: no ItemsAdder means nothing is cached; every load event clears the cache; a missing or unregistrable event class leaves caching off.
• ItemLoreRebuildTest.colourCodesAreStrippedExactlyAsBefore compares against the old replaceAll across about 530 lore pairs, including line breaks, surrogates, doubled § and trailing §.
• mvn -B -o clean verify on Windows: 1286 tests, 0 failures, line-coverage gate passes.

Dev testing on TFMCDev01 follows before this leaves draft.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 125c4670-b806-4d6e-bc6b-64fabc2c052a

Commits

Reviewing files that changed from the base of the PR and between 50d78a2 and a7f03fb.

Files selected for processing: 13

• src/main/java/net/tfminecraft/cooking/Cooking.java
• src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java
• src/main/java/net/tfminecraft/cooking/fishing/SeafoodWholeItems.java
• src/main/java/net/tfminecraft/cooking/manager/TagManager.java
• src/main/java/net/tfminecraft/cooking/utils/ItemRef.java
• src/main/java/net/tfminecraft/cooking/utils/ItemUpdater.java
• src/main/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHook.java
• src/test/java/dev/lone/itemsadder/api/Events/ItemsAdderLoadDataEvent.java
• src/test/java/net/tfminecraft/cooking/fishing/FishingRuntimeCoverageTest.java
• src/test/java/net/tfminecraft/cooking/fishing/ScanMatchParityTest.java
• src/test/java/net/tfminecraft/cooking/utils/ItemLoreRebuildTest.java
• src/test/java/net/tfminecraft/cooking/utils/ItemRefTemplateCacheTest.java
• src/test/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHookTest.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

• Performance
  • Repeated item lookups can now reuse cached templates when ItemsAdder is available. The cache is refreshed when ItemsAdder reloads and cleared during configuration reloads and plugin shutdown.
• Bug Fixes
  • Food and catch checks now read persistent item data directly, including for items without metadata.
• Tests
  • Added coverage for item matching, lore checks, template caching and cache refresh behaviour.

The changes add an ItemsAdder-aware cache for item templates, update persistent-data access in item matching, and reuse a compiled pattern in lore checks.

Changes: Item and data updates

Layer / File(s)	Summary
Cache eligible item templates src/main/java/net/tfminecraft/cooking/utils/ItemRef.java, src/test/java/net/tfminecraft/cooking/utils/ItemRefTemplateCacheTest.java	ItemRef caches eligible ItemsAdder item templates and returns cloned copies. Tests cover cache eligibility, reuse, invalidation, and build failures.
Connect cache to ItemsAdder lifecycle src/main/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHook.java, src/main/java/net/tfminecraft/cooking/Cooking.java, src/test/java/dev/lone/itemsadder/api/Events/ItemsAdderLoadDataEvent.java, src/test/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHookTest.java	Cooking registers the reload hook when ItemsAdder is available. Successful registration enables caching, and load events clear the cache. Shutdown disables caching, and configuration reload clears it. Tests cover registration and cache behaviour across load events.
Read persistent data for item matching src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java, src/main/java/net/tfminecraft/cooking/fishing/SeafoodWholeItems.java, src/main/java/net/tfminecraft/cooking/manager/TagManager.java, src/test/java/net/tfminecraft/cooking/fishing/ScanMatchParityTest.java, src/test/java/net/tfminecraft/cooking/fishing/FishingRuntimeCoverageTest.java	Fishing and tag matching read persistent data directly from item stacks. The parity tests compare updated matchers with reference checks.
Reuse the lore colour-code pattern src/main/java/net/tfminecraft/cooking/utils/ItemUpdater.java, src/test/java/net/tfminecraft/cooking/utils/ItemLoreRebuildTest.java	ItemUpdater reuses a compiled colour-code pattern. Tests compare the lore check with the previous rule across varied inputs.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant Cooking
  participant ItemsAdderReloadHook
  participant ItemRef
  participant TLibs
  Cooking->>ItemsAdderReloadHook: Register reload hook
  ItemsAdderReloadHook->>ItemRef: Enable caching after registration
  ItemRef->>TLibs: Build item on cache miss
  ItemsAdderReloadHook->>ItemRef: Clear cache on load event

Loading

Suggested reviewers: ryanbarlow97

Merge Risk: Minimal · up to a7f03

The change reuses item templates and clears them on reload. No merge-blocking failure has been established.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[XxFran10xX]

Dev test (TFMCDev01, 10-29 13:28–13:33 UTC, this PR's jar on its own; ~/perf-1029-test/scan.js and model.js on tf): PASS

• A bot holding two warm chicken legs, a basil and an artifact: the legs stay one stack of 2, warmth and freshness age, last_update advances every scan, over 8 s and again after /cooking reload and after a full iareload (cache reset hook). No Cooking/TLibs/Magic errors.
• Models match the other Cooking build on Dev exactly, fresh and after 6 s of scan rebuilds: chicken leg custom_model_data 10251, basil 10005, same names.
• Dev normally runs another session's cooking-DEV-sandwich.jar; it was swapped out only for this test and restored byte-identical with Cooking's data folder afterwards.

[XxFran10xX]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.