Skip to content

Fix knockout timing when discarding multiple Pokémon Tools#189

Closed
wunaidev wants to merge 3 commits intobcollazo:mainfrom
wunaidev:pr/fix-tool-discard-knockout-timing
Closed

Fix knockout timing when discarding multiple Pokémon Tools#189
wunaidev wants to merge 3 commits intobcollazo:mainfrom
wunaidev:pr/fix-tool-discard-knockout-timing

Conversation

@wunaidev
Copy link
Copy Markdown

Summary

Fix knockout timing for effects that discard multiple Pokémon Tools.

Problem

The engine was resolving knockouts too early during multi-Tool discard effects.

Before this change, discard_tool() immediately triggered knockout handling after each individual Tool was removed.

That meant an effect like Guzma could be resolved as:

  1. discard one Tool
  2. resolve KO / promotion
  3. discard the next Tool
  4. resolve KO / promotion again

That timing is incorrect.

When one discarded HP-boosting Tool knocks out the Active Pokémon, the engine can generate promotion choices too early.
If a later discarded Tool changes the Bench before that promotion resolves, those earlier promotion choices become
stale.

Why this matters

Without this change, the engine can:

  • generate promotion choices from an outdated board state
  • keep stale Activate(...) actions after the board has changed
  • enter illegal states where an Active slot becomes empty without the game ending
  • panic later during search / attack forecasting

So this is both a rules-correctness fix and a stability fix.

Rules basis

Knockouts should be checked after the full effect has finished resolving, not in the middle of it.

Relevant references:

So for effects that discard multiple Tools, the correct timing is:

  • discard all relevant Tools
  • then resolve knockouts once from the final board state

Changes

  • make discard_tool() only discard the Tool
  • move knockout handling to the enclosing effect boundary
  • resolve knockouts once after Guzma finishes discarding all opponent Tools

Tests

Added regression coverage for:

  • Guzma discarding multiple Giant Cape Tools in one effect
  • verifying that promotion choices are generated from the final post-discard board state

Validation

  • cargo fmt --all --check
  • cargo test -q --test tools_integration_test --test stadium_test
  • cargo test -q

Copy link
Copy Markdown
Owner

@bcollazo bcollazo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking of having attacks and effects not handle K.O.s (instead use handle_only type of functions), and do a full round of checks of K.O.s after each action is applied (handle_knockouts). What do you think?

Do you think something like that would work? That is, centralize handle_knockouts after applying any action... 🤔

Comment thread src/hooks/core.rs Outdated
Comment on lines +229 to +232
if !barrier_indices.is_empty() {
// Resolve knockouts only after all end-of-turn barrier discards have finished.
crate::actions::handle_knockouts(state, (tool_owner, 0), false);
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. I am not following here. Why might handling KO.s have to do with Metal Core Barrier? 🤔

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — Metal Core Barrier does not need a KO check here.

I updated that call site conservatively after changing the discard_tool() flow, but the actual bugged case is multi-
Tool discard with HP-modifying Tools like Giant Cape.

I'll drop the Metal Core Barrier part.

@wunaidev
Copy link
Copy Markdown
Author

I agree with the general direction.

My understanding is that the cleanest long-term model would be to treat KO handling as part of framework-level effect
resolution boundaries, rather than as a hidden side effect of low-level helpers or something each individual card
effect has to remember to do manually.

In other words:

  • low-level helpers should only perform state mutations
  • card/effect code should describe the effect steps
  • the enclosing resolver should do one KO / winner / promotion pass after the full effect finishes resolving

So the important boundary, to me, is "after a fully resolved effect", handled centrally by the resolver/framework.

@wunaidev
Copy link
Copy Markdown
Author

Removed the Metal Core Barrier KO check.

This PR now only does a single KO pass after all multi-tool discards finish (for example, Guzma).

@bcollazo
Copy link
Copy Markdown
Owner

bcollazo commented Mar 28, 2026

Hey! Although I'd like to centralize the handling of KOs elsewhere, this PR is still an improvement IMO. So if you could fix conflicts and CI, I think we can merge.

Love the test. 🙌

@bcollazo
Copy link
Copy Markdown
Owner

bcollazo commented Apr 1, 2026

Took these fixes and applied them in: #232. Thanks for the solution! 🙌

@bcollazo bcollazo closed this Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants