Skip to content

battle: attribute-defence shift rolls its own hit chance per attribute id - #1059

Merged
take-cheeze merged 1 commit into
masterfrom
claude/todos-zqtf9u
Aug 19, 2026
Merged

battle: attribute-defence shift rolls its own hit chance per attribute id#1059
take-cheeze merged 1 commit into
masterfrom
claude/todos-zqtf9u

Conversation

@take-cheeze

Copy link
Copy Markdown
Owner

Summary

  • Game::Battle#apply_attr_shift (an RPG2003 skill's attribute-defence rank shift, e.g. a "Ward Fire"-style resistance buff or a curse that weakens a resistance) applied the shift to every targeted attribute id unconditionally, with no accuracy roll at all.
  • RPG_RT rolls an independent Rand::PercentChance(to_hit_attribute_shift) per targeted attribute id inside Game_BattleAlgorithm::Skill::vExecute's affect_attr_defence loop (src/game_battlealgorithm.cpp, confirmed by fetching the live source), so a skill tagged to shift several attributes at once can land on some and miss others in the same cast — the same "rolled fresh per affected field" behavior this codebase's stat_mod_keys/HP/SP skill effects already have via #skill_effect_hits?.
  • Fixed by rolling #skill_effect_hits?(cmd) once per attribute id inside #apply_attr_shift's loop, reusing the same cmd[:chance] the skill's other effects already roll against.
  • While re-reading the surrounding C++ for context, separately verified that affect_attack/affect_defense/affect_spirit/affect_agility's own independent per-stat rolls are already correctly implemented by this codebase's existing stat_keys = (cmd[:stat_mod_keys] || []).select { skill_effect_hits?(cmd) } (each .select iteration re-invokes #skill_effect_hits? for a fresh, independent roll) — no change needed there.
  • Game_BattleAlgorithm::Skill::vExecute's separate IsImmuneToAttributeDownshifts() gate on negative shifts is a related, not-yet-investigated gap, deliberately left out of scope here.

Test plan

  • New scripts/rpg2k_logic_check.rb check: a two-attribute shift with a seeded RNG landing exactly one of two 50%-chance rolls — only the attribute whose own roll hit actually moves.
  • Confirmed the new check fails against the pre-fix code (expected [1], got [1, 2]) via git stash, and passes post-fix.
  • ruby scripts/rpg2k_logic_check.rb — 1035 checks passed.
  • ruby scripts/rpg2k_scene_check.rb — 751 checks passed.
  • ruby scripts/rpg2k_render_check.rb — 41 checks passed.
  • ruby scripts/rpg2k3_battle_gauge_check.rb — 15 checks, 0 failures.
  • ruby scripts/rpg2k3_battle_row_check.rb — 13 checks, 0 failures.

Generated by Claude Code

…e id

RPG_RT rolls an independent Rand::PercentChance(to_hit_attribute_shift)
per targeted attribute id inside Game_BattleAlgorithm::Skill::vExecute's
affect_attr_defence loop; apply_attr_shift applied the shift to every
listed id unconditionally with no roll at all.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PvVVCj619TThxYzb5CbavC
@take-cheeze
take-cheeze enabled auto-merge August 19, 2026 02:23
@github-actions github-actions Bot added auto-merge Auto-merge is enabled; treat as already review-approved (see AGENTS.md) engine:rpg2k RPG Maker 2000 games (LCF data, RPG_RT parity) component:events Event interpreter, event commands, move routes component:docs README, /docs, ADRs, changelog fragments component:tooling scripts/, test beds, parity harnesses, download helpers labels Aug 19, 2026
@take-cheeze
take-cheeze merged commit 5b2be06 into master Aug 19, 2026
12 checks passed
@take-cheeze
take-cheeze deleted the claude/todos-zqtf9u branch August 19, 2026 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge Auto-merge is enabled; treat as already review-approved (see AGENTS.md) component:docs README, /docs, ADRs, changelog fragments component:events Event interpreter, event commands, move routes component:tooling scripts/, test beds, parity harnesses, download helpers engine:rpg2k RPG Maker 2000 games (LCF data, RPG_RT parity)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants