Skip to content

feat: add per-base success bonus to the alloy forge scrap roll - #23

Merged
XxFran10xX merged 1 commit into
mainfrom
feat/alloy-base-bonus
Sep 27, 2026
Merged

XxFran10xX merged 1 commit into
mainfrom
feat/alloy-base-bonus

Conversation

@XxFran10xX

@XxFran10xX XxFran10xX commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds a flat, per-base success bonus to the alloy forge's new-discovery roll, so higher-tier base ingots make scrap less likely:

success% = min(max, base-success + bonus-per-sqrt × √totalValue + base-bonus[base ingredient])
alloy-forge:
  base-bonus-percent:
    steel_ingot: 10.0
    bronze_ingot: 15.0
    abyssalite_ingot: 20.0
    mythril_ingot: 25.0
  • The bonus applies only to the base ingredient. Catalysts never get it, so putting a mythril ingot into an iron forge is not a shortcut.
  • Ingredient value fields are unchanged. Raising them would also have strengthened alloy stat rolls, bumped ingredient revisions, and shifted Thievery totals.
  • If the section is missing, or an ingredient is not listed, its bonus is 0. Existing configs behave exactly as before until the section is added.
  • The bonus still respects max-success-percent. It is reloaded with ac reload.

Resulting scrap chance (base + one tier-1 gem)

Base Before After
Iron 91.1% 91.1%
Steel 89.1% 79.1%
Bronze 87.4% 72.4%
Abyssalite 86.0% 66.0%
Mythril 81.5% 56.5%

Only combinations nobody has forged yet use the new chance. Recorded recipes keep their result.

Testing

  • mvn clean verify builds locally.
  • The PR build jar will be loaded on TFMCDev01 to check startup, config load and ac reload.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added configurable scrap discovery bonuses for selected base ingredients, with bonuses ranging from 10% to 25%.
    • Unlisted ingredients and catalysts receive no base-ingredient bonus.

Higher-tier base ingots now lower the scrap chance of a new alloy
discovery through a flat, configurable bonus
(alloy-forge.base-bonus-percent). It only applies to the base, so
ingots used as catalysts gain nothing, and it does not touch
ingredient values, which also drive stat rolls, revisions and
Thievery totals.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@coderabbitai

coderabbitai Bot commented Sep 27, 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: 7af19484-3052-4d55-b845-020a6c10d935

📥 Commits

Reviewing files that changed from the base of the PR and between 0b781d0 and 4ed651e.

📒 Files selected for processing (4)
  • 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

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

The alloy forge configuration now defines flat bonuses for selected base ingredients. The loader caches positive values after clamping them to 0–100. AlloyForger.isScrap() adds the matching bonus to its success chance.

Changes

Alloy Forge Base Bonus

Layer / File(s) Summary
Configure and apply base bonus
src/main/resources/config.yml, 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
The configuration assigns bonuses to selected base ingredients. ConfigLoader loads positive, clamped values into Cache by lowercase ingredient ID. AlloyForger.isScrap() adds the base item’s bonus before applying the existing cap.

Priority: ⬇️ Low

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

Change: Feature

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 4ed65

Configured base bonuses apply to new discoveries, recorded recipes retain their stored outcomes, and ac reload refreshes the bonus settings. No actionable merge-blocking behavior is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4ed65

The bonus remains limited to eligible base ingredients and does not bypass forge permissions. A failed configuration reload can nevertheless change the odds used for new discoveries, and those outcomes can remain recorded after the configuration is repaired.

Retained concerns

  • Low · reliability · inferred: A failed configuration load can erase the active base bonuses; a subsequent new-discovery roll can permanently record an outcome under unintended odds. A later successful reload restores the bonuses but does not recalculate that recipe.
Security review details

Security Blast Radius

  • inferred — A changed new-discovery outcome is stored by recipe combination and can affect later forges of that combination. Recorded combinations do not receive a new roll solely because bonuses change.

Trust Boundaries and Controls

  • observed — The bonus lookup receives the station base, not its catalysts. Station insertion checks base eligibility, and the player-facing forge path checks ingredient permissions; no new permission bypass is established.

Resilience and Maintainability Implications

  • observed — The shared map is cleared and repopulated in place rather than published as a completed snapshot. Source evidence does not establish whether reload and forge execution can overlap at runtime.

Hardening Proposals

  • proposed — Validate and assemble a complete bonus map before publishing it, and retain the last known-good configuration when loading fails. This would protect new-discovery outcomes without depending on reload thread affinity.
🚥 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 4 functions across 3 files. (1 skipped: 1 … 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: adding a per-base success bonus to the Alloy Forge scrap roll.
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 4 functions across 3 files. (1 skipped: 1 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 the forge at night
Steel earns a boost, bronze shines bright
Abyssalite and mythril rise
The cached chance now joins the prize
Then off I hop beneath the skies

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

@XxFran10xX
XxFran10xX merged commit 2efc4f0 into main Sep 27, 2026
2 checks passed
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