fix enchanting

[AngeloTadeucci]

• Fix [General] Above +10 enchanting goes wrong in the revamped route. #610

Summary by CodeRabbit

• Refactor
  • Improved internal code organization and maintainability of enchantment handling logic.

[coderabbitai]

📝 Walkthrough

Walkthrough

Introduces a new local function ApplyEnchantStats within the Enchant method to consolidate duplicate logic that updates enchant statistics. This helper function is called at three locations where enchant upgrades succeed, eliminating code duplication while maintaining identical functional behavior.

Changes

Cohort / File(s)	Summary
Enchant Logic Refactoring Maple2.Server.Game/Manager/ItemEnchantManager.cs	Extracted duplicate enchant stat application logic into a new local helper function ApplyEnchantStats invoked at three call sites (successful Ophelia enchants and Peachy enchants upon reaching MaxExp). Consolidates BasicOptions merging and Enchants counter increment. Net change: +15/-16 lines.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• Fix enchanting² #498: Modifies related enchanting logic in ItemEnchantManager.cs, adjusting constants and rate/check logic that may interact with the refactored enchant stat application flow.

Suggested reviewers

• Zintixx

Poem

🐰 ✨ Three paths converged where stats took flight,
A rabbit refactored—consolidated right!
No duplicates left in the enchant parade,
One helper function makes the magic cascade! 🌟

🚥 Pre-merge checks | ✅ 1 | ❌ 4

❌ Failed checks (3 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR only refactors the ApplyEnchantStats logic without addressing the core issue: incorrect min/max attack values for revamp enchanting above +10 as described in issue #610.	Implement the actual fix for the revamp enchanting bug that causes incorrect min/max attack calculations, not just refactor existing code.
Out of Scope Changes check	⚠️ Warning	The changes are internal refactoring (extracting ApplyEnchantStats helper) that does not address the reported bug, making the refactoring potentially out of scope for a bug fix PR.	Focus on fixing the reported issue in #610 rather than performing unrelated internal refactoring in this PR.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fix enchanting' is vague and generic; it does not clearly describe the specific issue being fixed (incorrect min/max attack calculations in revamp enchanting above +10).	Use a more specific title like 'Fix revamp enchanting min/max attack calculation above +10' to clearly communicate the main change.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch enchanting

---

No actionable comments were generated in the recent review. 🎉

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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