Decouple the replacement zone scan from PERFORMANCE_MODE

[liamiak]

Rewritten since you last looked - +246/-11 across three files became +48/-12 across two, and the engine part is a net line deletion. What changed is at the bottom.

PERFORMANCE_MODE gates two unrelated things: the Spell.canPlay LKI re-control skip, which its settings text documents and warns about, and @Blackvipe99's Tap/Untap/ProduceMana zone skip from #11160, which it does not mention.

Only the first is a tradeoff. The second gives up nothing - zonesCheck already rejects those zones for every card that declares one, so the skip removes work whose result was thrown away. It ended up behind a flag whose description promises to disable correctness checks, and that is why it is off for almost everyone and never runs in CI.

So this isn't a new fast path. It takes the half of PERFORMANCE_MODE that was never lossy, makes it the default and unconditional behaviour, and leaves the flag holding only the branch its warning actually describes - one less thing hidden behind "this may break your game".

There is a second reason to want them apart, and it costs users today. The flag's other half is the live workaround for #8801 - casting cards owned by an opponent (Praetor's Grasp, Rev, Thief of Sanity, Decadent Dragon, and turn-control effects generally) breaks with Performance Mode on, and the standing advice is to turn it off. That advice currently also throws away this scan's speedup, because one flag gates both, which is what the thread was asking not to have to trade:

it would be nice if there were a more robust solution for it than telling people to deal with the tradeoff of turning off Performance Mode

After this, turning Performance Mode off to fix the card interaction keeps the tap/untap speedup. #8801 was closed stale rather than fixed, so that tradeoff is still real.

This makes the zone restriction unconditional and takes its zones from STATIC_ABILITIES_SOURCE_ZONES - the same thing @Blackvipe99 did for the fog scan in #11157, a few hundred lines down in this file. PERFORMANCE_MODE keeps its default and now governs only the Spell.canPlay branch, which is exactly what its description says.

if (ZONE_RESTRICTED_EVENTS.contains(event) && cardZone != null
        && !ZoneType.STATIC_ABILITIES_SOURCE_ZONES.contains(cardZone.getZoneType())) {
    return true;
}

It also delivers what #11160's comment promised - "in case a custom card wants one active from elsewhere". That skip returned before zonesCheck, so an ActiveZones$ declaration was ignored outright; taking the zones from the shared constant means a graveyard or exile declaration now works.

Why it isn't the general form

The obvious simplification is to drop the event set and restrict every replacement scan to STATIC_ABILITIES_SOURCE_ZONES, the way isPreventCombatDamageThisTurn already does further down this file. That breaks five cards. The constant deliberately excludes Hand (/*, Hand*/), and Loxodon Smiter, Obstinate Baloth, Nullhide Ferox, Wilt-Leaf Liege and Dodecapod all carry R:Event$ Moved | ActiveZones$ Hand. They are untouched here only because the restriction is scoped to Tap/Untap/ProduceMana. The fog scan can use the general form because DamageDone has no hand-active case; these three events are the set that does.

Measured

sim -d "Big 240531" "Big 240531" -n 3 -s 12345, same seed throughout, so every build plays the same games - all 16 runs below produced identical turn counts and outcomes. Four rounds with the four builds interleaved round-robin rather than batched: this box drifts by tens of percent between batches, and batching quietly credits that drift to whichever build ran last. In-game time, mean of four rounds:

	mean	vs master
master	127 482 ms	-
this PR	79 254 ms	1.61x
#11366 (trait cache) alone	53 513 ms	2.38x
both	50 134 ms	2.54x

Per-round ratios stay inside 1.59-1.63, 2.35-2.46 and 2.50-2.59.

I expected #11366 to subsume this, since the scan's cost is mostly the getReplacementEffects() rebuild that PR caches. It doesn't - caching makes each visit cheap, this stops ~1700 library and hand cards being visited at all - but the margin is smaller than this body used to claim. On top of #11366 it is worth 6.7% (per-round 5.5-8.0%), not the ~15% I had here before. That older number came from batched runs and I no longer trust it.

Safety

Every Tap and ProduceMana replacement in the pool declares ActiveZones$ - 121 Battlefield, 3 Command. The 49 declaring nothing are all Untap, all "doesn't untap during your untap step" on a permanent. Nothing needs a zone this skips.

The earlier version of this section understated what that means, so to be precise: zonesCheck treats a missing ActiveZones$ as active everywhere, not as battlefield-only -

return !this.hostCard.isPhasedOut()
        && (validHostZones == null || validHostZones.isEmpty()
        || (hostCardZone != null && validHostZones.contains(hostCardZone.getZoneType())));

so those 49 are formally live in a library too. They are unreachable in practice, because an untap event never targets a card there, but the skip narrows a default-everywhere rather than trimming a declared zone. That is the honest statement of the change.

Blast radius

Probed rather than argued: an undeclared Untap replacement hosted in each of the 19 ZoneTypes, master against this branch. Seven zones change, not the two this body used to imply - Hand and Library, plus the five command-adjacent deck zones SchemeDeck, PlanarDeck, AttractionDeck, ContraptionDeck and Junkyard, which are in PART_OF_COMMAND_ZONE but not in STATIC_ABILITIES_SOURCE_ZONES. Battlefield, Graveyard, Exile and Command are unchanged; Sideboard, Ante, Merged, Subgame and None were already unscanned. Flashback, Stack and ExtraHand the probe could not construct.

The five deck zones are unreachable in the current pool. The 49 undeclared effects sit on ordinary permanents - creatures, artifacts, auras, one land - so none of them can be in a scheme, planar, attraction or contraption deck. The three cards of those types that do carry one of these events (Edge of Malacol, Imprison This Insolent Wretch, Mirri) all declare ActiveZones$ Command, which already excluded their own deck zone. Hand and Library are the only zones where the narrowing is observable.

The one thing given up: the constant excludes Hand, so a custom card declaring ActiveZones$ Hand on these three events would stop working. Nothing in the pool does, and the fog check already has this property. If it matters, widening the shared constant serves all ~30 call sites rather than just this one.

Two tests. The first is a no-regression guard for the zone set, and I should be straight that it is only that: at the default flag setting every zone is scanned today, so it passes on master too and discriminates only with the flag on. The second covers the narrowing itself and does fail against master at the default setting - an undeclared replacement is active in every zone, so an untap scan reached one sitting in a library. 356 tests, 0 failures, checkstyle clean.

What changed since the first version

• A 165-line ReplacementScanZones class is gone. It parsed Event$/ActiveZones$ out of card script text at startup to derive the zone set - re-implementing the parser next door to the real one, to compute what is a constant. It also missed its own use case: Effect-hosted replacements (False Dawn, Ood Sphere) get their command zone from EffectEffect in code, not script, so a text scan can't see them.
• A middle version registered widenings at parseReplacement instead. Better, but still inventing a zone set STATIC_ABILITIES_SOURCE_ZONES already meant, and it benchmarked no faster - complexity for nothing.
• The offer to hide this behind a second, separately-named flag is withdrawn - a new flag to work around the first being mis-scoped isn't a fix.
• The original body said this supersedes AI: include ProduceMana in the performance-mode replacement zone skip #11327. That's backwards now: AI: include ProduceMana in the performance-mode replacement zone skip #11327 is merged and this builds on it.

🤖 Implemented with the assistance of Claude Code (Opus 5).

[tool4ever]

interesting idea, but at the same time pretty over-engineered
especially if it's only for something easily hardcoded
the right approach is just getting rid of the overhead instead of trying to detect it: #11058

[liamiak]

On #11058 being the right approach here — one profiling note, in case it's useful for sequencing. I profiled one cantHappenCheck(Tap) on a 2-player board of ~560 cards — so 560 iterations of the visitor loop — and split where the ~465µs goes:

• the traversal itself (forEachCardInGame walking the zones, empty body): ~4.5µs, ~1%
• c.getReplacementEffects() rebuilding each card's effect list inside the loop: ~370µs, ~79%

forEachCardInGame is already allocation-free by construction, and #11058 as drawn streams that traversal — so it targets the 1%. The 79% is the per-card getReplacementEffects() rebuild, which a stream iterates the same way; flatMap over the zones still calls it once per card. So streaming this entry point shouldn't move this path much on its own.

The thing that would actually retire the skip — for every event, not just these three — is making that per-card rebuild cheap: either caching the RE list per card the way keywords already are (getCachedKeywords/updateKeywordsCache), or a per-event replacement index like activeTriggers. Both are bigger than this and I don't think they're on a branch yet; the LKI/preList path in getReplacementList makes a naive per-card cache tricky (it computes effects against a hypothetical battlefield state, not the live card). So I don't think this contradicts the #11058 direction — just that this particular scan's cost is somewhere the traversal rework won't reach, that I can see.

[tool4ever]

Ok, thanks for checking
that means #11323 might not be needed and I need to take a look at #11314 too

[liamiak]

Rebased onto master - the file is untouched upstream since July, so it replayed clean. Re-verified the pool while I was in there: still 121 Battlefield + 3 Command declarations on these three events, and the 49 with no declaration are still all "doesn't untap during your untap step".

I've reframed the body around what I think is the actual point, which it was burying under the benchmark. Of the two things PERFORMANCE_MODE gates, only Spell.canPlay is a tradeoff. This scan never was one - zonesCheck already rejected those zones for anything that declares one, so the skip only ever removed work whose result was discarded. It sat behind a flag advertised as disabling correctness checks, which is why it is off for nearly everyone. This takes the half that was never lossy and makes it unconditional, so the flag is left holding only the branch its warning describes.

One argument for splitting the flag that I had missed and have also added: its other half is the live workaround for #8801, where casting cards owned by an opponent breaks with Performance Mode on. Telling people to turn it off currently costs them this scan too, since one flag gates both - and #8801 was closed stale rather than fixed, so that is still the situation. This doesn't touch Spell.canPlay, but it does mean the workaround stops carrying a speed penalty.

On over-engineered - you're right that the general form is the one to want, and I went to delete the event set and restrict every replacement scan to STATIC_ABILITIES_SOURCE_ZONES, the way the fog scan next door already does. It breaks five cards. The constant excludes Hand on purpose, and Loxodon Smiter, Obstinate Baloth, Nullhide Ferox, Wilt-Leaf Liege and Dodecapod are all Event$ Moved | ActiveZones$ Hand. DamageDone has no hand-active case, which is why the fog scan gets away with it; Tap/Untap/ProduceMana is the set that does. So the event list is the minimum that's correct rather than a flourish - but if you'd rather widen the shared constant to include Hand and go general, that serves all ~30 call sites and I'd happily write that instead.

I also had the safety section wrong and have fixed it. zonesCheck treats a missing ActiveZones$ as active everywhere, not battlefield-only, so those 49 are formally live in a library as well. Unreachable in practice - an untap event never targets a card there - but the change narrows a default, and the body now says so.

That also gave the branch a test worth having. The one that was here passes on master at the default flag setting, so it only ever guarded the zone set. The new one asserts the narrowing directly - an undeclared Untap replacement sitting in a library no longer reaches the scan - and fails against master's ReplacementHandler at the default setting.

I also went and probed the blast radius properly instead of asserting it, hosting an undeclared Untap replacement in each of the 19 ZoneTypes and diffing master against this branch. Seven zones change, not the two I had been describing: Hand and Library, plus SchemeDeck, PlanarDeck, AttractionDeck, ContraptionDeck and Junkyard - they are in PART_OF_COMMAND_ZONE but not in STATIC_ABILITIES_SOURCE_ZONES. Those five turn out to be unreachable: the 49 undeclared effects are all ordinary permanents, so none can sit in one of those decks, and the three scheme/plane/vanguard cards that do carry these events (Edge of Malacol, Imprison This Insolent Wretch, Mirri) all declare ActiveZones$ Command, which already excluded their deck zone. Harmless, but the body now lists all seven rather than leaving someone to find SchemeDeck themselves.

Re-measured against current master, four rounds with the builds interleaved rather than batched, identical games throughout:

	mean	vs master
master	127 482 ms	-
this PR	79 254 ms	1.61x
#11366 alone	53 513 ms	2.38x
both	50 134 ms	2.54x

Worth being straight that the increment is smaller than this body claimed. I had it at ~15% on top of #11366; interleaved it is 6.7%. The old figure came from batching all runs of one build together, and this box drifts enough between batches that the batching was doing the talking. #11366 is also a good deal stronger than I had credited it.

The real root cause is below both PRs, for what it's worth: TriggerHandler keeps an activeTriggers registry maintained by registerActiveTrigger/clearActiveTriggers, and ReplacementHandler has no equivalent, so every query walks every card to find effects that are usually absent. An index there would subsume this and most of #11366. I haven't built it and I'm not proposing to here - it's ~15 scattered maintenance points and every missed registration is a silent rules bug - but that's the thing this is working around.

Happy to close this if you'd rather have the index or the general form. Just don't want it sitting open if it isn't wanted.

[tool4ever]

• assuming user has no card where it functions from some exotic zone should not be the default
• at the same time widening the allowed zones reduces part of the speed improvements again
• finally the block is only meant as a bandaid, so not trying to complicate the preference text with it feels like an acceptable trade-off