Repository navigation
perf: refresh pickup lore once per player per tick - #75
Merged
Merged
Conversation
Every item a player picked up queued its own next-tick refresh of the whole inventory's aura and charge lore, and each refresh re-reads the lore of every item. Picking up a pile queued one full refresh per item entity. The listener now queues one refresh per player per tick: further pickups before it runs ride on it, since it reads the whole inventory then. That is the same result as several refreshes, because ArtifactLore.apply and ChargeLore.apply strip their own block and rebuild it from the item's data. The queued mark is cleared by its own task, also when SessionGuard skips the refresh (player gone or character switched), so the next pickup always queues again. Coin pickups never reach this (DenarEconomy cancels them). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. Summary
Merge Risk:
 |
github-actions
Bot
added
the
size:S
XxFran10xX
marked this pull request as ready for review
November 3, 2026 20:45
Contributor
Author
|
@coderabbitai review |
Action performed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
XxFran10xX
added a commit
that referenced
this pull request
Nov 6, 2026
…ly changed slots (#76) * perf: refresh pickup lore only on items that can carry it, and set only changed slots Each pickup (once per player per tick since #75) ran both lore writers over all 41 inventory slots and set every slot back. For an ordinary item that meant about eight copies of its meta, its lore read twice and run through regexes (ArtifactLore.readAura), all to change nothing, and then a slot packet per slot. On Main at 15+ players that was ~3 s of main thread per hour. VesselLore now reads an item's meta once to tell whether a writer can touch it: aura or charge-tier data, a charge item match, or lore with a "/" (an aura line that readAura could read back). Other items come back unchanged, as both writers left them before. The inventory refresh sets a slot again only when its item changed; unchanged slots were only resent to the client. Lore output is the same: artifacts and charges still go through the same writers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Skip items without meta before any lookup, and test an artifact slot Review: both writers skip an item without meta, so do that first instead of looking up keys and charge matches on it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Prove skipped items reach neither writer, and cover the no-meta skip Review: the plain-item test could not tell the skip from the old path, and MockBukkit's item factory never reports an empty meta. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Picking up several items in one tick now schedules one lore refresh for the player instead of one per item.
Why
Every player pickup scheduled a full-inventory lore refresh (
VesselLore.updateInventory) for the next tick. Each refresh re-reads every item's lore (ArtifactLore.readAura), so picking up N items in one tick ran N identical refreshes. Coins are not affected, because DenarEconomy cancels their pickup. The cost is small (under 1.8 s/h on Main).How
onPickupschedules the sameSessionGuard.runLater(... updateInventory)only when none is pending, then a secondrunTaskclears the mark.ArtifactLore.applyandChargeLore.applystrip their own lore block and rebuild it from the item's data, so one refresh gives the same result as several.Checked
ArtifactListenerTest:ArtifactListenerhas 0 missed instructions, branches and lines. The full local suite fails only in Windows file-lock suites that this change doesn't touch.Dev (TFMCDev01, 11-03)
magic-BLOOD-PICKUP-250a7d7-DEV.jar: Blood Magic draft Add the hidden Blood Magic path #71 (2479d18) + the getHolder change + this commit. Same class list as the previous Dev jar; onlyArtifactListenerdiffers.Draft: not merged, not released, nothing on Main or Tutorial.
🤖 Generated with Claude Code