combat-trainer: don't leave casting set after a failed spell prep (fixes #7563)

[MahtraDR]

Summary

Fixes #7563.

SpellProcess#prepare_spell discarded the return value of DRCA.prepare? and set game_state.casting = true unconditionally. When preparation failed — most commonly an unknown spell where the game replies You have no idea how to cast that spell — execute() then returned early on if game_state.casting, so check_offensive and check_training never ran. The character silently stopped casting all offensive and training spells until check_timer cleared the state 70s later, at which point check_buffs re-picked the same dead spell and the cycle repeated indefinitely. No error or status message appeared.

A character reaches this state by copying a spell list, or by circling to 10 (Analogous Patterns spells are forgotten), or — as noted in the issue thread — by dying while a spell was only temporarily memorized from a scroll.

Changes (all in combat-trainer.lic)

• Honor prepare?'s return value. On failure, reset casting/cast_timer and return before setting casting. This unblocks casting for every failure reason (unknown spell, area interference, exhausted retries).
• Disable spells the character genuinely doesn't know. New ct-spell-unknown flag on You have no idea how to cast that spell; when it trips, the spell is marked ct_spell_disabled and the user is messaged. Only that permanent message disables — transient failures just skip the tick and retry.
• Skip ct_spell_disabled entries in check_buffs, check_training, and check_offensive, so the disable covers all three spell paths (the issue confirms training is affected too), rather than re-attempting and spamming the error every tick.

I deliberately did not use the DRSpells.known_spells / validate.lic route suggested in the issue: it misses magical tattoos and needs a SPELLS send to populate.

Tests

Regression specs added to spec/combat_trainer_spec.rb:

• #prepare_spell sets casting on success; leaves it unset with a nil timer on failure; disables an unknown spell without setting casting.
• #check_offensive skips a spell flagged ct_spell_disabled.

Verification:

• bundle exec rspec spec/combat_trainer_spec.rb — 395 examples, 0 failures
• bundle exec rspec (full suite) — 3021 examples, 0 failures
• bundle exec rubocop combat-trainer.lic — no offenses

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d562fb34-6538-47d0-811b-15f67b1afdb1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger 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.}

[simtel12]

Tested this locally against the case from #7563: an Empath with Ease Burden in
buff_spells, a spell that character never learned. Before the patch, no offensive spell was
ever attempted and nothing was printed. With the patch, the spell is named and disabled once,
and offensive casting proceeds normally. That fixes the problem I reported.

Two gaps I noticed while reading it over.

1. check_health_empath defeats the disable.

It builds a throwaway hash on every call, so ct_spell_disabled is written to an object that
is discarded immediately:

# combat-trainer.lic:2255, and again at :2269, :2271, :2276
data = { 'abbrev' => 'vh', 'mana' => @empath_spells['VH'].first, ... }
prepare_spell(data, game_state)

If an Empath's empath_healing names a spell they do not know, the Disabling ... message
repeats on every tick. The starvation is gone, because the unless prepared return still
does its job, so this is message spam rather than a functional break. The FOC and HEAL
branches have the same shape.

The other three paths are fine. check_offensive, check_buffs, and check_training all
hand prepare_spell an object that lives in the settings list, so the flag sticks there.

2. The #check_offensive spec passes for a weaker reason than it looks.

The spec stubs sort_by_rate_then_rank: [], so skill is nil, and
ready_spells.find { |spell| spell['skill'] == skill } returns nil. The method then exits
at return unless data no matter what the filter did. Removing the ct_spell_disabled line
does make the spec fail, but only because the strict double raises on the unstubbed
is_offense_allowed? once the filter gets that far, not because the assertion catches the
regression.

Stubbing sort_by_rate_then_rank to return ['Debilitation'] would exercise the skip
directly.

Neither of these blocks the fix as far as I am concerned.

[MahtraDR]

Thanks @simtel12 — both spot-on. Pushed 069342f.

1. Empath healing defeating the disable. Root cause was keying the disable off a flag written onto the spell hash, which the check_health_empath VH/FOC/HEAL branches rebuild every tick (as you noted, check_offensive/check_buffs/check_training pass the settings-list object, so those stuck). Switched to a persistent @disabled_spells set keyed by abbrev, so the disable survives the throwaway hashes. check_health_empath now skips a disabled vh/foc/heal (the FOC/HEAL branch falls through to HEAL naturally if FOC is the one disabled), and the Disabling … message prints exactly once via Set#add?. This also stops mutating the caller's hash.

2. Weak #check_offensive spec. Correct — it was passing at return unless data. Reworked it to stub sort_by_rate_then_rank to return the spell's skill plus is_offense_allowed?/dancing?, so with the guard removed the disabled spell resolves to data and prepare_spell actually fires. Mutation-checked both new guards: dropping either makes the spec fail with prepare_spell "received: 1 time", not on an unstubbed-double error. Added a #check_health_empath regression spec for the rebuilt-hash case too.

Full suite green (3022 examples, 0 failures), rubocop clean.

[MahtraDR]

Tested this locally against the case from #7563: an Empath with Ease Burden in buff_spells, a spell that character never learned. Before the patch, no offensive spell was ever attempted and nothing was printed. With the patch, the spell is named and disabled once, and offensive casting proceeds normally. That fixes the problem I reported.

Please try again one more time?