Improve gemstone stat inheritance in alloys

[XxFran10xX]

Summary

• Give gemstone catalysts a configurable stat inheritance roll of 55 + 5 × ingredient value.
• Merge a gemstone's own positive stat amount when the alloy base already has that stat, covering armor and any other matching gem stat.
• Preserve existing non-gem catalyst behavior and prevent a later weaker gem from lowering the cap for a shared new stat.

Verification

• mvn clean verify: 4 tests passed; runtime JAR packaged.
• Tests cover all 40 configured gemstone item paths, tier odds, matching armor and matching health stats.

Rollout

• Existing server configs without the new keys use the 55/5 defaults.
• Existing saved alloy recipes and alloy stats are unchanged; new discoveries use the new roll.

Summary by CodeRabbit

• New Features
  • Gemstone catalysts can now independently add their stat modifiers during alloy forging, even when the base item lacks the stat.
  • Added configurable gemstone stat-roll chances, with a 55% base chance and a 5% bonus per ingredient value by default.

[coderabbitai]

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: e9669eaf-2bed-4fc8-8dd7-fa51202058ff

📥 Commits

Reviewing files that changed from the base of the PR and between 07565e2 and 70a91c4.

📒 Files selected for processing (6)

• pom.xml
• src/main/java/net/tfminecraft/advancedcrafting/cache/Cache.java
• src/main/java/net/tfminecraft/advancedcrafting/loaders/ConfigLoader.java
• src/main/java/net/tfminecraft/advancedcrafting/objects/alloys/AlloyForger.java
• src/main/resources/config.yml
• src/test/java/net/tfminecraft/advancedcrafting/objects/alloys/AlloyForgerTest.java

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

---

📝 Walkthrough

Walkthrough

The change adds configurable gemstone stat inheritance chances. Alloy stat merging now identifies gemstone catalysts and copies successful gemstone modifiers at full amount. New tests cover gemstone-path recognition, chance calculations, and modifier merging.

Changes

Gemstone Catalyst Stat Inheritance

Layer / File(s)	Summary
Gemstone chance settings src/main/java/net/tfminecraft/advancedcrafting/cache/Cache.java, src/main/java/net/tfminecraft/advancedcrafting/loaders/ConfigLoader.java, src/main/resources/config.yml	The configuration defines a base chance of 55.0 and a per-value bonus of 5.0. ConfigLoader.load applies bounds to both settings and stores them in Cache.
Gemstone merge rules src/main/java/net/tfminecraft/advancedcrafting/objects/alloys/AlloyForger.java, pom.xml, src/test/java/net/tfminecraft/advancedcrafting/objects/alloys/AlloyForgerTest.java	AlloyForger recognizes gemstone paths without case sensitivity and applies the configured chance to gemstone modifiers. Successful gemstone modifiers are copied at full amount. Tests cover path classification, chance values, and merge results. JUnit 4.13.2 is added as a test dependency.

Priority: ⬇️ Low

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

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant AlloyForger
  participant Cache
  participant CatalystModifier
  participant BaseModifier
  AlloyForger->>Cache: Read gemstone chance settings
  AlloyForger->>CatalystModifier: Check item path and modifier
  AlloyForger->>BaseModifier: Check matching stat and cap
  AlloyForger->>CatalystModifier: Copy modifier when inheritance succeeds

Loading

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 70a91

No actionable issue remains before merge; normal build and test checks still apply.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 70a91

The new rules affect newly forged alloys, but ingredient permissions and protected-stat checks remain in place, and existing saved recipes retain their results. No new security issue was established. Some runtime and persistence behavior remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — A player can select permitted ingredients for a forge attempt, while the server’s ingredient definitions and configuration determine gemstone classification, modifier values, and inheritance odds. Changed outcomes can become saved alloys; no new external service or credential boundary was evidenced.

Trust Boundaries and Controls

• observed — The player-facing forge rechecks permission for each ingredient. Gemstone modifiers remain subject to protected-stat filtering and the shared merge and cap stage; the new chance settings do not replace those controls.

Resilience and Maintainability Implications

• inferred — The pre-existing, separate alloy and recipe writes can leave an incomplete discovery after a partial failure. The available source does not establish whether runtime scheduling prevents concurrent discoveries of the same recipe, so these lifecycle guarantees remain uncertain rather than PR-introduced security findings.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (2 skipped: 2…	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: improved gemstone stat inheritance in alloys, including the configurable inheritance behavior.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (2 skipped: 2 unsupported.)

• 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 each gemstone bright
And tunes the chance by value just right
Full modifiers hop into the forge
Base stats stay steady, caps enlarge
The tests track each sparkling trail
Then I tuck in my carrot pail}

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