From 7f68e61d4c24eea7b7da2a3cf57e2aedb4fc44bf Mon Sep 17 00:00:00 2001 From: Autumn Wind <209156905+autumnmyst@users.noreply.github.com> Date: Tue, 19 May 2026 20:46:52 -0700 Subject: [PATCH 1/3] Yield Rework V2 follow-up fixes - 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. --- .../forge/gamemodes/match/HostedMatch.java | 2 + .../gamemodes/match/YieldController.java | 31 ++++++++++++- .../gui/control/FControlGameEventHandler.java | 10 ++++- .../forge/player/PlayerControllerHuman.java | 43 ++++++++++++++----- 4 files changed, 72 insertions(+), 14 deletions(-) diff --git a/forge-gui/src/main/java/forge/gamemodes/match/HostedMatch.java b/forge-gui/src/main/java/forge/gamemodes/match/HostedMatch.java index 64867031ac8..80478bb145b 100644 --- a/forge-gui/src/main/java/forge/gamemodes/match/HostedMatch.java +++ b/forge-gui/src/main/java/forge/gamemodes/match/HostedMatch.java @@ -228,6 +228,8 @@ public void startGame() { p.updateSleeve(); if (p.getController() instanceof PlayerControllerHuman humanController) { + // Per-tick yield flags survive clearAutoYields between games; wipe them so game N+1 starts clean. + humanController.getYieldController().resetForNewGame(); final IGuiGame gui = guis.get(p.getRegisteredPlayer()); humanController.setGui(gui); gui.setGameView(null); //clear out game view first so we don't copy into old game view diff --git a/forge-gui/src/main/java/forge/gamemodes/match/YieldController.java b/forge-gui/src/main/java/forge/gamemodes/match/YieldController.java index 5bc222645c7..522afbb14ef 100644 --- a/forge-gui/src/main/java/forge/gamemodes/match/YieldController.java +++ b/forge-gui/src/main/java/forge/gamemodes/match/YieldController.java @@ -5,6 +5,7 @@ import forge.game.card.CardView; import forge.game.combat.CombatView; import forge.game.phase.PhaseType; +import forge.game.player.Player; import forge.game.player.PlayerView; import forge.game.spellability.SpellAbilityStackInstance; import forge.game.spellability.StackItemView; @@ -137,6 +138,11 @@ public synchronized void setAutoPassUntilEndOfTurn(boolean active) { this.autoPassUntilEOT = active; } public synchronized void setMarker(PlayerView phaseOwner, PhaseType phase, boolean atOrPastAtClick) { + if (phaseOwner == null || phase == null) { + // Wire envelopes from an unhealthy client can land here; treat as a clear rather than store a junk marker. + clearMarker(); + return; + } autoPassUntilEOT = false; autoPassUntilStackEmpty = false; stackYieldRespectsInterrupts = false; @@ -260,6 +266,22 @@ public void clearAutoYields() { activeStore().onGameEnd(owner.getGame() == null || owner.getGame().getView().isMatchOver()); } + /** Reset per-game transient state between games of a match. {@link #clearAutoYields} drops + * stored yields but per-tick flags (lastSeenStackNonEmpty, wasAutoPassingLastTick, the + * yield-just-ended edge, autoPassInterrupted) would otherwise bleed into game 2 and + * mis-fire suggestions at its first prompt. */ + public synchronized void resetForNewGame() { + autoPassUntilEOT = false; + autoPassUntilStackEmpty = false; + stackYieldRespectsInterrupts = false; + clearMarker(); + declinedSuggestionTurn.clear(); + lastSeenStackNonEmpty = false; + wasAutoPassingLastTick = false; + yieldJustEndedFlag = false; + autoPassInterrupted = false; + } + public boolean getDisableAutoYields() { return activeStore().isDisabled(); } @@ -467,7 +489,9 @@ public void onSpellAbilityCast(SpellAbilityStackInstance si) { if (!shouldEvaluateInterrupts()) return; PlayerView local = owner.getLocalPlayerView(); if (local == null) return; - boolean isOpponent = !si.getActivatingPlayer().getView().equals(local); + // Triggered abilities and emblem-sourced spells can land here with a null activator. + Player activator = si.getActivatingPlayer(); + boolean isOpponent = activator != null && !activator.getView().equals(local); if (isOpponent && getBoolPref(FPref.YIELD_INTERRUPT_ON_OPPONENT_SPELL)) { applyInterrupt(); @@ -498,7 +522,10 @@ public void onAttackersDeclared(CombatView combat) { } } - private boolean shouldEvaluateInterrupts() { + /** True when an event-driven interrupt should be considered: either an interruptible + * yield is active, or APINA + RESPECTS_INTERRUPTS are both on (so the next priority + * window's auto-pass can be blocked by autoPassInterrupted). */ + public boolean shouldEvaluateInterrupts() { if (isInterruptibleYieldActive()) return true; return getBoolPref(FPref.YIELD_AUTO_PASS_NO_ACTIONS) && getBoolPref(FPref.YIELD_AUTO_PASS_RESPECTS_INTERRUPTS); } diff --git a/forge-gui/src/main/java/forge/gui/control/FControlGameEventHandler.java b/forge-gui/src/main/java/forge/gui/control/FControlGameEventHandler.java index 2dad935656a..cf9895eb2d5 100644 --- a/forge-gui/src/main/java/forge/gui/control/FControlGameEventHandler.java +++ b/forge-gui/src/main/java/forge/gui/control/FControlGameEventHandler.java @@ -282,11 +282,16 @@ public Void visit(final GameEventSpellAbilityCast event) { private void evaluateYieldInterruptForSpellCast(GameEventSpellAbilityCast event) { if (humanController == null) return; YieldController yc = humanController.getYieldController(); - if (!yc.isYieldActive()) return; + // isYieldActive() only covers explicit yields; APINA-with-respects-interrupts + // also wants to be told about casts so it can set autoPassInterrupted. + if (!yc.shouldEvaluateInterrupts()) return; GameView gv = matchController.getGameView(); if (gv == null || gv.getGame() == null) return; // Look up the actual SpellAbilityStackInstance by id (host-side; client gv.getGame() is null). + // event.si() can be null on reconnect-time event replays. + if (event.si() == null) return; int targetId = event.si().getId(); + if (targetId < 0) return; for (SpellAbilityStackInstance candidate : gv.getGame().getStack()) { if (candidate.getId() == targetId) { yc.onSpellAbilityCast(candidate); @@ -375,7 +380,8 @@ public Void visit(final GameEventBlockersDeclared event) { public Void visit(final GameEventAttackersDeclared event) { if (humanController != null) { YieldController yc = humanController.getYieldController(); - if (yc.isYieldActive()) { + // APINA-with-respects-interrupts wants the attackers signal too, not just explicit yields. + if (yc.shouldEvaluateInterrupts()) { GameView gv = matchController.getGameView(); if (gv != null && gv.getCombat() != null) yc.onAttackersDeclared(gv.getCombat()); } diff --git a/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java b/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java index a98e5525e23..8ad77b91a42 100644 --- a/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java +++ b/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java @@ -955,6 +955,11 @@ protected void reveal(final CardCollectionView cards, final ZoneType zone, final message += " " + localizer.getMessage("lblPlayerZone", "{player's}", zone.getTranslatedName().toLowerCase()); } final String fm = MessageUtil.formatMessage(message, getLocalPlayerView(), owner); + // While yielding with the reveal interrupt off, skip the modal the auto-pass would just + // plow through. Consistent with a normal reveal, nothing is logged. + if (shouldSuppressRevealModal()) { + return; + } if (cards.isEmpty()) { getGui().message(MessageUtil.formatMessage(localizer.getMessage("lblThereNoCardInPlayerZone", "{player's}", zone.getTranslatedName().toLowerCase()), getLocalPlayerView(), owner), fm); @@ -1784,19 +1789,37 @@ public void notifyOfValue(final SpellAbility sa, final GameObject realtedTarget, final String message = MessageUtil.formatNotificationMessage(sa, player, realtedTarget, value); if (sa != null && sa.isManaAbility()) { getGame().fireEvent(new GameEventAddLog(GameLogEntryType.LAND, message)); - } else if (sa != null && sa.getHostCard() != null && getGui().isLibgdxPort()) { - CardView cardView; - IPaperCard iPaperCard = sa.getHostCard().getPaperCard(); - if (iPaperCard != null) - cardView = CardView.getCardForUi(iPaperCard); - else - cardView = sa.getHostCard().getView(); - getGui().confirm(cardView, message, true, ImmutableList.of(localizer.getMessage("lblOK"))); - } else { - getGui().message(message, sa == null || sa.getHostCard() == null ? "" : CardView.get(sa.getHostCard()).toString()); + } else if (!shouldSuppressRevealModal()) { + // While yielding with the reveal interrupt off the modal is skipped entirely; like a + // normal reveal, nothing is logged. + if (sa != null && sa.getHostCard() != null && getGui().isLibgdxPort()) { + CardView cardView; + IPaperCard iPaperCard = sa.getHostCard().getPaperCard(); + if (iPaperCard != null) + cardView = CardView.getCardForUi(iPaperCard); + else + cardView = sa.getHostCard().getView(); + getGui().confirm(cardView, message, true, ImmutableList.of(localizer.getMessage("lblOK"))); + } else { + getGui().message(message, sa == null || sa.getHostCard() == null ? "" : CardView.get(sa.getHostCard()).toString()); + } } } + /** + * True when a reveal/notifyOfValue modal should be skipped while yielding: + * INTERRUPT_ON_REVEAL is off (if it were on, the reveal just interrupted, so + * the modal is meaningful) and {@link #mayAutoPass} confirms we'd auto-pass + * past it anyway (i.e. an explicit yield is active or APINA would fire — both + * of which already filter for autoPassInterrupted). Skipping is inherent to + * having the reveal interrupt off; there's no separate pref. Want the reveals? + * Turn on the reveal interrupt (with APINA-respects-interrupts). + */ + private boolean shouldSuppressRevealModal() { + if (yieldController.getBoolPref(FPref.YIELD_INTERRUPT_ON_REVEAL)) return false; + return mayAutoPass(); + } + /* * (non-Javadoc) * From c129dd04d5d5d2bf3f0a0d8dea01a76417e99e39 Mon Sep 17 00:00:00 2001 From: Autumn Wind <209156905+autumnmyst@users.noreply.github.com> Date: Fri, 22 May 2026 11:21:32 -0700 Subject: [PATCH 2/3] Remove uneccesary reset function and null check --- .../java/forge/gamemodes/match/HostedMatch.java | 2 -- .../forge/gamemodes/match/YieldController.java | 16 ---------------- .../gui/control/FControlGameEventHandler.java | 3 --- 3 files changed, 21 deletions(-) diff --git a/forge-gui/src/main/java/forge/gamemodes/match/HostedMatch.java b/forge-gui/src/main/java/forge/gamemodes/match/HostedMatch.java index 80478bb145b..64867031ac8 100644 --- a/forge-gui/src/main/java/forge/gamemodes/match/HostedMatch.java +++ b/forge-gui/src/main/java/forge/gamemodes/match/HostedMatch.java @@ -228,8 +228,6 @@ public void startGame() { p.updateSleeve(); if (p.getController() instanceof PlayerControllerHuman humanController) { - // Per-tick yield flags survive clearAutoYields between games; wipe them so game N+1 starts clean. - humanController.getYieldController().resetForNewGame(); final IGuiGame gui = guis.get(p.getRegisteredPlayer()); humanController.setGui(gui); gui.setGameView(null); //clear out game view first so we don't copy into old game view diff --git a/forge-gui/src/main/java/forge/gamemodes/match/YieldController.java b/forge-gui/src/main/java/forge/gamemodes/match/YieldController.java index 522afbb14ef..8a62eabc209 100644 --- a/forge-gui/src/main/java/forge/gamemodes/match/YieldController.java +++ b/forge-gui/src/main/java/forge/gamemodes/match/YieldController.java @@ -266,22 +266,6 @@ public void clearAutoYields() { activeStore().onGameEnd(owner.getGame() == null || owner.getGame().getView().isMatchOver()); } - /** Reset per-game transient state between games of a match. {@link #clearAutoYields} drops - * stored yields but per-tick flags (lastSeenStackNonEmpty, wasAutoPassingLastTick, the - * yield-just-ended edge, autoPassInterrupted) would otherwise bleed into game 2 and - * mis-fire suggestions at its first prompt. */ - public synchronized void resetForNewGame() { - autoPassUntilEOT = false; - autoPassUntilStackEmpty = false; - stackYieldRespectsInterrupts = false; - clearMarker(); - declinedSuggestionTurn.clear(); - lastSeenStackNonEmpty = false; - wasAutoPassingLastTick = false; - yieldJustEndedFlag = false; - autoPassInterrupted = false; - } - public boolean getDisableAutoYields() { return activeStore().isDisabled(); } diff --git a/forge-gui/src/main/java/forge/gui/control/FControlGameEventHandler.java b/forge-gui/src/main/java/forge/gui/control/FControlGameEventHandler.java index cf9895eb2d5..196bd1c99fd 100644 --- a/forge-gui/src/main/java/forge/gui/control/FControlGameEventHandler.java +++ b/forge-gui/src/main/java/forge/gui/control/FControlGameEventHandler.java @@ -288,10 +288,7 @@ private void evaluateYieldInterruptForSpellCast(GameEventSpellAbilityCast event) GameView gv = matchController.getGameView(); if (gv == null || gv.getGame() == null) return; // Look up the actual SpellAbilityStackInstance by id (host-side; client gv.getGame() is null). - // event.si() can be null on reconnect-time event replays. - if (event.si() == null) return; int targetId = event.si().getId(); - if (targetId < 0) return; for (SpellAbilityStackInstance candidate : gv.getGame().getStack()) { if (candidate.getId() == targetId) { yc.onSpellAbilityCast(candidate); From d55ff11cdf6d687fbaf7a00d86f0c4b36157f433 Mon Sep 17 00:00:00 2001 From: Autumn Wind <209156905+autumnmyst@users.noreply.github.com> Date: Sun, 31 May 2026 13:15:06 -0700 Subject: [PATCH 3/3] Add reveal logging and scope yield reveal-suppression to reveals only --- .../src/main/java/forge/game/GameAction.java | 14 ++++++++++++++ .../src/main/java/forge/game/GameLogEntryType.java | 1 + .../src/main/java/forge/game/GameLogVerbosity.java | 3 ++- forge-gui/res/languages/en-US.properties | 1 + .../java/forge/player/PlayerControllerHuman.java | 14 ++++++++------ 5 files changed, 26 insertions(+), 7 deletions(-) diff --git a/forge-game/src/main/java/forge/game/GameAction.java b/forge-game/src/main/java/forge/game/GameAction.java index 81dce2ff9ff..2dcc6ae0a44 100644 --- a/forge-game/src/main/java/forge/game/GameAction.java +++ b/forge-game/src/main/java/forge/game/GameAction.java @@ -2207,6 +2207,7 @@ public void revealTo(final CardCollectionView cards, final Iterable to, final ZoneType zone = cards.getFirst().getZone().getZoneType(); final Player owner = cards.getFirst().getOwner(); + logReveal(cards, zone, owner); for (final Player p : to) { p.getController().reveal(cards, zone, owner, messagePrefix, addSuffix); } @@ -2233,6 +2234,7 @@ public void reveal(CardCollectionView cards, ZoneType zt, Player cardOwner, bool reveal(cards, zt, cardOwner, dontRevealToOwner, messagePrefix, true); } public void reveal(CardCollectionView cards, ZoneType zt, Player cardOwner, boolean dontRevealToOwner, String messagePrefix, boolean msgAddSuffix) { + logReveal(cards, zt, cardOwner); for (Player p : game.getPlayers()) { if (dontRevealToOwner && cardOwner == p) { continue; @@ -2241,6 +2243,18 @@ public void reveal(CardCollectionView cards, ZoneType zt, Player cardOwner, bool } } + /** Add a single game-log line naming the revealed cards and the zone they came from. + * Fired once here (not per viewer) so a hand reveal is one entry, not one-per-card or + * one-per-opponent. Gated for display by the REVEAL verbosity category. */ + private void logReveal(final CardCollectionView cards, final ZoneType zone, final Player owner) { + if (cards.isEmpty()) { + return; + } + game.fireEvent(new GameEventAddLog(GameLogEntryType.REVEAL, + Localizer.getInstance().getMessage("lblRevealLogEntry", + Lang.joinHomogenous(cards), owner, zone.getTranslatedName().toLowerCase()))); + } + public void revealUnplayableByAI(String title, Map>> unplayableCards) { // Notify both players for (Player p : game.getPlayers()) { diff --git a/forge-game/src/main/java/forge/game/GameLogEntryType.java b/forge-game/src/main/java/forge/game/GameLogEntryType.java index cc5cd61e85c..03458b07b0f 100644 --- a/forge-game/src/main/java/forge/game/GameLogEntryType.java +++ b/forge-game/src/main/java/forge/game/GameLogEntryType.java @@ -8,6 +8,7 @@ public enum GameLogEntryType { ANTE("Ante"), DRAFT("Draft"), ZONE_CHANGE("Zone Change"), + REVEAL("Reveal"), PLAYER_CONTROL("Player Control"), DAMAGE("Damage"), LIFE("Life"), diff --git a/forge-game/src/main/java/forge/game/GameLogVerbosity.java b/forge-game/src/main/java/forge/game/GameLogVerbosity.java index 23d346ad180..60915c2e92a 100644 --- a/forge-game/src/main/java/forge/game/GameLogVerbosity.java +++ b/forge-game/src/main/java/forge/game/GameLogVerbosity.java @@ -12,7 +12,8 @@ public enum GameLogVerbosity { EnumSet.of(GameLogEntryType.GAME_OUTCOME, GameLogEntryType.MATCH_RESULTS, GameLogEntryType.TURN, GameLogEntryType.MULLIGAN, GameLogEntryType.ANTE, GameLogEntryType.DAMAGE, - GameLogEntryType.ZONE_CHANGE, GameLogEntryType.LAND, + GameLogEntryType.ZONE_CHANGE, GameLogEntryType.REVEAL, + GameLogEntryType.LAND, GameLogEntryType.DISCARD, GameLogEntryType.COMBAT, GameLogEntryType.STACK_ADD, GameLogEntryType.STACK_RESOLVE, GameLogEntryType.LIFE)), diff --git a/forge-gui/res/languages/en-US.properties b/forge-gui/res/languages/en-US.properties index cbf27b4d1a1..eb62f6dd3f9 100644 --- a/forge-gui/res/languages/en-US.properties +++ b/forge-gui/res/languages/en-US.properties @@ -2113,6 +2113,7 @@ lblGainControl=gain control. lblReturnToHand=return to hand. lbldiscard=discard. lblReveal=reveal +lblRevealLogEntry={0} revealed from {1}'s {2} lblTap=tap lblCurrentCard=Current Card lblSelectNSpecifyTypeCardsToAction=Select %d {0} card(s) to {1} diff --git a/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java b/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java index 8ad77b91a42..345105a8498 100644 --- a/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java +++ b/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java @@ -956,7 +956,8 @@ protected void reveal(final CardCollectionView cards, final ZoneType zone, final } final String fm = MessageUtil.formatMessage(message, getLocalPlayerView(), owner); // While yielding with the reveal interrupt off, skip the modal the auto-pass would just - // plow through. Consistent with a normal reveal, nothing is logged. + // plow through. The reveal is still recorded in the game log (GameAction logs it once, + // independent of the modal), so the information isn't lost. if (shouldSuppressRevealModal()) { return; } @@ -1789,9 +1790,9 @@ public void notifyOfValue(final SpellAbility sa, final GameObject realtedTarget, final String message = MessageUtil.formatNotificationMessage(sa, player, realtedTarget, value); if (sa != null && sa.isManaAbility()) { getGame().fireEvent(new GameEventAddLog(GameLogEntryType.LAND, message)); - } else if (!shouldSuppressRevealModal()) { - // While yielding with the reveal interrupt off the modal is skipped entirely; like a - // normal reveal, nothing is logged. + } else { + // notifyOfValue carries many non-reveal messages (coin flips, vote results, + // "Attack declaration invalid", ...), some important, so it is never suppressed. if (sa != null && sa.getHostCard() != null && getGui().isLibgdxPort()) { CardView cardView; IPaperCard iPaperCard = sa.getHostCard().getPaperCard(); @@ -1807,13 +1808,14 @@ public void notifyOfValue(final SpellAbility sa, final GameObject realtedTarget, } /** - * True when a reveal/notifyOfValue modal should be skipped while yielding: + * True when a card-reveal modal should be skipped while yielding: * INTERRUPT_ON_REVEAL is off (if it were on, the reveal just interrupted, so * the modal is meaningful) and {@link #mayAutoPass} confirms we'd auto-pass * past it anyway (i.e. an explicit yield is active or APINA would fire — both * of which already filter for autoPassInterrupted). Skipping is inherent to * having the reveal interrupt off; there's no separate pref. Want the reveals? - * Turn on the reveal interrupt (with APINA-respects-interrupts). + * Turn on the reveal interrupt (with APINA-respects-interrupts). Only gates the + * reveal modal — the reveal is still written to the game log either way. */ private boolean shouldSuppressRevealModal() { if (yieldController.getBoolPref(FPref.YIELD_INTERRUPT_ON_REVEAL)) return false;