Skip to content

Yield Rework V2 minor follow-up fixes and reveal-suppress pref#10724

Open
autumnmyst wants to merge 1 commit into
Card-Forge:masterfrom
autumnmyst:minor-yield-fixes
Open

Yield Rework V2 minor follow-up fixes and reveal-suppress pref#10724
autumnmyst wants to merge 1 commit into
Card-Forge:masterfrom
autumnmyst:minor-yield-fixes

Conversation

@autumnmyst
Copy link
Copy Markdown
Contributor

@autumnmyst autumnmyst commented May 20, 2026

  • Auto-pass-no-actions with "respects interrupts" now properly breaks on opponent casts and attacker declarations. The event handler gated on isYieldActive() (explicit EOT/stack/marker yields only), so APINA was never told about these events and kept passing; gate on the broader shouldEvaluateInterrupts() instead.
  • NPE guards: null phaseOwner/phase in setMarker (treated as a clear), null activator in onSpellAbilityCast, and null si()/negative targetId seen on reconnect-time event replays.
  • resetForNewGame() clears per-tick yield flags between games of a match so game 2 doesn't inherit game 1's auto-pass / suggestion state.
  • "Suppress reveals while yielding" skips the reveal()/notifyOfValue() modal when set, interrupt-on-reveal is off, and auto-pass is active.

@autumnmyst
Copy link
Copy Markdown
Contributor Author

I considered logging the reveals when suppressed, but that seemed like a gameplay change, as normally reveals are not logged. So I concluded no reveals should be logged, suppressed or otherwise. It's explicitly for speed, sacrificing that info.

@autumnmyst autumnmyst force-pushed the minor-yield-fixes branch from 03e110d to a1df03e Compare May 20, 2026 17:05
- Auto-pass-no-actions with "respects interrupts" now actually breaks on
  opponent casts and attacker declarations. The event handler gated on
  isYieldActive() (explicit EOT/stack/marker yields only), so APINA was
  never told about these events and kept passing; gate on the broader
  shouldEvaluateInterrupts() instead.
- NPE guards: null phaseOwner/phase in setMarker (treated as a clear),
  null activator in onSpellAbilityCast, and null si()/negative targetId
  seen on reconnect-time event replays.
- resetForNewGame() clears per-tick yield flags between games of a match
  so game 2 doesn't inherit game 1's auto-pass / suggestion state.
- Reveals are now skipped inherently while yielding when the reveal
  interrupt is off: reveal()/notifyOfValue() drop the modal that the
  auto-pass would just plow through (and, like a normal reveal, log
  nothing). Want the reveals? Turn on the reveal interrupt. No separate
  pref/checkbox.
@autumnmyst autumnmyst force-pushed the minor-yield-fixes branch from a1df03e to c4227f9 Compare May 20, 2026 17:08
@Jetz72
Copy link
Copy Markdown
Contributor

Jetz72 commented May 20, 2026

I considered logging the reveals when suppressed, but that seemed like a gameplay change, as normally reveals are not logged. So I concluded no reveals should be logged, suppressed or otherwise.

Is there a standing reason we wouldn't want them in the game log? Or is it just not included because nobody's bothered to include it thus far? My initial impression is that reveals would be a pretty sensible addition to the game log, whether or not the modal is skipped.

@Jetz72 Jetz72 requested a review from MostCromulent May 20, 2026 18:30
@autumnmyst
Copy link
Copy Markdown
Contributor Author

I considered logging the reveals when suppressed, but that seemed like a gameplay change, as normally reveals are not logged. So I concluded no reveals should be logged, suppressed or otherwise.

Is there a standing reason we wouldn't want them in the game log? Or is it just not included because nobody's bothered to include it thus far? My initial impression is that reveals would be a pretty sensible addition to the game log, whether or not the modal is skipped.

I agree with that. If that's a feature we're looking for it's a simple addition. I wasn't sure if remembering what was revealed manually was an important piece of gameplay, though personally I would rather log it.

@MostCromulent
Copy link
Copy Markdown
Contributor

MostCromulent commented May 20, 2026

Is there a standing reason we wouldn't want them in the game log? Or is it just not included because nobody's bothered to include it thus far? My initial impression is that reveals would be a pretty sensible addition to the game log, whether or not the modal is skipped.

I suspect its just nobody has bothered to add it, and agree it would be good QoL inclusion. Only question is whether it creates any rules issue since in paper cards are just revealed temporarily but log appears as permanent record / you lose the gameplay challenge of having to remember revealed cards?

Edit: apparently MTGO displays revealed cards in some sort of log, and Arena keeps revealed cards visible until hand state changes. So adding to the log should be safe.

If that's a feature we're looking for it's a simple addition.

If consensus is to add reveal logging then imo:

  1. It should produce a single log line with all of the cards + the zone they were revealed from, rather than flooding the log with 7 separate entries when a player's hand is revealed.
  2. Should be wired up to the "Medium" log verbosity preset and added as a new checkbox to the custom log verbosity dialog; see here for precedent Better game log UI: inline card images, improved verbosity presets #9889

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.

3 participants