feat(alchemy): interactive cauldron brewing, heat control, and potion purity engine - #192
feat(alchemy): interactive cauldron brewing, heat control, and potion purity engine#192angelTomo9 wants to merge 2 commits into
Conversation
|
Freno acá antes de que sigas invirtiendo tiempo, porque el problema no es el código. El estado del repositorioOchenta y tres de dos personas. Y hay siete títulos que existen dos veces con autores Ninguno cita una issueRevisé los 26 que abriste hoy y ninguno referencia una issue del repositorio. Y las Las 29 issues abiertas son de otra cosa: el editor de mapas por etapas, soporte mobile, O sea que esto es trabajo real sobre cosas que nadie pidió, y por lo tanto no se puede Lo que dice el CONTRIBUTINGSe subió hoy a las 03:02, unas horas antes de tu primera PR de esta tanda, así que es
No es burocracia. Es exactamente lo que evita que dos personas escriban lo mismo, que es Qué te propongoElegí una issue abierta que te interese, comentala con qué archivo vas a tocar y cómo Y decime qué querés hacer con las que ya están abiertas. Si hay alguna que sí corresponde |
…expand test suite
|
|
||
| it("rejects brewing when player lacks required alchemy skill", () => { | ||
| const session = AlchemyBrewingCauldronEngine.startSession("elixir_of_strength")!; // Requires 60 | ||
| AlchemyBrewingCauldronEngine.addIngredient(session, "ogre_blood"); | ||
| AlchemyBrewingCauldronEngine.addIngredient(session, "mountain_sage"); | ||
| AlchemyBrewingCauldronEngine.addIngredient(session, "dragon_scale_dust"); | ||
|
|
||
| const result = AlchemyBrewingCauldronEngine.finishBrew(session, 30); // Skill only 30 | ||
| expect(result.success).toBe(false); | ||
| expect(result.purityPercent).toBe(10); | ||
| expect(result.reason).toContain("Insufficient Alchemy skill"); | ||
| }); | ||
|
|
||
| it("handles null session for unknown recipes and finishBrew invalid session", () => { | ||
| const nullSession = AlchemyBrewingCauldronEngine.startSession("non_existent_recipe"); |
There was a problem hiding this comment.
⚠️ Quality: Commit drops test coverage for heat/stir-direction penalties
The removed test ("downgrades purity if cauldron overheats and stirring direction is reversed") was the only case exercising the overheated/underheated badHeatTicks penalty (L163-164) and the wrong-requiredStirDirection -25 penalty (L152-154). Its replacements all use the correct stir direction and either optimal or zero heat, so those two penalty branches are now completely untested and regressions in the core purity math would pass CI. Add a test that overheats/underheats and reverses stir direction to assert the expected purity reduction.
Was this helpful? React with 👍 / 👎
| // Skill gives up to +10 bonus purity for master alchemists (skill 100) vs novice | ||
| const skillBonus = Math.floor((skill - recipe.minAlchemySkill) / 5); | ||
| let purity = 100 + skillBonus; |
There was a problem hiding this comment.
💡 Quality: Skill bonus is uncapped and comment overstates the max
The comment claims "up to +10 bonus purity for master alchemists (skill 100)", but Math.floor((skill - recipe.minAlchemySkill) / 5) yields +13 for greater_mana_potion (minSkill 35) and +8 for elixir_of_strength (minSkill 60) at skill 100, so the bonus is neither capped at 10 nor consistent across recipes. Either clamp the bonus (e.g. Math.min(10, ...)) or fix the comment to match the actual formula.
Was this helpful? React with 👍 / 👎
| // Heat penalty: If optimal heat was NEVER applied, heavily penalize cold brew (-50) | ||
| if (session.heatExposureTicks.optimal === 0) { | ||
| purity -= 50; |
There was a problem hiding this comment.
💡 Quality: finishBrew ignores recipe.targetHeat, hardcodes OPTIMAL check
The unheated-brew penalty checks heatExposureTicks.optimal === 0 regardless of the recipe's targetHeat field, which is otherwise never read. Both current recipes target OPTIMAL so there is no visible bug, but any future recipe with a non-OPTIMAL targetHeat would be scored against the wrong heat band. Drive the penalty off recipe.targetHeat to keep the field meaningful.
Was this helpful? React with 👍 / 👎
Code Review
|
| Auto-apply | Compact |
|
|
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
Summary
Implements an interactive alchemy cauldron brewing engine for OpenAO MMORPG.
Features