Skip to content

perf: trace the Hand Pick crosshair once per tick and only near a site - #37

Merged
XxFran10xX merged 1 commit into
mainfrom
perf/idle-tick-cost
Oct 30, 2026
Merged

XxFran10xX merged 1 commit into
mainfrom
perf/idle-tick-cost

Conversation

@XxFran10xX

Copy link
Copy Markdown
Contributor

Why

Main profiles, 22 hourly spark dumps 10-29 07:00 → 10-30 05:00 (peak ~30 players, ~23 ms/tick total). HandPickService.tick cost 0.25 ms/tick. Every tick, for every player:

  • syncHeldTool, including getTargetBlockExact ray-traces and isExcavationFill;
  • DigTools.isAllowed → ItemMatcher.matches (ItemsAdder/MMOItems id lookups, with item NBT copies).

Change

  • The tool check comes before the ray-trace. A player without an excavation tool only gets the mining lock cleared.
  • The trace runs only when an established excavation's ruin chunk is within 8 blocks (SiteRepository.hasEstablishedIn). A trace can't return a prism cell farther away.
  • One trace and one site lookup per tick serve both the lock and the HUD (before: up to 3 of each). A player mid-dig isn't traced.
  • ItemMatcher asks ItemsAdder/MMOItems for a stack's id once per check instead of once per configured entry.
  • The lock condition, HUD condition and HUD text are kept exactly; only repeated, side-effect-free reads in the same tick were removed.

Testing

  • New tests:
    • no tool means no trace;
    • no site in reach means no trace, and in reach means one trace;
    • one trace per tick feeds both the lock and the HUD;
    • a player mid-dig isn't traced;
    • range edges;
    • each pack plugin is asked once per check.
  • 814 tests locally; the same 3 Windows-only SiteEstatesTest failures as main. Pack-API integration tests 10/10.
  • Dev (10-30, 11:55–16:47 UTC): all ten perf branches from this round ran together (InteractibleFurniture, Cooking, Magic, RPCharacters, Thievery, VFBuilders, SimpleFactions, Archaeo, VehicleFramework, TrialRooms). Bot probes ran on these jars and again on the previous jars for comparison. No errors from any of the ten plugins in the log, including through a scheduled restart. arprobe.js on an established site: hold a Hand Pick and look at fill, swap tools, look away, walk off. The new jar and the previous jar gave identical results, with no errors.
    • Caveat: on that Dev site neither jar showed the STRATUM HUD or locked vanilla mining, so this compared the tool/no-site paths. The locked path is covered by the unit tests only. Please try a real dig on Dev before merging.

🤖 Generated with Claude Code

The Hand Pick tick ran for every online player every tick. It matched the
held tool up to three times, traced the crosshair up to three times and
looked the target up in the site list each time, even for players holding
no excavation tool or standing nowhere near an excavation.

- A player without a whitelisted tool now only has the mining lock
  cleared; the crosshair is not traced (the result was never used).
- The crosshair is traced only when an established excavation's ruin
  chunk is within trace reach. A prism never leaves its ruin chunk, so
  outside that range the lookup could never find a site.
- One trace and one site lookup per tick now serve both the mining lock
  and the HUD. A cycling player needs no trace at all.
- Matching a stack against the whitelist asks ItemsAdder and MMOItems
  for its ids once per match instead of once per configured entry.

The lock, HUD text and cue timing are the same on the same tick.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: 41fbb8bd-a959-4ad9-b266-42c6e7dea8ae
Commits

Reviewing files that changed from the base of the PR and between 8454a96 and f69cae5.

Files selected for processing: 8
  • src/main/java/net/tfminecraft/archaeo/excavation/DigTools.java
  • src/main/java/net/tfminecraft/archaeo/excavation/HandPickService.java
  • src/main/java/net/tfminecraft/archaeo/item/ItemMatcher.java
  • src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java
  • src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java
  • src/test/java/net/tfminecraft/archaeo/item/DigToolsTest.java
  • src/test/java/net/tfminecraft/archaeo/item/ItemMatcherPackIT.java
  • src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.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

  • Bug Fixes
    • Mining controls now behave more consistently: vanilla mining remains locked only when using an allowed tool while targeting excavation fill, and is unlocked when no allowed tool is held.
    • The excavation HUD remains visible when aiming at opened air within an established site, as well as while cycling.
    • Crosshair targeting is limited to nearby established sites, helping avoid unnecessary checks during play.
  • Performance
    • Checks for supported tools and item matches are more efficient, with no change to existing item-matching rules.

The change adds per-stack item lookup caching and a chunk-range query for established sites. HandPickService uses the query to gate crosshair tracing and to share target information between mining-lock and HUD updates.

Changes: Excavation tool and targeting flow

Layer / File(s) Summary
Cache item lookups and restore allowed tools
src/main/java/net/tfminecraft/archaeo/item/ItemMatcher.java, src/main/java/net/tfminecraft/archaeo/excavation/DigTools.java, src/test/java/net/tfminecraft/archaeo/item/DigToolsTest.java, src/test/java/net/tfminecraft/archaeo/item/ItemMatcherPackIT.java
ItemMatcher adds a per-stack Probe that caches ItemsAdder and MMOItems lookups. DigTools reuses one probe across configured references and separates allowed-tool restoration from whitelist checking. Tests cover probe reuse and the updated matcher call.
Query established sites by chunk bounds
src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java, src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java
SiteRepository adds an inclusive chunk-range query for established sites in a named world. Tests cover range boundaries, world names, and lifecycle status.
Reuse nearby-site targeting for mining lock and HUD
src/main/java/net/tfminecraft/archaeo/excavation/HandPickService.java, src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java
HandPickService checks for a nearby established site before tracing. It reuses the target for mining-lock and HUD updates, and uses a shared reach constant. Tests cover tool changes, skipped traces, and trace reuse within a tick.

Priority: Low

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

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  actor Player
  participant HandPickService
  participant DigTools
  participant SiteRepository
  Player->>HandPickService: tick
  HandPickService->>DigTools: restoreAllowedTool
  HandPickService->>SiteRepository: hasEstablishedIn(world, chunk bounds)
  SiteRepository-->>HandPickService: established-site result
  HandPickService->>HandPickService: trace to REACH when a site is nearby
  HandPickService->>Player: update mining lock and HUD
Loading

Suggested reviewers: nowkoo, ryanbarlow97

Merge Risk: Minimal · up to f69ca

No concrete merge-blocking issue was found. A real-dig check remains useful because the locked mining and HUD path was not exercised on the Dev server.

- [ ] 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

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

@github-actions github-actions Bot added the size:L 100-499 effective changed lines (tests excluded in mixed PRs). label Oct 30, 2026
@XxFran10xX
XxFran10xX marked this pull request as ready for review October 30, 2026 17:28
@XxFran10xX

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
✅ 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.

@XxFran10xX
XxFran10xX merged commit bc90933 into main Oct 30, 2026
9 checks passed
@XxFran10xX
XxFran10xX deleted the perf/idle-tick-cost branch October 30, 2026 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 effective changed lines (tests excluded in mixed PRs).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant