Skip to content

feat: record the materials each mage weapon was crafted from - #36

Open
XxFran10xX wants to merge 1 commit into
mainfrom
feat/stamp-gear-craft-inputs
Open

XxFran10xX wants to merge 1 commit into
mainfrom
feat/stamp-gear-craft-inputs

Conversation

@XxFran10xX

@XxFran10xX XxFran10xX commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Recycler returns the current cost of a weapon's stamped parts. That is wrong when a part's cost changed after crafting, and it returns materials for weapons staff crafted for free (magic.bypass_crafting_cost).

  • GearInventoryManager.tryPrepare stamps the charged map (already computed for abort refunds) onto the prepared weapon: magic:gear_craft_inputs, JSON item path to amount. Empty for staff bypass crafts.
  • Both copyGearPdc helpers (socket rewrite after charging, and GearRefresher) carry the stamp to the rebuilt item, like gear_parts.
  • GearProvenance.readInputs(ItemStack) returns the map, or null for weapons crafted before this change.

Recycler will read this in a follow-up PR.

Note: this overlaps with the open coverage PR #35 (same GearProvenance/GearItemBuilder files); whichever merges second needs a small rebase.

Testing

  • mvn verify passes locally (Magic main has no test suite yet).
  • End-to-end craft → recycle test on TFMCDev01 together with the Recycler change.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Crafted gear now keeps a record of the materials charged during its creation.
    • This crafting information remains available when gear is refreshed or rewritten, including items crafted without material charges.

The weapon now carries what the craft actually charged (gear_craft_inputs
PDC, item path to amount; empty for staff bypass crafts). Socket rewrites
and refreshes copy it along with the part list. Recycler reads it with
GearProvenance.readInputs instead of today's part costs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 7f152d50-fc4e-4d78-879e-839c304a0dd4

📥 Commits

Reviewing files that changed from the base of the PR and between 927045e and b5956c7.

📒 Files selected for processing (5)
  • src/main/java/net/tfminecraft/magic/gear/GearItemBuilder.java
  • src/main/java/net/tfminecraft/magic/gear/GearKeys.java
  • src/main/java/net/tfminecraft/magic/gear/GearProvenance.java
  • src/main/java/net/tfminecraft/magic/gear/GearRefresher.java
  • src/main/java/net/tfminecraft/magic/gear/gui/GearInventoryManager.java

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


📝 Walkthrough

Walkthrough

Gear crafting now records the materials charged as JSON metadata on the prepared item. Gear copy and refresh operations preserve this metadata.

Changes

Craft input provenance

Layer / File(s) Summary
Craft input metadata
src/main/java/net/tfminecraft/magic/gear/GearKeys.java, src/main/java/net/tfminecraft/magic/gear/GearProvenance.java
Adds the craftInputs key and methods to store charged materials as JSON and read them as a map.
Stamp and preserve craft inputs
src/main/java/net/tfminecraft/magic/gear/gui/GearInventoryManager.java, src/main/java/net/tfminecraft/magic/gear/GearItemBuilder.java, src/main/java/net/tfminecraft/magic/gear/GearRefresher.java
Stamps the prepared item with the charged-cost map after costs are taken. Copy and refresh paths copy the stored value when present.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: drefvelin

Merge Risk: ⚪ Minimal · up to b5956

Crafted gear records its cost map and carries it through the inspected rebuild paths. No actionable merge blocker is established; normal checks remain appropriate.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b5956

The change preserves craft-cost history without moving current refund authority to item metadata. No new material-credit attack path was demonstrated, but completeness of the recorded costs and compatibility with future recycling remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The demonstrated new write scope is prepared gear metadata and copies carried through rebuilding and station item persistence. Current abort material credits still derive from station occupancy, not the item's craft-input JSON.

Security Findings and Attack Paths

  • inferred — No present metadata-to-material-credit attack path was established. No readInputs caller was resolved in the inspected repository, the copy helpers only preserve the string, and the current refund path uses occupancy-owned costs. This conclusion does not cover the planned Recycler consumer or external callers.

Trust Boundaries and Controls

  • observed — The resolved writer receives server-computed costs from selected parts, rather than accepting player-supplied JSON. Abort authorization retains its owner UUID or magic.admin check before removing occupancy and issuing a refund. The new metadata parser does not participate in that authorization.

Resilience and Maintainability Implications

  • observed — Shutdown stops orb processing before saving and clearing station state. Reload reconstructs the saved item, owner, and charged map; missing furniture causes the saved item to be dropped. These recovery paths retain separate station-owned accounting rather than relying on parsing the new stamp.

Hardening Proposals

  • proposed — Before a future recycler makes this metadata authoritative for material credits, define validation for allowed item paths and bounded nonnegative amounts, preserve explicit-empty versus missing or corrupt semantics, and establish the evidence needed to trust the recorded charge. Namespacing and typed JSON decoding alone do not supply those guarantees.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: recording the materials used to craft each mage weapon.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit checks the crafting tray,
And notes what costs were paid today.
The map is tucked in gear with care,
Then copied when the item’s repaired.
Through refresh, the record stays—
Hop, hop, through metadata’s maze!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant