Repository navigation
perf: read the artifact key through the stack's data view - #72
Conversation
The TLibs item scan calls ArtifactAttuneScanHandler.matches() on every non-air slot ten times a second. ArtifactIds.hasKey copied the whole item meta to test one key; it now reads the key through the read-only ItemStack#getPersistentDataContainer view, with the same answers for items with and without meta. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Direct persistent-data key check src/main/java/net/tfminecraft/magic/artifact/ArtifactIds.java, src/test/java/net/tfminecraft/magic/artifact/ArtifactIdsKeyViewTest.java, src/test/java/net/tfminecraft/magic/ChargeEdgeCoverageTest.java |
ArtifactIds.hasKey reads the stack’s persistent data container directly. Tests compare artifact and attunement checks with a full-meta reference and provide an empty persistent-data container for the no-metadata case. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Refactor
Suggested labels: size:S
Suggested reviewers: ryanbarlow97
Merge Risk: 
8afa3
-
Artifact key lookups should retain their prior behavior. No merge-blocking issue was identified; run the standard CI checks before merging.
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.
Comment @coderabbitai help to get the list of available commands.
github-actions
Bot
added
the
size:XS
|
Dev test (TFMCDev01, 10-29 13:26 UTC; Built as |
|
@coderabbitai review |
✅ Action performed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Companion PR: TF-Minecraft/Cooking#98 (same scan path: Cooking's food and fish checks, plus kept ItemsAdder templates).
Why
TLibs
ItemScanServicescans one player's inventory (plus any open container) every 2 ticks. It calls every registered handler'smatches()on every non-air slot. Magic'sArtifactAttuneScanHandler.matches→ArtifactIds.hasKeycalledhasItemMeta()and thengetItemMeta()to test for theartifact_idkey.Inventory.getItemreturns a mirror, so eachgetItemMeta()builds a fullCraftMetaItem. That copies every component and the whole custom-data tag, which is large on MMOItems and ItemsAdder items. This happened once per slot, 10 scans a second, only to read one key.Changes
ArtifactIds.hasKeynow reads the key throughItemStack#getPersistentDataContainer(), the read-onlyPersistentDataContainerViewin Paper 1.21.10 API (confirmed in thepaper-api-1.21.10-R0.1-SNAPSHOTjar the pom compiles against). It checks the same key with the sameSTRINGtype.hasKeyget the same saving.ArtifactCareStore.applyinupdate()is untouched. It only runs for artifacts.Why behaviour is identical
The view and
getItemMeta().getPersistentDataContainer()read the samePublicBukkitValuescompound incustom_data, with the same type check. The droppedhasItemMeta()guard could only ever reject items whose PDC is empty, and those already answer false. Null and air still answer false before any read. The scan runs on the server thread, and nothing changes about when it runs or whatupdate()does.Testing
ArtifactIdsKeyViewTestcompareshasKey(andArtifactAttuneScanHandler.matches) against a copy of the origin/main implementation. It covers null, air, items with and without meta, written artifacts, a blank id, the id stored as a number, another artifact key only, and the id under another namespace.ChargeEdgeCoverageTeststubs the data view on its mocked no-meta stack, because Mockito returns null where Paper always returns a view.mvn -B -o clean verifyon Windows: the only failures are the same 147 Windows-onlygear.ownershipfile tests that fail on origin/main, and the jacoco gaps match origin/main exactly. Linux CI is authoritative.Dev testing on TFMCDev01 follows before this leaves draft.
🤖 Generated with Claude Code