feat(enchanting): arcane disenchanting and mystic essence extraction forge engine

[angelTomo9]

Summary

Implements an arcane equipment disenchanting and mystic essence extraction forge engine for OpenAO MMORPG.

Features

• Item rarity tier yield scaling (Common, Magic, Rare, Epic, Legendary)
• Enchanter skill proficiency scaling
• Critical essence extraction rolls with doubled yields
• Full unit test coverage under Vitest

[gitar-bot]

Code Review ✅ Approved 3 resolved / 3 findings

Implements the arcane equipment disenchanting and mystic essence extraction forge engine with rarity and proficiency scaling. Addressed issues regarding enchantmentPower not affecting yields, totalDustEquivalent ignoring extracted essences, and NaN skill producing NaN yields.

✅ 3 resolved

✅ Bug: enchantmentPower is accepted but never affects yields

📄 api/src/lib/enchantmentDisenchantForge.ts:20 📄 api/src/lib/enchantmentDisenchantForge.ts:48-62
DisenchantableItem.enchantmentPower is documented as +1 to +10 and passed in, but disenchantItem never uses it — yields depend only on rarity, skill, and crit. Either factor enchantmentPower into the yield/dust computation as the feature description implies, or remove the field to avoid implying an effect that doesn't exist.

✅ Quality: totalDustEquivalent ignores extracted essences

📄 api/src/lib/enchantmentDisenchantForge.ts:26 📄 api/src/lib/enchantmentDisenchantForge.ts:91
totalDustEquivalent is set to dustQty only, so the 'dust equivalent' of a legendary/epic result excludes the mystic essences/shards produced. If callers use this field to value a disenchant, higher-tier items are undervalued. Either include an essence-to-dust conversion in the total or rename the field to totalArcaneDust to reflect that it is dust-only.

✅ Edge Case: NaN skill produces NaN yields silently

📄 api/src/lib/enchantmentDisenchantForge.ts:59-69
If playerEnchantingSkill is NaN, Math.min(100, Math.max(1, NaN)) returns NaN, so skillFactor and dustQty become NaN while success stays true — producing an item yield of quantity NaN. Guard against non-finite skill (e.g. default/floor to 1 when Number.isNaN(playerEnchantingSkill)).

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

---

Important

Your trial ends in 5 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

_{Was this helpful? React with 👍 / 👎 | Gitar}