diff --git a/client/src/adapter/types.ts b/client/src/adapter/types.ts index ab1ff095f8..2c4fc78719 100644 --- a/client/src/adapter/types.ts +++ b/client/src/adapter/types.ts @@ -322,7 +322,7 @@ export interface MeldSelection { // display-only badges + Confirm gating. `#[serde(tag = "kind")]` in the engine. export type CombatRequirement = | { kind: "MustAttack"; players: PlayerId[]; sources?: ObjectId[] } - | { kind: "MustBlock"; sources?: ObjectId[] } + | { kind: "MustBlock"; sources?: ObjectId[]; attackers?: ObjectId[] } | { kind: "CantAttack"; sources?: ObjectId[] } | { kind: "CantBlock"; sources?: ObjectId[] }; diff --git a/client/src/components/board/ActionButton.tsx b/client/src/components/board/ActionButton.tsx index 2edb945120..f7c763e475 100644 --- a/client/src/components/board/ActionButton.tsx +++ b/client/src/components/board/ActionButton.tsx @@ -9,10 +9,8 @@ import { usePhaseInfo } from "../../hooks/usePhaseInfo.ts"; import { useGameStore } from "../../stores/gameStore.ts"; import { DRAFT_BOT_AI_SEAT, useMultiplayerDraftStore } from "../../stores/multiplayerDraftStore.ts"; import { useMultiplayerStore } from "../../stores/multiplayerStore.ts"; -import { useUiStore } from "../../stores/uiStore.ts"; +import { blockerAssignmentPairs, useUiStore } from "../../stores/uiStore.ts"; import { buildAttacks, hasMultipleAttackTargets, getValidAttackTargets, getValidAttackTargetsByAttacker } from "../../utils/combat.ts"; -import { useBlockRequirements } from "../combat/useBlockRequirements.ts"; -import { useBlockerConstraints } from "../combat/useBlockerConstraints.ts"; import { gameButtonClass } from "../ui/buttonStyles.ts"; import { GameplayTooltip } from "../ui/GameplayTooltip.tsx"; import { AttackTargetPicker } from "../controls/AttackTargetPicker.tsx"; @@ -72,20 +70,10 @@ export function ActionButton() { const setCombatMode = useUiStore((s) => s.setCombatMode); const setCombatClickHandler = useUiStore((s) => s.setCombatClickHandler); - // Engine-declared per-attacker minimum-blocker requirements (menace / - // "blocked by N or more"). Used to block confirmation while any attacker is - // under-assigned, so the player gets a clear message instead of an engine - // rejection (CR 702.111b / CR 509.1b). - const { byAttacker: blockRequirements } = useBlockRequirements(); - const incompleteBlockCount = useMemo( - () => Array.from(blockRequirements.values()).filter((r) => r.status === "incomplete").length, - [blockRequirements], + const blockerPairs = useMemo( + () => blockerAssignmentPairs(blockerAssignments), + [blockerAssignments], ); - // CR 509.1c: engine-provided must-block gating. Attacker must-attack - // requirements are NOT gated client-side — the engine strictly validates the - // declaration (CR 508.1d) and rejects an illegal submission; the must-attack - // badges (see AttackRequirementBadges / useAttackRequirements) are display only. - const { unsatisfiedMustBlockCount } = useBlockerConstraints(); const canCompanionToHand = useGameStore((s) => s.legalActions.some((a) => a.type === "CompanionToHand"), @@ -149,26 +137,32 @@ export function ActionButton() { [waitingFor], ); + // A new declaration prompt invalidates a partially selected blocker from the + // prior prompt, even though both prompts share the same combat mode. + const blockerPrompt = waitingFor?.type === "DeclareBlockers" ? waitingFor : null; + useEffect(() => { + setPendingBlocker(null); + }, [blockerPrompt]); + // Blocker click handler const handleBlockerClick = useCallback( (objectId: ObjectId) => { - // Click an already-assigned blocker to unassign - if (blockerAssignments.has(objectId)) { - removeBlockerAssignment(objectId); + // Selecting a blocker never clears its other assignments: one blocker can + // be assigned to multiple attackers. A second click on an attacker toggles + // only that pair, using the engine-provided candidate list. + if (validBlockerIds.includes(objectId) && validBlockTargets[objectId]?.length > 0) { + setPendingBlocker((current) => current === objectId ? null : objectId); return; } - if (pendingBlocker === null) { - // First click: select a valid blocker (must have at least one valid target) - if (validBlockerIds.includes(objectId) && validBlockTargets[objectId]?.length > 0) { - setPendingBlocker(objectId); - } - } else { - // Second click: assign to an attacker (only if engine says this pair is valid) + if (pendingBlocker !== null) { const validTargetsForBlocker = validBlockTargets[pendingBlocker] ?? []; if (combatAttackerIds.includes(objectId) && validTargetsForBlocker.includes(objectId)) { - assignBlocker(pendingBlocker, objectId); - setPendingBlocker(null); + if (blockerAssignments.get(pendingBlocker)?.has(objectId)) { + removeBlockerAssignment(pendingBlocker, objectId); + } else { + assignBlocker(pendingBlocker, objectId); + } } } }, @@ -241,7 +235,7 @@ export function ActionButton() { function handleConfirmBlockers() { dispatchAction({ type: "DeclareBlockers", - data: { assignments: Array.from(blockerAssignments.entries()) }, + data: { assignments: blockerPairs }, }); } @@ -317,14 +311,14 @@ export function ActionButton() { {mode === "combat-blockers" && ( <> - {blockerAssignments.size > 0 ? ( + {blockerPairs.length > 0 ? ( <> 0 || unsatisfiedMustBlockCount > 0} + disabled={actionBlocked} onClick={handleConfirmBlockers} - className={gameButtonClass({ tone: "emerald", size: "md", disabled: actionBlocked || incompleteBlockCount > 0 || unsatisfiedMustBlockCount > 0, className: primaryButtonClass })} + className={gameButtonClass({ tone: "emerald", size: "md", disabled: actionBlocked, className: primaryButtonClass })} > - {t("actionButton.confirmBlockers", { count: blockerAssignments.size })} + {t("actionButton.confirmBlockers", { count: blockerPairs.length })} ) : ( 0} + disabled={actionBlocked} onClick={() => handleSkipConfirm("blockers")} - className={gameButtonClass({ tone: "slate", size: "md", disabled: actionBlocked || unsatisfiedMustBlockCount > 0, className: primaryButtonClass })} + className={gameButtonClass({ tone: "slate", size: "md", disabled: actionBlocked, className: primaryButtonClass })} > {skipArmed === "blockers" ? t("actionButton.blockWithNoneConfirm") @@ -350,16 +344,6 @@ export function ActionButton() { {t("actionButton.selectAttackerForBlocker")} )} - {pendingBlocker === null && incompleteBlockCount > 0 && ( - - {t("combat.blockIncomplete", { count: incompleteBlockCount })} - - )} - {pendingBlocker === null && incompleteBlockCount === 0 && unsatisfiedMustBlockCount > 0 && ( - - {t("combat.unsatisfiedMustBlock", { count: unsatisfiedMustBlockCount })} - - )} > )} diff --git a/client/src/components/board/BlockAssignmentLines.tsx b/client/src/components/board/BlockAssignmentLines.tsx index 1d2e791156..88bf82f268 100644 --- a/client/src/components/board/BlockAssignmentLines.tsx +++ b/client/src/components/board/BlockAssignmentLines.tsx @@ -2,7 +2,7 @@ import { useEffect, useMemo, useRef, useState } from "react"; import { createPortal } from "react-dom"; import { usePreferencesStore } from "../../stores/preferencesStore.ts"; -import { useUiStore } from "../../stores/uiStore.ts"; +import { blockerAssignmentPairs, useUiStore } from "../../stores/uiStore.ts"; import { useGameStore } from "../../stores/gameStore.ts"; import { usePlayerId } from "../../hooks/usePlayerId.ts"; import { useRafPositions } from "../../hooks/useRafPositions.ts"; @@ -10,7 +10,10 @@ import { arcPath } from "../../hooks/useAttackerArrowPositions.ts"; import { objectAnchorSelector } from "../../utils/objectAnchorSelector.ts"; import { getVisibleBoardPlayerIds, isOneOnOne } from "../../viewmodel/gameStateView.ts"; import type { ObjectId, PlayerId } from "../../adapter/types.ts"; -import { filterVisibleBlockerPairs } from "./blockAssignmentVisibility.ts"; +import { + filterVisibleBlockerPairs, + type BlockerAssignmentPair, +} from "./blockAssignmentVisibility.ts"; const BLOCK_COLOR = "rgba(56,189,248,0.95)"; const BLOCK_COLOR_HEAD = "rgba(56,189,248,0.9)"; @@ -89,10 +92,10 @@ export function BlockAssignmentLines() { {showCreatureArrows && - Array.from(positions.entries()).map(([blockerId, pos]) => { + Array.from(positions.entries()).map(([pairKey, pos]) => { const d = arcPath(pos.from, pos.to); return ( - + , + uiAssignments: ReadonlyMap>, engineAssignments: Record | null, -): Map { +): BlockerAssignmentPair[] { return useMemo(() => { - const merged = new Map(uiAssignments); + const merged = new Map(); + for (const pair of blockerAssignmentPairs(uiAssignments)) { + merged.set(pair.join(":"), pair); + } if (engineAssignments) { for (const [blockerId, attackerIds] of Object.entries(engineAssignments)) { - if (attackerIds.length > 0) { - merged.set(Number(blockerId), attackerIds[0]); + for (const attackerId of attackerIds) { + const pair: BlockerAssignmentPair = [Number(blockerId), attackerId]; + merged.set(pair.join(":"), pair); } } } - return merged; + return Array.from(merged.values()); }, [uiAssignments, engineAssignments]); } diff --git a/client/src/components/board/GroupedPermanent.tsx b/client/src/components/board/GroupedPermanent.tsx index 228f2cfabc..53aafc6e2f 100644 --- a/client/src/components/board/GroupedPermanent.tsx +++ b/client/src/components/board/GroupedPermanent.tsx @@ -146,7 +146,6 @@ export const GroupedPermanentDisplay = memo(function GroupedPermanentDisplay({ const validBlockerIds = new Set(waitingFor.data.valid_blocker_ids); const eligibleIds = group.ids.filter((id) => validBlockerIds.has(id) - && !blockerAssignments.has(id) && (waitingFor.data.valid_block_targets[id]?.length ?? 0) > 0, ); return eligibleIds.length > 0 ? { mode: "blockers", eligibleIds } : null; diff --git a/client/src/components/board/__tests__/ActionButton.test.tsx b/client/src/components/board/__tests__/ActionButton.test.tsx index 5625ac00b7..0af5c7ae1a 100644 --- a/client/src/components/board/__tests__/ActionButton.test.tsx +++ b/client/src/components/board/__tests__/ActionButton.test.tsx @@ -283,9 +283,7 @@ describe("ActionButton", () => { }); }); - it("gates Block with None on an unassigned must-block creature, independent of menace", () => { - // No block_requirements (menace) present, so incompleteBlockCount is 0 — the - // gate here is purely the engine-provided must-block constraint. + it("does not client-gate Block with None on an unassigned must-block creature", () => { const wf: WaitingFor = { type: "DeclareBlockers", data: { @@ -299,24 +297,50 @@ describe("ActionButton", () => { useUiStore.setState({ selectedAttackers: [], blockerAssignments: new Map() }); render(); - expect(screen.getByRole("button", { name: "Block with None" })).toBeDisabled(); + expect(screen.getByRole("button", { name: "Block with None" })).toBeEnabled(); }); - it("enables Confirm Blockers once the must-block creature is assigned", () => { + it("submits every selected blocker pair without client-side requirement gating", () => { const wf: WaitingFor = { type: "DeclareBlockers", data: { player: 0, valid_blocker_ids: [100], - valid_block_targets: { "100": [200] }, + valid_block_targets: { "100": [200, 201] }, blocker_constraints: { "100": { kind: "MustBlock" } }, }, }; useGameStore.setState({ gameState: createGameState(wf), waitingFor: wf, legalActions: [] }); - useUiStore.setState({ selectedAttackers: [], blockerAssignments: new Map([[100, 200]]) }); + useUiStore.setState({ + selectedAttackers: [], + blockerAssignments: new Map([[100, new Set([200, 201])]]), + }); render(); - expect(screen.getByRole("button", { name: "Confirm Blockers (1)" })).toBeEnabled(); + const confirm = screen.getByRole("button", { name: "Confirm Blockers (2)" }); + expect(confirm).toBeEnabled(); + fireEvent.click(confirm); + expect(vi.mocked(dispatchAction)).toHaveBeenCalledWith({ + type: "DeclareBlockers", + data: { assignments: [[100, 200], [100, 201]] }, + }); + }); + + it("clears a pending blocker when the engine supplies a new declaration prompt", () => { + render(); + + act(() => useUiStore.getState().combatClickHandler?.(100)); + expect(screen.getByText("Select the attacker this blocker should defend against")).toBeInTheDocument(); + + const nextPrompt = blockerPrompt(); + act(() => { + useGameStore.setState({ + gameState: createGameState(nextPrompt), + waitingFor: nextPrompt, + }); + }); + + expect(screen.queryByText("Select the attacker this blocker should defend against")).not.toBeInTheDocument(); }); it("shows blocker controls when turn decision controller differs from blocking player (issue #1199)", () => { diff --git a/client/src/components/board/__tests__/BlockAssignmentLines.test.ts b/client/src/components/board/__tests__/BlockAssignmentLines.test.ts index b9dfdf6780..d807e68b0e 100644 --- a/client/src/components/board/__tests__/BlockAssignmentLines.test.ts +++ b/client/src/components/board/__tests__/BlockAssignmentLines.test.ts @@ -4,38 +4,51 @@ import { filterVisibleBlockerPairs } from "../blockAssignmentVisibility.ts"; describe("filterVisibleBlockerPairs", () => { it("keeps blocker pairs controlled by visible players", () => { - const pairs = new Map([ + const pairs: [number, number][] = [ [10, 100], [20, 200], [30, 300], - ]); + ]; const objects = { 10: { controller: 1 }, 20: { controller: 2 }, 30: { controller: 3 }, }; - expect([...filterVisibleBlockerPairs(pairs, objects, new Set([0, 2])).entries()]).toEqual([ + expect(filterVisibleBlockerPairs(pairs, objects, new Set([0, 2]))).toEqual([ [20, 200], ]); }); it("keeps all live split-mode opponent pairs when every opponent is visible", () => { - const pairs = new Map([ + const pairs: [number, number][] = [ [10, 100], [20, 200], [30, 300], - ]); + ]; const objects = { 10: { controller: 1 }, 20: { controller: 2 }, 30: { controller: 3 }, }; - expect([...filterVisibleBlockerPairs(pairs, objects, new Set([0, 1, 2, 3])).entries()]).toEqual([ + expect(filterVisibleBlockerPairs(pairs, objects, new Set([0, 1, 2, 3]))).toEqual([ [10, 100], [20, 200], [30, 300], ]); }); + + it("retains every pair when one visible blocker blocks multiple attackers", () => { + const pairs: [number, number][] = [ + [10, 100], + [10, 200], + ]; + const objects = { 10: { controller: 2 } }; + + expect(filterVisibleBlockerPairs(pairs, objects, new Set([0, 2]))).toEqual([ + [10, 100], + [10, 200], + ]); + }); }); diff --git a/client/src/components/board/blockAssignmentVisibility.ts b/client/src/components/board/blockAssignmentVisibility.ts index 69b063337a..e8895a2761 100644 --- a/client/src/components/board/blockAssignmentVisibility.ts +++ b/client/src/components/board/blockAssignmentVisibility.ts @@ -1,15 +1,15 @@ import type { ObjectId, PlayerId } from "../../adapter/types.ts"; +export type BlockerAssignmentPair = [ObjectId, ObjectId]; + export function filterVisibleBlockerPairs( - pairs: Map, + pairs: readonly BlockerAssignmentPair[], objects: Record | null, visiblePlayerIds: ReadonlySet, -): Map { - if (!objects) return pairs; - return new Map( - Array.from(pairs.entries()).filter(([blockerId]) => { - const blockerController = objects[String(blockerId)]?.controller; - return blockerController == null || visiblePlayerIds.has(blockerController); - }), - ); +): BlockerAssignmentPair[] { + if (!objects) return [...pairs]; + return pairs.filter(([blockerId]) => { + const blockerController = objects[String(blockerId)]?.controller; + return blockerController == null || visiblePlayerIds.has(blockerController); + }); } diff --git a/client/src/components/combat/BlockRequirementBadges.tsx b/client/src/components/combat/BlockRequirementBadges.tsx index 78ce80313a..885353813c 100644 --- a/client/src/components/combat/BlockRequirementBadges.tsx +++ b/client/src/components/combat/BlockRequirementBadges.tsx @@ -4,7 +4,7 @@ import { useTranslation } from "react-i18next"; import type { ObjectId } from "../../adapter/types.ts"; import { useGameStore } from "../../stores/gameStore.ts"; -import { useBlockRequirements, type BlockRequirement } from "./useBlockRequirements.ts"; +import { useBlockRequirements } from "./useBlockRequirements.ts"; interface Anchor { x: number; @@ -59,22 +59,11 @@ function useAttackerAnchors(attackerIds: ObjectId[]): Map { return anchors; } -function badgeTone(status: BlockRequirement["status"]): string { - switch (status) { - case "satisfied": - return "border-emerald-300/60 bg-emerald-950/85 text-emerald-100"; - case "incomplete": - return "border-amber-300/70 bg-amber-950/90 text-amber-100 animate-pulse"; - case "pending": - return "border-rose-300/50 bg-rose-950/85 text-rose-100"; - } -} - /** * Floating "needs N blockers" badge over each attacker with a minimum-blocker * requirement (menace / "blocked by N or more"). Renders only while the local - * player is assigning blockers. Pure display of engine-provided requirements vs - * the player's in-progress assignments (see `useBlockRequirements`). + * player is assigning blockers. It displays engine-provided requirements only; + * the engine validates the submitted declaration. */ export function BlockRequirementBadges() { const { t } = useTranslation("game"); @@ -90,16 +79,8 @@ export function BlockRequirementBadges() { {Array.from(byAttacker.values()).map((req) => { const anchor = anchors.get(req.attackerId); if (!anchor) return null; - const label = - req.status === "satisfied" - ? t("combat.blockSatisfiedBadge", { required: req.required }) - : req.status === "incomplete" - ? t("combat.blockProgressBadge", { assigned: req.assigned, required: req.required }) - : t("combat.blockNeedsBadge", { required: req.required }); - const baseTitle = - req.status === "incomplete" - ? t("combat.blockIncompleteAttacker", { assigned: req.assigned, required: req.required }) - : t("combat.menaceRequirement", { count: req.required }); + const label = t("combat.blockNeedsBadge", { required: req.required }); + const baseTitle = t("combat.menaceRequirement", { count: req.required }); // Display-only source attribution: suppress the self-source (Menace's // carrier is the attacker itself → bare badge) and skip ids that no // longer resolve (departed-source guard), mirroring AttackRequirementBadges. @@ -118,7 +99,7 @@ export function BlockRequirementBadges() { > {/* Two-pronged "menace" glyph — a single attacker needing multiple blockers. */} diff --git a/client/src/components/combat/BlockerConstraintBadges.tsx b/client/src/components/combat/BlockerConstraintBadges.tsx index 81755c3075..732022e89e 100644 --- a/client/src/components/combat/BlockerConstraintBadges.tsx +++ b/client/src/components/combat/BlockerConstraintBadges.tsx @@ -59,8 +59,6 @@ function useObjectAnchors(objectIds: ObjectId[]): Map { function badgeTone(status: BlockerConstraint["status"]): string { switch (status) { - case "satisfied": - return "border-emerald-300/60 bg-emerald-950/85 text-emerald-100"; case "pending": return "border-rose-300/50 bg-rose-950/85 text-rose-100 animate-pulse"; case "info": @@ -89,11 +87,14 @@ export function BlockerConstraintBadges() { {Array.from(byObject.values()).map((req) => { const anchor = anchors.get(req.objectId); if (!anchor) return null; + const exactNames = req.attackers + .map((id) => objects?.[String(id)]?.name) + .filter((name): name is string => !!name); const label = req.kind === "CantBlock" ? t("combat.cantBlockBadge") - : req.status === "satisfied" - ? t("combat.mustBlockSatisfiedBadge") + : exactNames.length > 0 + ? t("combat.mustBlockExactBadge", { attacker: exactNames.join(", ") }) : t("combat.mustBlockBadge"); // Display-only source attribution: suppress a self-source (intrinsic // requirement shows a bare badge) and skip ids that no longer resolve diff --git a/client/src/components/combat/__tests__/combatRequirements.test.tsx b/client/src/components/combat/__tests__/combatRequirements.test.tsx index 7e9e9bfae8..4bf20377da 100644 --- a/client/src/components/combat/__tests__/combatRequirements.test.tsx +++ b/client/src/components/combat/__tests__/combatRequirements.test.tsx @@ -8,6 +8,7 @@ import { AttackRequirementBadges } from "../AttackRequirementBadges.tsx"; import { BlockerConstraintBadges } from "../BlockerConstraintBadges.tsx"; import { useAttackRequirements } from "../useAttackRequirements.ts"; import { useBlockerConstraints } from "../useBlockerConstraints.ts"; +import { useBlockRequirements } from "../useBlockRequirements.ts"; function setWaitingFor(waitingFor: WaitingFor | undefined) { useGameStore.setState({ waitingFor }); @@ -113,10 +114,10 @@ describe("useBlockerConstraints", () => { it("does not throw on an empty or undefined constraint map", () => { setWaitingFor(undefined); - expect(renderHook(() => useBlockerConstraints()).result.current.unsatisfiedMustBlockCount).toBe(0); + expect(renderHook(() => useBlockerConstraints()).result.current.byObject.size).toBe(0); }); - it("satisfies the must-block gate when the creature is assigned (1 -> 0)", () => { + it("keeps must-block as an engine-validated display state without exposing a gate", () => { setWaitingFor({ type: "DeclareBlockers", data: { @@ -129,13 +130,33 @@ describe("useBlockerConstraints", () => { useUiStore.setState({ blockerAssignments: new Map() }); let r = renderHook(() => useBlockerConstraints()); - expect(r.result.current.unsatisfiedMustBlockCount).toBe(1); expect(r.result.current.byObject.get(100)?.status).toBe("pending"); + expect("unsatisfiedMustBlockCount" in r.result.current).toBe(false); - useUiStore.setState({ blockerAssignments: new Map([[100, 200]]) }); + useUiStore.setState({ blockerAssignments: new Map([[100, new Set([200])]]) }); r = renderHook(() => useBlockerConstraints()); - expect(r.result.current.unsatisfiedMustBlockCount).toBe(0); - expect(r.result.current.byObject.get(100)?.status).toBe("satisfied"); + expect(r.result.current.byObject.get(100)?.status).toBe("pending"); + }); + + it("retains engine-provided exact must-block attackers without locally validating pairs", () => { + setWaitingFor({ + type: "DeclareBlockers", + data: { + player: 0, + valid_blocker_ids: [100], + valid_block_targets: { "100": [200, 201] }, + blocker_constraints: { "100": { kind: "MustBlock", attackers: [200] } }, + }, + }); + + useUiStore.setState({ blockerAssignments: new Map([[100, new Set([201])]]) }); + let r = renderHook(() => useBlockerConstraints()); + expect(r.result.current.byObject.get(100)?.status).toBe("pending"); + expect(r.result.current.byObject.get(100)?.attackers).toEqual([200]); + + useUiStore.setState({ blockerAssignments: new Map([[100, new Set([200, 201])]]) }); + r = renderHook(() => useBlockerConstraints()); + expect(r.result.current.byObject.get(100)?.status).toBe("pending"); }); it("passes engine-provided sources through unchanged (display-only)", () => { @@ -153,6 +174,30 @@ describe("useBlockerConstraints", () => { }); }); +describe("useBlockRequirements", () => { + beforeEach(() => { + useUiStore.setState({ selectedAttackers: [], blockerAssignments: new Map() }); + }); + afterEach(() => setWaitingFor(undefined)); + + it("does not infer a minimum-blocker requirement's satisfaction from local pairs", () => { + setWaitingFor({ + type: "DeclareBlockers", + data: { + player: 0, + valid_blocker_ids: [100], + valid_block_targets: { "100": [200] }, + block_requirements: { "200": { count: 2, sources: [300] } }, + }, + }); + + useUiStore.setState({ blockerAssignments: new Map([[100, new Set([200])]]) }); + const requirement = renderHook(() => useBlockRequirements()).result.current.byAttacker.get(200); + expect(requirement).toMatchObject({ attackerId: 200, required: 2, sources: [300] }); + expect("status" in (requirement ?? {})).toBe(false); + }); +}); + describe("AttackRequirementBadges source attribution (display-only)", () => { beforeEach(() => { useUiStore.setState({ selectedAttackers: [], blockerAssignments: new Map() }); diff --git a/client/src/components/combat/useBlockRequirements.ts b/client/src/components/combat/useBlockRequirements.ts index 846a93a88d..5e027bc99a 100644 --- a/client/src/components/combat/useBlockRequirements.ts +++ b/client/src/components/combat/useBlockRequirements.ts @@ -1,78 +1,48 @@ import { useMemo } from "react"; import { useGameStore } from "../../stores/gameStore.ts"; -import { useUiStore } from "../../stores/uiStore.ts"; import type { ObjectId } from "../../adapter/types.ts"; /** - * Status of an attacker's minimum-blocker requirement, derived from the - * engine-provided `block_requirements` (CR 702.111b Menace / CR 509.1b) and the - * player's in-progress assignments: - * - `pending` — not yet blocked (legal: "or not at all"). - * - `incomplete` — blocked by 1+ but fewer than required (illegal to confirm). - * - `satisfied` — blocked by at least the required count. + * Engine-provided minimum-blocker requirement (CR 702.111b Menace / CR 509.1b). + * The engine alone determines whether an in-progress declaration is legal. */ -export type BlockRequirementStatus = "pending" | "incomplete" | "satisfied"; - export interface BlockRequirement { attackerId: ObjectId; required: number; - assigned: number; - status: BlockRequirementStatus; /** CR 702.111b / CR 509.1b: permanents imposing the min-blocker floor. */ sources: ObjectId[]; } export interface BlockRequirements { byAttacker: Map; - /** Any attacker started but not finished — confirmation must be blocked. */ - hasIncomplete: boolean; } -const EMPTY: BlockRequirements = { byAttacker: new Map(), hasIncomplete: false }; +const EMPTY: BlockRequirements = { byAttacker: new Map() }; /** - * Compares the engine-declared per-attacker minimum-blocker requirements against - * the player's current assignments. The requirement values come entirely from - * the engine (`DeclareBlockers.block_requirements`); this only counts the user's - * own pending selections against them — no game-rules logic lives here. + * Returns the engine-declared per-attacker minimum-blocker requirements. */ export function useBlockRequirements(): BlockRequirements { const blockRequirements = useGameStore((s) => s.waitingFor?.type === "DeclareBlockers" ? s.waitingFor.data.block_requirements : undefined, ); - const blockerAssignments = useUiStore((s) => s.blockerAssignments); return useMemo(() => { if (!blockRequirements || Object.keys(blockRequirements).length === 0) { return EMPTY; } - // blockerAssignments maps blockerId -> attackerId; invert to count blockers - // assigned to each attacker. - const assignedPerAttacker = new Map(); - for (const attackerId of blockerAssignments.values()) { - assignedPerAttacker.set(attackerId, (assignedPerAttacker.get(attackerId) ?? 0) + 1); - } - const byAttacker = new Map(); - let hasIncomplete = false; for (const [attackerKey, requirement] of Object.entries(blockRequirements)) { const attackerId = Number(attackerKey); - const required = requirement.count; - const assigned = assignedPerAttacker.get(attackerId) ?? 0; - const status: BlockRequirementStatus = - assigned === 0 ? "pending" : assigned < required ? "incomplete" : "satisfied"; - if (status === "incomplete") hasIncomplete = true; byAttacker.set(attackerId, { attackerId, - required, - assigned, - status, + required: requirement.count, sources: requirement.sources ?? [], }); } - return { byAttacker, hasIncomplete }; - }, [blockRequirements, blockerAssignments]); + return { byAttacker }; + }, [blockRequirements]); } diff --git a/client/src/components/combat/useBlockerConstraints.ts b/client/src/components/combat/useBlockerConstraints.ts index db17dab5c7..9b66361aab 100644 --- a/client/src/components/combat/useBlockerConstraints.ts +++ b/client/src/components/combat/useBlockerConstraints.ts @@ -1,20 +1,16 @@ import { useMemo } from "react"; import { useGameStore } from "../../stores/gameStore.ts"; -import { useUiStore } from "../../stores/uiStore.ts"; import type { CombatRequirement, ObjectId } from "../../adapter/types.ts"; /** * Per-creature blocker-constraint status, derived from the engine-provided * `blocker_constraints` (CR 509.1c must-block / CR 509.1b can't-block) and the - * player's in-progress blocker assignments: - * - `pending` — a MustBlock creature not yet assigned to any attacker (illegal - * to confirm). The engine already excludes must-block creatures - * with zero legal targets, so no target check is needed here. - * - `satisfied` — a MustBlock creature currently assigned. + * engine-provided constraints: + * - `pending` — a MustBlock requirement the engine still needs to validate. * - `info` — a CantBlock creature (informational; can never be assigned). */ -export type BlockerConstraintStatus = "pending" | "satisfied" | "info"; +export type BlockerConstraintStatus = "pending" | "info"; export interface BlockerConstraint { objectId: ObjectId; @@ -22,50 +18,44 @@ export interface BlockerConstraint { status: BlockerConstraintStatus; /** Engine-provided objects imposing this constraint (CR 509.1b/c). */ sources: ObjectId[]; + /** Engine-provided exact attackers this creature is asked to block. */ + attackers: ObjectId[]; } export interface BlockerConstraints { byObject: Map; - /** MustBlock creatures not yet assigned — confirmation must be blocked. */ - unsatisfiedMustBlockCount: number; } -const EMPTY: BlockerConstraints = { byObject: new Map(), unsatisfiedMustBlockCount: 0 }; +const EMPTY: BlockerConstraints = { byObject: new Map() }; /** * Compares the engine-declared per-creature blocker constraints against the * player's current assignments. All constraint values come entirely from the - * engine (`DeclareBlockers.blocker_constraints`); this only counts the user's own - * in-progress assignments against them — no game-rules logic lives here. + * engine (`DeclareBlockers.blocker_constraints`). It deliberately does not infer + * that a local assignment satisfies a requirement: CR 509.1c validation belongs + * exclusively to the engine. */ export function useBlockerConstraints(): BlockerConstraints { const blockerConstraints = useGameStore((s) => s.waitingFor?.type === "DeclareBlockers" ? s.waitingFor.data.blocker_constraints : undefined, ); - const blockerAssignments = useUiStore((s) => s.blockerAssignments); - return useMemo(() => { if (!blockerConstraints || Object.keys(blockerConstraints).length === 0) { return EMPTY; } - // blockerAssignments maps blockerId -> attackerId; a must-block creature is - // satisfied once it appears as a key (assigned to some attacker). const byObject = new Map(); - let unsatisfiedMustBlockCount = 0; for (const [key, requirement] of Object.entries(blockerConstraints)) { const objectId = Number(key); if (requirement.kind === "MustBlock") { - const status: BlockerConstraintStatus = blockerAssignments.has(objectId) - ? "satisfied" - : "pending"; - if (status === "pending") unsatisfiedMustBlockCount += 1; + const requiredAttackers = requirement.attackers ?? []; byObject.set(objectId, { objectId, kind: requirement.kind, - status, + status: "pending", sources: requirement.sources ?? [], + attackers: requiredAttackers, }); } else if (requirement.kind === "CantBlock") { byObject.set(objectId, { @@ -73,10 +63,11 @@ export function useBlockerConstraints(): BlockerConstraints { kind: requirement.kind, status: "info", sources: requirement.sources ?? [], + attackers: [], }); } } - return { byObject, unsatisfiedMustBlockCount }; - }, [blockerConstraints, blockerAssignments]); + return { byObject }; + }, [blockerConstraints]); } diff --git a/client/src/hooks/useRafPositions.ts b/client/src/hooks/useRafPositions.ts index 05905510d2..97b84e025e 100644 --- a/client/src/hooks/useRafPositions.ts +++ b/client/src/hooks/useRafPositions.ts @@ -1,6 +1,6 @@ import { useEffect, useRef, useState } from "react"; -import type { ObjectId } from "../adapter/types.ts"; +import type { BlockerAssignmentPair } from "../components/board/blockAssignmentVisibility.ts"; import { objectAnchorSelector } from "../utils/objectAnchorSelector.ts"; export interface LinePosition { @@ -9,15 +9,16 @@ export interface LinePosition { length: number; } -/** RAF polling for element positions — stabilizes after 10 unchanged frames. - * Pairs map: key → value, where both are ObjectIds with `data-object-id` attributes. */ -export function useRafPositions(pairs: Map): Map { - const [positions, setPositions] = useState>(new Map()); +/** RAF polling for element positions — stabilizes after 10 unchanged frames. */ +export function useRafPositions( + pairs: readonly BlockerAssignmentPair[], +): Map { + const [positions, setPositions] = useState>(new Map()); const prevRectsRef = useRef>(new Map()); const stableCountRef = useRef(0); useEffect(() => { - if (pairs.size === 0) { + if (pairs.length === 0) { setPositions(new Map()); return; } @@ -58,7 +59,7 @@ export function useRafPositions(pairs: Map): Map(); + const next = new Map(); for (const [fromId, toId] of pairs) { const fromRect = currentRects.get(String(fromId)); const toRect = currentRects.get(String(toId)); @@ -73,7 +74,7 @@ export function useRafPositions(pairs: Map): Map { beforeEach(() => { @@ -15,6 +15,7 @@ describe("uiStore", () => { selectedCardIds: [], fullControl: false, autoPass: false, + blockerAssignments: new Map(), }); }); }); @@ -125,6 +126,22 @@ describe("uiStore", () => { expect(useUiStore.getState().autoPass).toBe(true); }); + it("retains multiple attackers for one blocker and removes only the chosen pair", () => { + act(() => { + useUiStore.getState().assignBlocker(10, 100); + useUiStore.getState().assignBlocker(10, 101); + useUiStore.getState().assignBlocker(10, 100); + }); + + expect(blockerAssignmentPairs(useUiStore.getState().blockerAssignments)).toEqual([ + [10, 100], + [10, 101], + ]); + + act(() => useUiStore.getState().removeBlockerAssignment(10, 100)); + expect(blockerAssignmentPairs(useUiStore.getState().blockerAssignments)).toEqual([[10, 101]]); + }); + it("toggleDebugClickModeButtonVisible flips the pinned click-mode control", () => { expect(useUiStore.getState().debugClickModeButtonVisible).toBe(false); act(() => useUiStore.getState().toggleDebugClickModeButtonVisible()); diff --git a/client/src/stores/uiStore.ts b/client/src/stores/uiStore.ts index 1b1704534f..87b25fdfb0 100644 --- a/client/src/stores/uiStore.ts +++ b/client/src/stores/uiStore.ts @@ -9,6 +9,21 @@ import { DICE_ROLL_DURATION_MS, TURN_BANNER_DURATION_MS } from "../animation/typ import { usePreferencesStore } from "./preferencesStore"; import type { FilterKey } from "../components/modal/cardChoice/gridSelection"; +/** + * Ephemeral, player-selected blocker pairs. A creature can block more than one + * attacker, so the value is a set rather than a single attacker id. + */ +export type BlockerAssignments = Map>; + +/** Flatten the UI's per-blocker representation at the engine action boundary. */ +export function blockerAssignmentPairs( + assignments: ReadonlyMap>, +): [ObjectId, ObjectId][] { + return Array.from(assignments, ([blockerId, attackerIds]) => + Array.from(attackerIds, (attackerId): [ObjectId, ObjectId] => [blockerId, attackerId]), + ).flat(); +} + /** * A dice-roll / coin-flip moment to animate, surfaced from engine-authored * `DieRolled` / `CoinFlipped` events. `context` is supplied by the delivering @@ -160,7 +175,7 @@ interface UiStoreState { /** CR 702.22c: attacking bands declared this combat (each inner array is one * band of attacker ids). Empty when no bands are declared. */ attackerBands: ObjectId[][]; - blockerAssignments: Map; + blockerAssignments: BlockerAssignments; combatClickHandler: ((id: ObjectId) => void) | null; previewSticky: boolean; isDragging: boolean; @@ -262,7 +277,7 @@ interface UiStoreActions { selectAllAttackers: (ids: ObjectId[]) => void; setAttackerBands: (bands: ObjectId[][]) => void; assignBlocker: (blockerId: ObjectId, attackerId: ObjectId) => void; - removeBlockerAssignment: (blockerId: ObjectId) => void; + removeBlockerAssignment: (blockerId: ObjectId, attackerId?: ObjectId) => void; clearCombatSelection: () => void; setCombatClickHandler: (handler: ((id: ObjectId) => void) | null) => void; setPreviewSticky: (sticky: boolean) => void; @@ -548,14 +563,26 @@ export const useUiStore = create()((set, get) => ({ assignBlocker: (blockerId, attackerId) => set((state) => { const next = new Map(state.blockerAssignments); - next.set(blockerId, attackerId); + const attackerIds = new Set(next.get(blockerId)); + attackerIds.add(attackerId); + next.set(blockerId, attackerIds); return { blockerAssignments: next }; }), - removeBlockerAssignment: (blockerId) => + removeBlockerAssignment: (blockerId, attackerId) => set((state) => { const next = new Map(state.blockerAssignments); - next.delete(blockerId); + if (attackerId === undefined) { + next.delete(blockerId); + } else { + const attackerIds = new Set(next.get(blockerId)); + attackerIds.delete(attackerId); + if (attackerIds.size === 0) { + next.delete(blockerId); + } else { + next.set(blockerId, attackerIds); + } + } return { blockerAssignments: next }; }), diff --git a/client/src/viewmodel/__tests__/blockerSorting.test.ts b/client/src/viewmodel/__tests__/blockerSorting.test.ts index 56c42b9ce8..21ce62ba17 100644 --- a/client/src/viewmodel/__tests__/blockerSorting.test.ts +++ b/client/src/viewmodel/__tests__/blockerSorting.test.ts @@ -10,9 +10,9 @@ describe("sortCreaturesForBlockers", () => { it("orders blockers by their assigned attacker's opponent column", () => { const players = [g(1), g(2)]; const opponents = [g(102), g(101)]; // atk 102 -> col 0, atk 101 -> col 1 - const assignments = new Map([ - [1, 101], // col 1 - [2, 102], // col 0 + const assignments = new Map>([ + [1, new Set([101])], // col 1 + [2, new Set([102])], // col 0 ]); const sorted = sortCreaturesForBlockers(players, opponents, assignments); expect(sorted.map((p) => p.ids[0])).toEqual([2, 1]); @@ -25,10 +25,10 @@ describe("sortCreaturesForBlockers", () => { // defined. The result must preserve the input order deterministically. const players = [g(1), g(2), g(3)]; const opponents: GroupedPermanent[] = []; // no attacker columns at all - const assignments = new Map([ - [1, 901], - [2, 902], - [3, 903], + const assignments = new Map>([ + [1, new Set([901])], + [2, new Set([902])], + [3, new Set([903])], ]); const sorted = sortCreaturesForBlockers(players, opponents, assignments); expect(sorted.map((p) => p.ids[0])).toEqual([1, 2, 3]); @@ -37,11 +37,24 @@ describe("sortCreaturesForBlockers", () => { it("sorts off-row (no-column) blockers after those with a real column", () => { const players = [g(10), g(11)]; const opponents = [g(500)]; // col 0 - const assignments = new Map([ - [10, 999], // Infinity (not on the opponent row) - [11, 500], // col 0 + const assignments = new Map>([ + [10, new Set([999])], // Infinity (not on the opponent row) + [11, new Set([500])], // col 0 ]); const sorted = sortCreaturesForBlockers(players, opponents, assignments); expect(sorted.map((p) => p.ids[0])).toEqual([11, 10]); }); + + it("uses the earliest attacker column when one blocker blocks multiple attackers", () => { + const players = [g(10), g(11)]; + const opponents = [g(500), g(501)]; + const assignments = new Map>([ + [10, new Set([501, 500])], + [11, new Set([501])], + ]); + + const sorted = sortCreaturesForBlockers(players, opponents, assignments); + + expect(sorted.map((p) => p.ids[0])).toEqual([10, 11]); + }); }); diff --git a/client/src/viewmodel/blockerSorting.ts b/client/src/viewmodel/blockerSorting.ts index ced91cee0e..3ce54bd249 100644 --- a/client/src/viewmodel/blockerSorting.ts +++ b/client/src/viewmodel/blockerSorting.ts @@ -12,7 +12,7 @@ import type { GroupedPermanent } from "./battlefieldProps"; export function sortCreaturesForBlockers( playerCreatures: GroupedPermanent[], opponentCreatures: GroupedPermanent[], - blockerAssignments: Map, + blockerAssignments: ReadonlyMap>, ): GroupedPermanent[] { if (blockerAssignments.size === 0) return playerCreatures; @@ -57,13 +57,13 @@ export function sortCreaturesForBlockers( /** Get the minimum attacker column for any blocker in this group. */ function getMinAttackerColumn( group: GroupedPermanent, - blockerAssignments: Map, + blockerAssignments: ReadonlyMap>, attackerColumn: Map, ): number { let min = Infinity; for (const id of group.ids) { - const attackerId = blockerAssignments.get(id); - if (attackerId !== undefined) { + const attackerIds = blockerAssignments.get(id); + for (const attackerId of attackerIds ?? []) { const col = attackerColumn.get(attackerId) ?? Infinity; if (col < min) min = col; } diff --git a/crates/engine/src/ai_support/candidates.rs b/crates/engine/src/ai_support/candidates.rs index 07233e584b..4fbdb6ff43 100644 --- a/crates/engine/src/ai_support/candidates.rs +++ b/crates/engine/src/ai_support/candidates.rs @@ -917,7 +917,7 @@ pub fn candidate_actions_broad_with_probe( valid_blocker_ids, valid_block_targets, .. - } => blocker_actions(*player, valid_blocker_ids, valid_block_targets), + } => blocker_actions(state, *player, valid_blocker_ids, valid_block_targets), WaitingFor::UntapChoice { player, candidates, .. } => candidates @@ -4288,6 +4288,7 @@ fn attacker_actions( } fn blocker_actions( + state: &GameState, player: PlayerId, valid_blocker_ids: &[crate::types::identifiers::ObjectId], valid_block_targets: &std::collections::HashMap< @@ -4295,29 +4296,29 @@ fn blocker_actions( Vec, >, ) -> Vec { - let mut actions = vec![candidate( - GameAction::DeclareBlockers { - assignments: Vec::new(), - }, - TacticalClass::Block, - Some(player), - )]; + let mut proposals = vec![Vec::new()]; for &blocker_id in valid_blocker_ids { if let Some(targets) = valid_block_targets.get(&blocker_id) { for &attacker_id in targets { - actions.push(candidate( - GameAction::DeclareBlockers { - assignments: vec![(blocker_id, attacker_id)], - }, - TacticalClass::Block, - Some(player), - )); + proposals.push(vec![(blocker_id, attacker_id)]); } } } - actions + let mut seen = HashSet::new(); + crate::game::combat::complete_blocker_proposals(state, player, &proposals) + .into_iter() + .filter_map(|action| { + let GameAction::DeclareBlockers { assignments } = &action else { + return None; + }; + let mut key = assignments.clone(); + key.sort_unstable(); + seen.insert(key) + .then(|| candidate(action, TacticalClass::Block, Some(player))) + }) + .collect() } fn select_cards_variants( diff --git a/crates/engine/src/database/synthesis.rs b/crates/engine/src/database/synthesis.rs index 56dbb5173c..b8a2ca5791 100644 --- a/crates/engine/src/database/synthesis.rs +++ b/crates/engine/src/database/synthesis.rs @@ -5039,6 +5039,7 @@ fn is_provoke_attack_trigger(t: &TriggerDefinition) -> bool { execute.sub_ability.as_deref().map(|a| &*a.effect), Some(Effect::ForceBlock { target: TargetFilter::ParentTarget, + .. }) ) } @@ -5167,9 +5168,14 @@ fn build_provoke_trigger() -> TriggerDefinition { AbilityKind::Spell, Effect::ForceBlock { target: TargetFilter::ParentTarget, + attacker: Some(crate::types::ability::ForceBlockAttackerRef::Source), + duration: Duration::UntilEndOfCombat, }, ) - .description("CR 509.1c: that creature blocks this creature this turn if able".to_string()); + .description( + "CR 702.39a + CR 509.1c: that creature blocks this creature this combat if able" + .to_string(), + ); // CR 702.39a + CR 701.26b: "you may have target creature ... untap" — the // optional parent body untaps the chosen defender, then force-blocks it. @@ -5184,7 +5190,7 @@ fn build_provoke_trigger() -> TriggerDefinition { .optional() .sub_ability(force_block) .description( - "Provoke — untap target creature defending player controls; it blocks this turn if able" + "Provoke — untap target creature defending player controls; it blocks this combat if able" .to_string(), ); @@ -13575,6 +13581,8 @@ mod provoke_synthesis_tests { &*sub.effect, Effect::ForceBlock { target: TargetFilter::ParentTarget, + attacker: Some(crate::types::ability::ForceBlockAttackerRef::Source), + duration: Duration::UntilEndOfCombat, } ), "sub-ability must force-block the parent (untapped) target via ParentTarget, got {:?}", @@ -13823,7 +13831,7 @@ mod provoke_runtime_tests { use crate::types::ability::{ContinuousModification, EffectKind, TargetRef}; use crate::types::events::GameEvent; use crate::types::game_state::GameState; - use crate::types::identifiers::{CardId, ObjectId}; + use crate::types::identifiers::{CardId, ObjectId, ObjectIncarnationRef}; use crate::types::player::PlayerId; use crate::types::statics::StaticMode; @@ -13874,6 +13882,11 @@ mod provoke_runtime_tests { vec![TargetRef::Object(defender)], ); resolved.optional = false; + // Match the shared stack boundary: source-referential force-blocks + // bind the exact attacking incarnation before their continuation runs. + resolved.bind_force_block_source_recursive(Some(ObjectIncarnationRef::from_object( + &state.objects[&provoker], + ))); let mut events = Vec::new(); resolve_ability_chain(&mut state, &resolved, &mut events, 0).unwrap(); @@ -13892,7 +13905,7 @@ mod provoke_runtime_tests { m, ContinuousModification::AddStaticMode { mode: StaticMode::MustBlockAttacker { attacker }, - } if *attacker == provoker + } if attacker.object_id == provoker ) }) }); @@ -13901,6 +13914,13 @@ mod provoke_runtime_tests { "Provoke must apply MustBlockAttacker bound to the provoking attacker, \ reusing the existing source-referential ForceBlock resolver" ); + assert!( + state + .transient_continuous_effects + .iter() + .any(|effect| effect.duration == Duration::UntilEndOfCombat), + "CR 702.39a's forced block lasts only for this combat" + ); assert!(events.iter().any(|e| matches!( e, diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index 88715b9a29..836cb9ecc3 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -1813,6 +1813,7 @@ fn legacy_trigger_condition(x: &TriggerCondition) -> bool { | TriggerCondition::CastSpellThisTurn { .. } | TriggerCondition::SpellCastWithVariantThisTurn { .. } | TriggerCondition::SourceEnteredThisTurn + | TriggerCondition::SourceAttackedThisCombat | TriggerCondition::SourceIsHarnessed | TriggerCondition::SourceIsAttacking | TriggerCondition::SourceIsTransformed @@ -2830,7 +2831,6 @@ fn legacy_effect(x: &Effect) -> bool { | Effect::Unsuspect { target, .. } | Effect::PhaseOut { target } | Effect::PhaseIn { target } - | Effect::ForceBlock { target } | Effect::BecomePrepared { target } | Effect::BecomeUnprepared { target } | Effect::BecomeSaddled { target } @@ -2861,6 +2861,10 @@ fn legacy_effect(x: &Effect) -> bool { | Effect::DiscardCard { target, .. } | Effect::Animate { target, .. } => legacy_target_filter(target), + Effect::ForceBlock { + target, duration, .. + } => legacy_target_filter(target) || legacy_duration(duration), + Effect::GainActivatedAbilitiesOfTarget { target, recipient, @@ -3670,6 +3674,7 @@ fn walk_ability( source_incarnation: _, // self-transform epoch latch, no read/write effect trigger_source: _, // exact triggered-source authority, no read/write effect trigger_definition_ref: _, // exact trigger occurrence, no read/write effect + force_block_attacker: _, // exact force-block referent, no read/write effect controller: _, original_controller: _, scoped_player: _, @@ -5348,6 +5353,14 @@ fn rw_effect( p.merge(rw_duration(duration)); (p, None) } + Effect::ForceBlock { + target, duration, .. + } => { + let mut p = ext_write(StateKind::Other); + flag_legacy_write_target(&mut p, target); + p.merge(rw_duration(duration)); + (p, None) + } // §L14 (CR 500.8): an additional phase/step is a turn-structure write. Effect::AdditionalPhase { target: _, @@ -5500,7 +5513,6 @@ fn rw_effect( | Effect::RevealFromHand { .. } | Effect::ChooseDamageSource { .. } | Effect::PhaseIn { .. } - | Effect::ForceBlock { .. } | Effect::BecomeUnprepared { .. } | Effect::BecomeSaddled { .. } | Effect::SetClassLevel { .. } @@ -6088,6 +6100,7 @@ fn rw_trigger_condition(x: &TriggerCondition) -> RwProfile { } TriggerCondition::DuringPlayersTurn { player } => rw_player_filter(player), TriggerCondition::SourceEnteredThisTurn + | TriggerCondition::SourceAttackedThisCombat | TriggerCondition::SourceIsHarnessed | TriggerCondition::SourceIsAttacking | TriggerCondition::SourceIsTransformed diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index 3ff5bb6ffa..058f648328 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -231,6 +231,7 @@ fn resolved_ability_axes(a: &ResolvedAbility, mode: ScanMode) -> Axes { source_incarnation: _, // self-transform epoch latch, no dynamic read trigger_source: _, // exact triggered-source authority, no dynamic read trigger_definition_ref: _, // exact trigger occurrence, no dynamic read + force_block_attacker: _, // exact force-block referent, no dynamic read controller: _, // player id original_controller: _, // player id scoped_player: _, // player id (iteration binding) @@ -1142,9 +1143,14 @@ fn scan_effect(x: &Effect, mode: ScanMode) -> Axes { acc = acc.or(scan_target_filter(target, target_ctx, mode)); acc } - Effect::ForceBlock { target } => { + Effect::ForceBlock { + target, + attacker: _, + duration, + } => { let mut acc = Axes::NONE; acc = acc.or(scan_target_filter(target, target_ctx, mode)); + acc = acc.or(scan_duration(duration, mode)); acc } Effect::ForceAttack { @@ -3086,11 +3092,13 @@ fn scan_trigger_condition(x: &TriggerCondition, mode: ScanMode) -> Axes { acc = acc.or(scan_player_filter(player, mode)); acc } - TriggerCondition::SourceEnteredThisTurn => Axes { - event: false, - sibling: false, - projected: true, - }, + TriggerCondition::SourceEnteredThisTurn | TriggerCondition::SourceAttackedThisCombat => { + Axes { + event: false, + sibling: false, + projected: true, + } + } TriggerCondition::EchoDue => Axes::NONE, TriggerCondition::MinCoAttackers { filter, minimum: _ } => { let mut acc = Axes::NONE; @@ -6431,6 +6439,7 @@ pub(crate) fn ability_resolution_choice_freedom(a: &ResolvedAbility) -> Resoluti source_incarnation: _, // self-transform epoch latch, no resolution-time choice trigger_source: _, // exact triggered-source authority, no choice trigger_definition_ref: _, // exact trigger occurrence, no choice + force_block_attacker: _, // exact force-block referent, no choice controller: _, // player id original_controller: _, // player id scoped_player: _, // player id (iteration binding) diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index feb2e61daf..e39f5f9783 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -4863,7 +4863,7 @@ fn concretize_granting_object_in_effect(effect: &mut Effect, granter: ObjectId) | Effect::Connive { target, .. } | Effect::PhaseOut { target } | Effect::PhaseIn { target } - | Effect::ForceBlock { target } + | Effect::ForceBlock { target, .. } | Effect::ForceAttack { target, .. } | Effect::CastCopyOfCard { target, .. } | Effect::CopyTokenOf { target, .. } diff --git a/crates/engine/src/game/combat.rs b/crates/engine/src/game/combat.rs index 2efe0c85b7..2bf0237174 100644 --- a/crates/engine/src/game/combat.rs +++ b/crates/engine/src/game/combat.rs @@ -10,7 +10,7 @@ use crate::types::ability::{StaticDefinition, TargetFilter, TargetRef}; use crate::types::card_type::{CoreType, Supertype}; use crate::types::events::GameEvent; use crate::types::game_state::GameState; -use crate::types::identifiers::ObjectId; +use crate::types::identifiers::{ObjectId, ObjectIncarnationRef}; use crate::types::keywords::Keyword; use crate::types::mana::ManaColor; use crate::types::player::PlayerId; @@ -132,6 +132,12 @@ pub enum CombatRequirement { MustBlock { #[serde(default, skip_serializing_if = "Vec::is_empty")] sources: Vec, + /// Exact attackers named by "block that Wolf/this creature if able" + /// requirements. Empty retains the legacy generic-only wire shape. + /// Display-only: the CR 509.1c maximum is enforced by + /// BlockDeclarationConstraints, never reconstructed by a client. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + attackers: Vec, }, /// CR 508.1c: this creature can't attack (informational — the UI greys it). /// `sources` = the restriction carriers (Pacifism, Angelic Arbiter). @@ -215,6 +221,11 @@ pub struct CombatState { /// `attacked_defenders_this_combat`. #[serde(default)] pub creature_attacked_defenders_this_combat: HashMap>, + /// CR 400.7 + CR 508.1: exact current-combat attack ledger for source + /// intervening-if conditions. The raw-id defender map remains a display + /// history; this identity ledger must not match a re-entered object. + #[serde(default)] + pub attacking_incarnations_this_combat: HashSet, pub damage_assignments: HashMap>, pub first_strike_done: bool, /// CR 510.4: Combatants that had first strike or double strike as the first @@ -240,6 +251,7 @@ impl PartialEq for CombatState { && self.attacked_defenders_this_combat == other.attacked_defenders_this_combat && self.creature_attacked_defenders_this_combat == other.creature_attacked_defenders_this_combat + && self.attacking_incarnations_this_combat == other.attacking_incarnations_this_combat && self.first_strike_done == other.first_strike_done && self.first_strike_participants == other.first_strike_participants } @@ -1312,11 +1324,454 @@ fn creature_has_must_block_requirement( }) } +/// One independently scored CR 509.1c block requirement. Keeping these as a +/// multiset is essential: one declared pair can satisfy several requirements, +/// while incompatible requirements must be maximized together rather than +/// greedily enforced one at a time. +#[derive(Clone)] +enum BlockDeclarationRequirement { + Generic { + blocker: ObjectId, + }, + Exact { + blocker: ObjectId, + attacker: ObjectId, + }, + Attacker { + attacker: ObjectId, + by: Option, + source: ObjectId, + anchor: Option, + }, + Every { + blocker: ObjectId, + attacker: ObjectId, + }, +} + +/// The single live model for CR 509.1 blocker declarations. This mirrors +/// AttackDeclarationConstraints: hard legality is owned by +/// validate_blockers_core, and this model owns only the requirement multiset and +/// the exact candidate-pair universe used to find the tax-free maximum. +struct BlockDeclarationConstraints { + player: PlayerId, + /// Legal per-blocker declarations, including the empty choice. Enumerating + /// this product respects each blocker's capacity before search rather than + /// exploring every raw pair subset and rejecting almost all of them at a + /// leaf. + choices: Vec>>, + requirements: Vec, + /// Memoized feasibility frontier: for every remaining blocker-choice index + /// and requirement, whether any later local choice can satisfy it. This + /// avoids rescanning the Cartesian product at every search node. + future_requirement_satisfaction: Vec>, +} + +impl BlockDeclarationConstraints { + fn build(state: &GameState, player: PlayerId) -> Self { + let valid = get_valid_block_targets_for_player(state, player); + let mut pairs: Vec<(ObjectId, ObjectId)> = valid + .iter() + .flat_map(|(&blocker, attackers)| { + attackers.iter().map(move |&attacker| (blocker, attacker)) + }) + .collect(); + pairs.sort_unstable(); + pairs.dedup(); + + let mut requirements = Vec::new(); + let blocker_restriction = collect_blocker_restriction_statics(state); + let block_restriction = collect_block_restriction_statics(state); + let blocker_allowed = collect_blocker_allowed_statics(state); + let shadow = static_kind_present(state, StaticModeKind::CanBlockShadow); + let has_must_block = static_kind_present(state, StaticModeKind::MustBlock); + + for &blocker in valid.keys() { + if creature_has_must_block_requirement( + state, + blocker, + player, + has_must_block, + &blocker_restriction, + &block_restriction, + &blocker_allowed, + shadow, + ) { + requirements.push(BlockDeclarationRequirement::Generic { blocker }); + } + let Some(object) = state.objects.get(&blocker) else { + continue; + }; + for def in super::functioning_abilities::active_static_definitions(state, object) { + let StaticMode::MustBlockAttacker { attacker } = def.mode else { + continue; + }; + let live = state.combat.as_ref().is_some_and(|combat| { + combat.attackers.iter().any(|info| { + info.object_id == attacker.object_id + && info.defending_player == player + && state + .objects + .get(&attacker.object_id) + .is_some_and(|o| ObjectIncarnationRef::from_object(o) == attacker) + }) + }); + if live && pairs.contains(&(blocker, attacker.object_id)) { + requirements.push(BlockDeclarationRequirement::Exact { + blocker, + attacker: attacker.object_id, + }); + } + } + } + + let must_be_blocked = collect_must_be_blocked_statics(state); + if let Some(combat) = &state.combat { + for info in combat + .attackers + .iter() + .filter(|info| info.defending_player == player) + { + let attacker = info.object_id; + for (by, source, anchor) in + must_be_blocked_requirements_for_attacker(state, attacker, &must_be_blocked) + { + if pairs.iter().any(|(blocker, aid)| { + *aid == attacker + && by.is_none_or(|filter| { + matches_target_filter( + state, + *blocker, + filter, + &blocker_filter_context(state, source, anchor), + ) + }) + }) { + requirements.push(BlockDeclarationRequirement::Attacker { + attacker, + by: by.cloned(), + source, + anchor, + }); + } + } + for (filter, source, anchor) in must_be_blocked_by_all_requirements_for_attacker( + state, + attacker, + &must_be_blocked, + ) { + for &(blocker, aid) in &pairs { + if aid == attacker + && filter.is_none_or(|f| { + matches_target_filter( + state, + blocker, + f, + &blocker_filter_context(state, source, anchor), + ) + }) + { + requirements + .push(BlockDeclarationRequirement::Every { blocker, attacker }); + } + } + } + } + } + // Keep the entire legal pair universe. A pair that does not directly + // score a requirement can still be coupled to one that does through a + // multi-blocker capacity, menace floor, or another CR 509.1b legality + // restriction. Pruning it before the complete-declaration legality + // check can therefore hide the only maximum legal witness. + let mut by_blocker: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + for (blocker, attacker) in pairs { + by_blocker.entry(blocker).or_default().push(attacker); + } + let choices = by_blocker + .into_iter() + .filter_map(|(blocker, mut attackers)| { + attackers.sort_unstable(); + attackers.dedup(); + let object = state.objects.get(&blocker)?; + let maximum = extra_block_limit(state, object).min(attackers.len() as u32) as usize; + Some(blocker_assignment_choices(blocker, &attackers, maximum)) + }) + .collect::>(); + + let mut future_requirement_satisfaction = + vec![vec![false; requirements.len()]; choices.len() + 1]; + for choice_index in (0..choices.len()).rev() { + let mut remaining = future_requirement_satisfaction[choice_index + 1].clone(); + for (requirement_index, requirement) in requirements.iter().enumerate() { + remaining[requirement_index] |= choices[choice_index] + .iter() + .flatten() + .any(|pair| requirement_is_satisfied(state, requirement, &[*pair])); + } + future_requirement_satisfaction[choice_index] = remaining; + } + + Self { + player, + choices, + requirements, + future_requirement_satisfaction, + } + } + + fn score_with_state(&self, state: &GameState, assignments: &[(ObjectId, ObjectId)]) -> u32 { + self.requirements + .iter() + .filter(|requirement| self.requirement_is_satisfied(state, requirement, assignments)) + .count() as u32 + } + + fn requirement_is_satisfied( + &self, + state: &GameState, + requirement: &BlockDeclarationRequirement, + assignments: &[(ObjectId, ObjectId)], + ) -> bool { + requirement_is_satisfied(state, requirement, assignments) + } + + /// Memoize the optimistic score for the exact requirement state at this + /// blocker-choice frontier. The cached value intentionally ignores hard + /// legality: it is only an upper bound, so a wider bound can cost work but + /// can never prune a legal CR 509.1c witness. + fn partial_score_upper_bound( + &self, + state: &GameState, + choice_index: usize, + assignments: &[(ObjectId, ObjectId)], + upper_bound_memo: &mut HashMap<(usize, Vec), u32>, + ) -> u32 { + let satisfied: Vec = self + .requirements + .iter() + .map(|requirement| self.requirement_is_satisfied(state, requirement, assignments)) + .collect(); + let key = (choice_index, satisfied.clone()); + if let Some(&upper_bound) = upper_bound_memo.get(&key) { + return upper_bound; + } + let upper_bound = satisfied + .iter() + .enumerate() + .filter(|(requirement_index, is_satisfied)| { + **is_satisfied + || self.future_requirement_satisfaction[choice_index][*requirement_index] + }) + .count() as u32; + upper_bound_memo.insert(key, upper_bound); + upper_bound + } + + fn max_free_score(&self, state: &GameState) -> u32 { + self.best_free_declaration(state).1 + } + + fn best_free_declaration(&self, state: &GameState) -> (Vec<(ObjectId, ObjectId)>, u32) { + if self.requirements.is_empty() { + return (Vec::new(), 0); + } + let mut best = (Vec::new(), 0); + let mut chosen = Vec::new(); + let mut upper_bound_memo = HashMap::new(); + self.search_free(state, 0, &mut chosen, &mut best, &mut upper_bound_memo); + best + } + + fn search_free( + &self, + state: &GameState, + index: usize, + chosen: &mut Vec<(ObjectId, ObjectId)>, + best: &mut (Vec<(ObjectId, ObjectId)>, u32), + upper_bound_memo: &mut HashMap<(usize, Vec), u32>, + ) { + let upper_bound = self.partial_score_upper_bound(state, index, chosen, upper_bound_memo); + // A final declaration can only grow from this prefix. When the best + // possible score merely ties the incumbent, a longer prefix cannot + // improve the shortest-witness tie break; an equal-length prefix can + // improve it only if it is lexicographically earlier. This preserves + // the existing score → length → lex ordering exactly while cutting the + // wide-board equal-score Cartesian tail. + if upper_bound < best.1 + || (upper_bound == best.1 + && (chosen.len() > best.0.len() + || (chosen.len() == best.0.len() && *chosen >= best.0))) + { + return; + } + if index == self.choices.len() { + if validate_blockers_core(state, self.player, chosen).is_ok() + && compute_block_tax(state, chosen).is_none() + { + let score = self.score_with_state(state, chosen); + let better = score > best.1 + || (score == best.1 + && (chosen.len() < best.0.len() + || (chosen.len() == best.0.len() && *chosen < best.0))); + if better { + *best = (chosen.clone(), score); + } + } + return; + } + for choice in &self.choices[index] { + let start = chosen.len(); + chosen.extend(choice.iter().copied()); + self.search_free(state, index + 1, chosen, best, upper_bound_memo); + chosen.truncate(start); + } + } +} + +/// Evaluates one CR 509.1c requirement against a complete or partial +/// declaration. The solver uses the same predicate for scoring and its +/// memoized remaining-choice feasibility frontier. +fn requirement_is_satisfied( + state: &GameState, + requirement: &BlockDeclarationRequirement, + assignments: &[(ObjectId, ObjectId)], +) -> bool { + match requirement { + BlockDeclarationRequirement::Generic { blocker } => { + assignments.iter().any(|(b, _)| b == blocker) + } + BlockDeclarationRequirement::Exact { blocker, attacker } + | BlockDeclarationRequirement::Every { blocker, attacker } => { + assignments.contains(&(*blocker, *attacker)) + } + BlockDeclarationRequirement::Attacker { + attacker, + by, + source, + anchor, + } => assignments.iter().any(|(blocker, aid)| { + *aid == *attacker + && by.as_ref().is_none_or(|filter| { + matches_target_filter( + state, + *blocker, + filter, + &blocker_filter_context(state, *source, *anchor), + ) + }) + }), + } +} + +/// Enumerate a single blocker's capacity-bounded attacker subsets in stable +/// order. The empty declaration is always legal at this local layer; global +/// restrictions and CR 509.1c requirements are applied by the shared solver. +fn blocker_assignment_choices( + blocker: ObjectId, + attackers: &[ObjectId], + maximum: usize, +) -> Vec> { + fn collect( + blocker: ObjectId, + attackers: &[ObjectId], + maximum: usize, + start: usize, + current: &mut Vec<(ObjectId, ObjectId)>, + output: &mut Vec>, + ) { + output.push(current.clone()); + if current.len() == maximum { + return; + } + for index in start..attackers.len() { + current.push((blocker, attackers[index])); + collect(blocker, attackers, maximum, index + 1, current, output); + current.pop(); + } + } + + let mut output = Vec::new(); + collect(blocker, attackers, maximum, 0, &mut Vec::new(), &mut output); + output +} + +/// CR 509.1c: complete an AI/generated blocker proposal through the same +/// maximum-requirement authority as a player declaration. A valid, tax-free +/// maximum proposal is preserved; every other proposal becomes the deterministic +/// tax-free witness so callers cannot wedge on a rejected declaration. +pub fn complete_blocker_proposal( + state: &GameState, + player: PlayerId, + proposed: &[(ObjectId, ObjectId)], +) -> crate::types::actions::GameAction { + let constraints = BlockDeclarationConstraints::build(state, player); + let required = constraints.max_free_score(state); + let valid = validate_blockers_core(state, player, proposed).is_ok() + && constraints.score_with_state(state, proposed) >= required + && compute_block_tax(state, proposed).is_none(); + let assignments = if valid { + proposed.to_vec() + } else { + constraints.best_free_declaration(state).0 + }; + crate::types::actions::GameAction::DeclareBlockers { assignments } +} + +/// Batch form of [`complete_blocker_proposal`] for engine legal-action +/// generation. The constraints model and its deterministic witness are shared +/// across all candidates from one unchanged state. +pub fn complete_blocker_proposals( + state: &GameState, + player: PlayerId, + proposals: &[Vec<(ObjectId, ObjectId)>], +) -> Vec { + let constraints = BlockDeclarationConstraints::build(state, player); + let required = constraints.max_free_score(state); + let witness = constraints.best_free_declaration(state).0; + proposals + .iter() + .map(|proposal| { + let valid = validate_blockers_core(state, player, proposal).is_ok() + && constraints.score_with_state(state, proposal) >= required + && compute_block_tax(state, proposal).is_none(); + crate::types::actions::GameAction::DeclareBlockers { + assignments: if valid { + proposal.clone() + } else { + witness.clone() + }, + } + }) + .collect() +} + /// Validate one defending player's blocker declaration per CR 509.1 and CR 802.4. pub fn validate_blockers_for_player( state: &GameState, player: PlayerId, assignments: &[(ObjectId, ObjectId)], +) -> Result<(), String> { + let constraints = BlockDeclarationConstraints::build(state, player); + validate_blockers_core(state, player, assignments)?; + let required = constraints.max_free_score(state); + let score = constraints.score_with_state(state, assignments); + if score < required { + return Err(format!( + "Declaration obeys {score} block requirement(s) but {required} are obtainable without paying a cost (CR 509.1c)" + )); + } + Ok(()) +} + +/// The hard-restriction half of declaring blockers. Requirements are deliberately +/// excluded here: CR 509.1c compares a declaration with the maximum number of +/// requirements obtainable by *a whole declaration*, not with each requirement +/// independently. +fn validate_blockers_core( + state: &GameState, + player: PlayerId, + assignments: &[(ObjectId, ObjectId)], ) -> Result<(), String> { // CR 509.1b: Block restrictions make the declaration illegal if disobeyed. if let Some(max) = max_blockers_each_combat(state) { @@ -1359,7 +1814,6 @@ pub fn validate_blockers_for_player( let blocker_restriction = collect_blocker_restriction_statics(state); let block_restriction = collect_block_restriction_statics(state); let blocker_allowed = collect_blocker_allowed_statics(state); - let must_be_blocked = collect_must_be_blocked_statics(state); // CR 604.1: loop-invariant existence gate for the shadow block-lift (CR // 509.1b/609.4/702.28b). Hoisted once so the per-blocker shadow scan below // and every `can_block_pair_with_precomputed` call skip the O(N) @@ -1790,307 +2244,6 @@ pub fn validate_blockers_for_player( } } - // CR 509.1c: MustBeBlocked — if a creature with a "must be blocked if able" - // requirement is attacking, the defending player must obey it by assigning a - // qualifying blocker whenever one is able. Each requirement on the attacker - // is enforced independently (an attacker may carry both a bare and a filtered - // requirement): `by == None` ⇒ any assigned blocker satisfies it; `by == - // Some(filter)` ⇒ only an assigned blocker matching `filter` does (Ace's - // Baseball Bat: a Dalek; Slayer's Cleaver: an Eldrazi). Uses the engine's - // existing per-attacker greedy approximation of the CR 509.1c requirement- - // maximization rule, applied uniformly to the bare and filtered forms. - if let Some(combat) = &state.combat { - // Collect all assigned blocker IDs for quick lookup - let assigned_blockers: std::collections::HashSet = assignments - .iter() - .map(|&(blocker_id, _)| blocker_id) - .collect(); - - for attacker_info in &combat.attackers { - if attacker_info.defending_player != player { - continue; - } - let attacker_id = attacker_info.object_id; - - let requirements: Vec<(Option<&TargetFilter>, ObjectId, Option)> = - must_be_blocked_requirements_for_attacker(state, attacker_id, &must_be_blocked) - .collect(); - - for (by, src_id, anchor) in requirements { - // CR 509.1c: the requirement is obeyed if a qualifying blocker is - // already assigned to this attacker — `None` ⇒ any assigned - // blocker; `Some(filter)` ⇒ an assigned blocker matching `filter`. - let satisfied = blockers_per_attacker - .get(&attacker_id) - .is_some_and(|blockers| { - blockers.iter().any(|blocker_id| match by { - None => true, - Some(filter) => matches_target_filter( - state, - *blocker_id, - filter, - &blocker_filter_context(state, src_id, anchor), - ), - }) - }); - if satisfied { - continue; - } - - // Check if any defending creature not yet assigned to THIS - // attacker could legally block it AND (for the filtered form) - // match the filter. If so, the declaration is illegal because - // that creature should have been assigned. CR 509.1b: a - // creature that can't legally block doesn't make the - // requirement obey-able. - // - // CR 509.1c: a creature already blocking another attacker is - // still "able" to block this one if it has spare block - // capacity granted by ExtraBlockers — mirror of the - // MustBeBlockedByAll path at line ~1496 above. - let has_available_blocker = state.battlefield.iter().any(|id| { - // Skip creatures already assigned to this specific attacker - // — they're already counted in the `satisfied` check above. - if blockers_per_attacker - .get(&attacker_id) - .is_some_and(|blockers| blockers.contains(id)) - { - return false; - } - let Some(obj) = state.objects.get(id) else { - return false; - }; - if obj.controller != player - || !obj.card_types.core_types.contains(&CoreType::Creature) - || obj.tapped - { - return false; - } - // A creature blocking other attacker(s) is only "able" to - // also block this one if it has spare block capacity. - // CR 509.1c: a blocker at its per-creature limit cannot - // take on an additional block. - let assigned_count = attackers_per_blocker.get(id).copied().unwrap_or(0); - if assigned_count >= extra_block_limit(state, obj) { - return false; - } - can_block_pair_with_precomputed( - state, - *id, - attacker_id, - &blocker_restriction, - &block_restriction, - &blocker_allowed, - can_block_shadow_exists, - ) && match by { - None => true, - Some(filter) => matches_target_filter( - state, - *id, - filter, - &blocker_filter_context(state, src_id, anchor), - ), - } - }); - - if has_available_blocker { - return Err(match by { - None => format!("{attacker_id:?} must be blocked if able (CR 509.1c)"), - Some(_) => format!( - "{attacker_id:?} must be blocked by a qualifying creature if able (CR 509.1c)" - ), - }); - } - } - } - - // CR 509.1c: MustBeBlockedByAll — the "lure" requirement ("All creatures - // able to block ~ do so": Lure, Prized Unicorn, Breaker of Armies, …). - // Every creature this defending player controls that could legally block - // the lured attacker carries a "must block it" requirement, so — unlike - // MustBeBlocked, which is satisfied by a single blocker — there must be - // *no* creature with spare block capacity able to block it left off that - // attacker. A blocker already at its per-creature block limit is not - // able to add another block; a blocker with ExtraBlockers spare capacity - // still must also block the lured attacker. - for attacker_info in &combat.attackers { - if attacker_info.defending_player != player { - continue; - } - let attacker_id = attacker_info.object_id; - - // Collect the requirements before the inner `state.battlefield` iter - // to drop the precomputed borrow (mirrors the sibling collect at - // ~1479-1481 for the MustBeBlocked loop). - let requirements: Vec<(Option<&TargetFilter>, ObjectId, Option)> = - must_be_blocked_by_all_requirements_for_attacker( - state, - attacker_id, - &must_be_blocked, - ) - .collect(); - - for (blockers, src_id, anchor) in requirements { - // Any untapped defender with spare block capacity that could - // legally block the lured attacker should have been declared as - // its blocker. `blockers == None` ⇒ every idle able creature is - // compelled (unchanged Lure); `Some(filter)` ⇒ only idle able - // creatures matching `filter` are compelled — non-matching - // creatures stay legal to leave off. CR 509.1c. - let has_idle_able_blocker = state.battlefield.iter().any(|id| { - if blockers_per_attacker - .get(&attacker_id) - .is_some_and(|assigned| assigned.contains(id)) - { - return false; - } - let Some(obj) = state.objects.get(id) else { - return false; - }; - if obj.controller != player - || !obj.card_types.core_types.contains(&CoreType::Creature) - || obj.tapped - || !can_block_pair_with_precomputed( - state, - *id, - attacker_id, - &blocker_restriction, - &block_restriction, - &blocker_allowed, - can_block_shadow_exists, - ) - { - return false; - } - let assigned_count = attackers_per_blocker.get(id).copied().unwrap_or(0); - assigned_count < extra_block_limit(state, obj) - // CR 509.1c: filtered lure — only creatures matching the - // filter carry the "must block" requirement. - && blockers.is_none_or(|f| { - matches_target_filter( - state, - *id, - f, - &blocker_filter_context(state, src_id, anchor), - ) - }) - }); - if has_idle_able_blocker { - return Err(match blockers { - None => format!( - "{attacker_id:?} must be blocked by every creature able to block it (CR 509.1c)" - ), - Some(_) => format!( - "{attacker_id:?} must be blocked by every qualifying creature able to block it (CR 509.1c)" - ), - }); - } - } - } - - // CR 509.1c + CR 802.4b: Check MustBlock only for this defending - // player's declaration. - // If a defending creature has MustBlock and isn't assigned as a blocker, - // verify it couldn't legally block any attacker. - // CR 604.1: hoist the MustBlock existence gate once before iterating N - // permanents so the per-permanent `check_static_ability` re-scan is - // skipped when no functioning MustBlock static exists (O(N^2) -> O(N)). - let has_must_block_static = static_kind_present(state, StaticModeKind::MustBlock); - for &obj_id in &state.battlefield { - // Already assigned as a blocker — constraint satisfied. The rest of - // the enforcement predicate is the shared authority used by the - // display-population helper (`blocker_constraints_for_player`). - if assigned_blockers.contains(&obj_id) { - continue; - } - if creature_has_must_block_requirement( - state, - obj_id, - player, - has_must_block_static, - &blocker_restriction, - &block_restriction, - &blocker_allowed, - can_block_shadow_exists, - ) { - return Err(format!("{:?} must block if able (CR 509.1c)", obj_id)); - } - } - - // CR 702.39a + CR 509.1c: MustBlockAttacker — a creature forced to block - // a SPECIFIC attacker (Provoke; "target creature blocks ~ this turn if - // able") must be declared as a blocker of *that* attacker when it can - // legally block it. This is stricter than generic MustBlock: blocking a - // different attacker does not satisfy the requirement. - for &obj_id in &state.battlefield { - let Some(obj) = state.objects.get(&obj_id) else { - continue; - }; - if obj.controller != player || !obj.card_types.core_types.contains(&CoreType::Creature) - { - continue; - } - // The specific attackers this creature is required to block. - // CR 702.26b + CR 604.1: `active_static_definitions` owns the gating. - let required: Vec = - super::functioning_abilities::active_static_definitions(state, obj) - .filter_map(|sd| match sd.mode { - StaticMode::MustBlockAttacker { attacker } => Some(attacker), - _ => None, - }) - .collect(); - if required.is_empty() { - continue; - } - // A creature that can't block at all carries no requirement - // (CR 509.1a tapped / CR 702.147a decayed / CR 701.35a detained / - // CantBlock). `can_block_pair` does not itself reject tapped, so the - // explicit guards mirror the generic MustBlock check above. - if obj.tapped - || obj.has_keyword(&Keyword::Decayed) - || !obj.detained_by.is_empty() - || blocker_has_cant_block_static_from_precomputed( - state, - obj_id, - &blocker_restriction, - ) - { - continue; - } - for attacker_id in required { - // Only enforce while that attacker is actually attacking this - // defending player this combat. - let is_active_attacker = combat - .attackers - .iter() - .any(|ai| ai.object_id == attacker_id && ai.defending_player == player); - if !is_active_attacker { - continue; - } - // If the creature can legally block the named attacker but isn't - // declared as its blocker, the declaration is illegal. - let assigned_to_attacker = assignments - .iter() - .any(|&(blocker, attacker)| blocker == obj_id && attacker == attacker_id); - if !assigned_to_attacker - && can_block_pair_with_precomputed( - state, - obj_id, - attacker_id, - &blocker_restriction, - &block_restriction, - &blocker_allowed, - can_block_shadow_exists, - ) - { - return Err(format!( - "{obj_id:?} must block {attacker_id:?} this turn if able (CR 509.1c)" - )); - } - } - } - } - Ok(()) } @@ -2392,8 +2545,16 @@ pub fn compute_block_tax( crate::types::mana::ManaCost, Vec<(ObjectId, crate::types::mana::ManaCost)>, )> { - let pairs: Vec<(ObjectId, Option)> = - assignments.iter().map(|(b, _)| (*b, None)).collect(); + // CR 509.1a + CR 509.1d: a creature may block more than one attacker, but + // a block tax that applies to "each creature ... that blocks" applies once + // to that creature, not once to every declared pair. + let mut blockers: Vec = assignments.iter().map(|(blocker, _)| *blocker).collect(); + blockers.sort_unstable(); + blockers.dedup(); + let pairs: Vec<(ObjectId, Option)> = blockers + .into_iter() + .map(|blocker| (blocker, None)) + .collect(); compute_combat_tax( state, &pairs, @@ -4018,6 +4179,10 @@ pub(super) fn commit_attack_declaration( }) .collect(); apply_attack_band_ids(&mut attackers, bands); + let attacking_incarnations_this_combat = attacker_ids + .iter() + .filter_map(|id| state.objects.get(id).map(ObjectIncarnationRef::from_object)) + .collect(); let combat = state.combat.get_or_insert_with(CombatState::default); combat.attackers = attackers; state.players_attacked_this_step = combat @@ -4037,6 +4202,7 @@ pub(super) fn commit_attack_declaration( .iter() .map(|attacker| (attacker.object_id, attacker.defending_player)) .collect(); + combat.attacking_incarnations_this_combat = attacking_incarnations_this_combat; combat.attacked_defenders_this_combat.clear(); combat.creature_attacked_defenders_this_combat.clear(); for (attacker_id, defending_player) in &creature_attacked_defenders { @@ -4161,6 +4327,58 @@ pub(super) fn snapshot_attack_declaration( (snap_attacks, snap_bands) } +/// CR 400.7 + CR 509.1d: Capture the exact blocker/attacker pairs quoted by a +/// blocking tax. Both ends are identities because either object may leave and +/// re-enter while the defending player decides whether to pay. +pub fn snapshot_block_declaration( + state: &GameState, + assignments: &[(ObjectId, ObjectId)], +) -> Vec<( + crate::types::identifiers::ObjectIncarnationRef, + crate::types::identifiers::ObjectIncarnationRef, +)> { + assignments + .iter() + .filter_map(|(blocker, attacker)| { + Some(( + crate::types::identifiers::ObjectIncarnationRef::from_object( + state.objects.get(blocker)?, + ), + crate::types::identifiers::ObjectIncarnationRef::from_object( + state.objects.get(attacker)?, + ), + )) + }) + .collect() +} + +/// CR 400.7 + CR 509.1d: Recover only pairs whose two exact snapshots still +/// name the live objects. This deliberately performs no tax recomputation. +pub fn block_declaration_from_snapshot( + state: &GameState, + assignments: &[( + crate::types::identifiers::ObjectIncarnationRef, + crate::types::identifiers::ObjectIncarnationRef, + )], +) -> Vec<(ObjectId, ObjectId)> { + assignments + .iter() + .filter_map(|(blocker, attacker)| { + let blocker_live = state.objects.get(&blocker.object_id).is_some_and(|object| { + crate::types::identifiers::ObjectIncarnationRef::from_object(object) == *blocker + }); + let attacker_live = state + .objects + .get(&attacker.object_id) + .is_some_and(|object| { + crate::types::identifiers::ObjectIncarnationRef::from_object(object) + == *attacker + }); + (blocker_live && attacker_live).then_some((blocker.object_id, attacker.object_id)) + }) + .collect() +} + /// Declare attackers without explicit band declarations. pub fn declare_attackers( state: &mut GameState, @@ -4564,7 +4782,7 @@ pub fn blocker_constraints_for_player( continue; } if valid_block_targets.contains_key(&obj_id) { - if creature_has_must_block_requirement( + let generic = creature_has_must_block_requirement( state, obj_id, player, @@ -4573,16 +4791,44 @@ pub fn blocker_constraints_for_player( &block_restriction, &blocker_allowed, can_block_shadow_exists, - ) { + ); + let mut attackers: Vec = + super::functioning_abilities::active_static_definitions(state, obj) + .filter_map(|definition| match definition.mode { + StaticMode::MustBlockAttacker { attacker } + if valid_block_targets + .get(&obj_id) + .is_some_and(|targets| targets.contains(&attacker.object_id)) + && state.combat.as_ref().is_some_and(|combat| { + combat.attackers.iter().any(|info| { + info.object_id == attacker.object_id + && info.defending_player == player + && state.objects.get(&attacker.object_id).is_some_and( + |object| { + ObjectIncarnationRef::from_object(object) + == attacker + }, + ) + }) + }) => + { + Some(attacker.object_id) + } + _ => None, + }) + .collect(); + attackers.sort_unstable(); + attackers.dedup(); + if generic || !attackers.is_empty() { constraints.insert( obj_id, CombatRequirement::MustBlock { - sources: must_block_sources_gated( - state, - obj, - obj_id, - has_must_block_static, - ), + sources: if generic { + must_block_sources_gated(state, obj, obj_id, has_must_block_static) + } else { + vec![obj_id] + }, + attackers, }, ); } @@ -4640,6 +4886,26 @@ pub fn build_declare_attackers_waiting_for( } } +/// CR 509.1a-c: Build a fresh blocker prompt from the sole live constraint +/// surface. Used after a declined block tax so stale proposal pairs cannot be +/// silently filtered and committed. +pub fn build_declare_blockers_waiting_for( + state: &GameState, + player: PlayerId, +) -> crate::types::game_state::WaitingFor { + let valid_block_targets = get_valid_block_targets_for_player(state, player); + let valid_blocker_ids = ordered_valid_blocker_ids(&valid_block_targets); + let block_requirements = block_requirements_for_player(state, player); + let blocker_constraints = blocker_constraints_for_player(state, player, &valid_block_targets); + crate::types::game_state::WaitingFor::DeclareBlockers { + player, + valid_blocker_ids, + valid_block_targets, + block_requirements, + blocker_constraints, + } +} + pub fn refresh_combat_declaration_waiting_for(state: &mut GameState) { match &state.waiting_for { crate::types::game_state::WaitingFor::DeclareAttackers { .. } => { @@ -10069,6 +10335,34 @@ mod tests { assert!(validate_blockers(&state, &[(blocker, attacker)]).is_ok()); } + /// CR 509.1c: a wide defender board still chooses the deterministic + /// shortest lexicographic maximum witness. This exercises the solver's + /// equal-bound dominance pruning: the old raw Cartesian search would visit + /// every subset of sixteen otherwise interchangeable blockers. + #[test] + fn wide_must_block_board_keeps_shortest_lexicographic_witness() { + let mut state = setup(); + let attacker = create_creature(&mut state, PlayerId(0), "Lure Beast", 3, 3); + add_must_be_blocked(&mut state, attacker); + let blockers: Vec<_> = (0..16) + .map(|index| { + create_creature(&mut state, PlayerId(1), &format!("Blocker {index}"), 2, 2) + }) + .collect(); + state.combat = Some(CombatState { + attackers: vec![AttackerInfo::attacking_player(attacker, PlayerId(1))], + ..Default::default() + }); + + let crate::types::actions::GameAction::DeclareBlockers { assignments } = + complete_blocker_proposal(&state, PlayerId(1), &[]) + else { + panic!("blocker completion must return a blocker declaration"); + }; + assert_eq!(assignments, vec![(blockers[0], attacker)]); + assert!(validate_blockers(&state, &assignments).is_ok()); + } + #[test] fn must_be_blocked_ok_when_no_legal_blockers() { // CR 509.1c "if able": no legal blockers means empty assignment is fine. @@ -10877,6 +11171,9 @@ mod tests { } fn add_must_block_attacker(state: &mut GameState, creature: ObjectId, attacker: ObjectId) { + let attacker = crate::types::identifiers::ObjectIncarnationRef::from_object( + state.objects.get(&attacker).unwrap(), + ); state .objects .get_mut(&creature) @@ -10914,6 +11211,31 @@ mod tests { assert!(validate_blockers(&state, &[(forced, provoker)]).is_ok()); } + #[test] + fn competing_exact_block_requirements_accept_a_maximum_declaration() { + // CR 509.1c: requirements are maximized as a whole. One ordinary + // blocker cannot block both attackers, so either one-pair declaration + // obeys the obtainable maximum; the former greedy per-requirement loops + // incorrectly rejected both. + let mut state = setup(); + let first = create_creature(&mut state, PlayerId(0), "First", 2, 2); + let second = create_creature(&mut state, PlayerId(0), "Second", 2, 2); + let blocker = create_creature(&mut state, PlayerId(1), "Guard", 2, 2); + add_must_block_attacker(&mut state, blocker, first); + add_must_block_attacker(&mut state, blocker, second); + state.combat = Some(CombatState { + attackers: vec![ + AttackerInfo::attacking_player(first, PlayerId(1)), + AttackerInfo::attacking_player(second, PlayerId(1)), + ], + ..Default::default() + }); + + assert!(validate_blockers(&state, &[]).is_err()); + assert!(validate_blockers(&state, &[(blocker, first)]).is_ok()); + assert!(validate_blockers(&state, &[(blocker, second)]).is_ok()); + } + #[test] fn must_block_attacker_exempt_when_cannot_block() { // CR 509.1a: a tapped creature can't block, so the provoke requirement @@ -11282,7 +11604,8 @@ mod tests { constraints.get(&blocker), // CR 509.1c: intrinsic MustBlock → carrier is the creature itself. Some(&CombatRequirement::MustBlock { - sources: vec![blocker] + sources: vec![blocker], + attackers: vec![], }), "a must-block creature able to block the attacker is MustBlock" ); @@ -11322,7 +11645,8 @@ mod tests { blocker_constraints_for_player(&state, PlayerId(1), &valid).get(&blocker), // CR 509.1c: intrinsic MustBlock → carrier is the creature itself. Some(&CombatRequirement::MustBlock { - sources: vec![blocker] + sources: vec![blocker], + attackers: vec![], }), "reach-guard: ground blocker CAN block the ground attacker" ); diff --git a/crates/engine/src/game/combat_damage.rs b/crates/engine/src/game/combat_damage.rs index b4a9158917..508dd3d327 100644 --- a/crates/engine/src/game/combat_damage.rs +++ b/crates/engine/src/game/combat_damage.rs @@ -1444,6 +1444,7 @@ mod tests { timestamp: ts, duration: Duration::UntilEndOfTurn, affected: TargetFilter::SelfRef, + affected_recipient: None, modifications: vec![ ContinuousModification::AddPower { value: 1 }, ContinuousModification::AddToughness { value: 1 }, diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index a328e00979..2e707310e6 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -2353,7 +2353,6 @@ fn effect_details(effect: &Effect) -> Vec<(String, String)> { | Effect::Connive { target, .. } | Effect::PhaseOut { target } | Effect::PhaseIn { target } - | Effect::ForceBlock { target } | Effect::ForceAttack { target, .. } | Effect::Transform { target } // CR 710.4: the flipping permanent is the effect's single reported target. @@ -2364,6 +2363,19 @@ fn effect_details(effect: &Effect) -> Vec<(String, String)> { | Effect::RemoveAllDamage { target } => { d.push(("target".into(), fmt_target(target))); } + Effect::ForceBlock { + target, + attacker, + duration, + } => { + d.push(("target".into(), fmt_target(target))); + if let Some(attacker) = attacker { + d.push(("attacker".into(), format!("{attacker:?}"))); + } + if *duration != Duration::UntilEndOfTurn { + d.push(("duration".into(), format!("{duration:?}"))); + } + } // CR 702.50a: EpicCopy's parameters live in its snapshotted ability. Effect::EpicCopy { .. } => {} Effect::Intensify { .. } => {} @@ -3938,6 +3950,7 @@ fn fmt_trigger_condition(cond: &crate::types::ability::TriggerCondition) -> Stri } TC::ControlsNone { filter } => format!("you control no {}", fmt_target(filter)), TC::AttackedThisTurn => "attacked this turn".into(), + TC::SourceAttackedThisCombat => "source attacked this combat".into(), TC::FirstCombatPhaseOfTurn => "first combat phase of the turn".into(), TC::CastSpellThisTurn { filter } => match filter { Some(f) => format!("cast a {} spell this turn", fmt_target(f)), @@ -13817,7 +13830,7 @@ mod tests { let mut face = make_face(); face.static_abilities.push( StaticDefinition::new(StaticMode::MustBlockAttacker { - attacker: ObjectId(42), + attacker: crate::types::identifiers::ObjectIncarnationRef::of(ObjectId(42), 0), }) .description("Target creature blocks this creature this turn if able.".to_string()), ); diff --git a/crates/engine/src/game/effects/additional_phase.rs b/crates/engine/src/game/effects/additional_phase.rs index ebc1811c7e..0e6e4a613b 100644 --- a/crates/engine/src/game/effects/additional_phase.rs +++ b/crates/engine/src/game/effects/additional_phase.rs @@ -270,6 +270,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, targets: vec![], kind: AbilityKind::Spell, sub_ability: None, diff --git a/crates/engine/src/game/effects/double.rs b/crates/engine/src/game/effects/double.rs index 2e79d947cf..602948423b 100644 --- a/crates/engine/src/game/effects/double.rs +++ b/crates/engine/src/game/effects/double.rs @@ -322,6 +322,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, targets, kind: AbilityKind::Spell, sub_ability: None, diff --git a/crates/engine/src/game/effects/extra_turn.rs b/crates/engine/src/game/effects/extra_turn.rs index a319d7f88d..0bd0738e82 100644 --- a/crates/engine/src/game/effects/extra_turn.rs +++ b/crates/engine/src/game/effects/extra_turn.rs @@ -67,6 +67,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, targets: vec![], kind: AbilityKind::Spell, sub_ability: None, diff --git a/crates/engine/src/game/effects/force_block.rs b/crates/engine/src/game/effects/force_block.rs index f56d20377a..d26bce05a9 100644 --- a/crates/engine/src/game/effects/force_block.rs +++ b/crates/engine/src/game/effects/force_block.rs @@ -1,18 +1,13 @@ use crate::game::targeting::resolved_object_ids_for_filter; use crate::types::ability::{ - ContinuousModification, Duration, Effect, EffectError, EffectKind, ResolvedAbility, - TargetFilter, + ContinuousModification, Effect, EffectError, EffectKind, ResolvedAbility, TargetFilter, }; use crate::types::events::GameEvent; use crate::types::game_state::GameState; +use crate::types::identifiers::ObjectIncarnationRef; use crate::types::statics::StaticMode; -/// CR 509.1c: Force block — the target creature must block this turn if able. -/// -/// If the effect source is currently an attacker, this is the Provoke/source- -/// referential shape (CR 702.39a: "block this creature if able"), so grant -/// `MustBlockAttacker { attacker: source }`. Otherwise preserve the generic -/// attacker-agnostic `MustBlock` shape for "blocks this turn if able". +/// CR 509.1c: Force block — the target creature must block if able. /// /// Note: `MustBlock` (creature must block any attacker), `MustBlockAttacker` /// (creature must block one specific attacker), and `MustBeBlocked` (creature @@ -29,27 +24,53 @@ pub fn resolve( ability: &ResolvedAbility, events: &mut Vec, ) -> Result<(), EffectError> { - let target_filter = match &ability.effect { - Effect::ForceBlock { target } => target, + let (target_filter, named_attacker, has_named_attacker, duration) = match &ability.effect { + Effect::ForceBlock { + target, + attacker, + duration, + .. + } => ( + target, + ability.force_block_attacker.or_else(|| match attacker { + Some(crate::types::ability::ForceBlockAttackerRef::Source) => ability + .source_incarnation + .map(|incarnation| ObjectIncarnationRef { + object_id: ability.source_id, + incarnation, + }), + _ => None, + }), + attacker.is_some(), + duration, + ), _ => return Ok(()), }; - let source_is_active_attacker = state.combat.as_ref().is_some_and(|combat| { - combat - .attackers - .iter() - .any(|attacker| attacker.object_id == ability.source_id) - }); - let mode = if source_is_active_attacker { - // CR 702.39a + CR 509.1c: Provoke/source-referential force-block - // effects require blocking this specific attacking source. - StaticMode::MustBlockAttacker { - attacker: ability.source_id, + let mode = match named_attacker { + // CR 400.7 + CR 509.1c: Only a still-live exact incarnation that is + // currently attacking is a legal named attacker. A departed/re-entered + // object cannot be rediscovered from its raw id. + Some(attacker) + if state.combat.as_ref().is_some_and(|combat| { + combat.attackers.iter().any(|info| { + info.object_id == attacker.object_id + && state + .objects + .get(&attacker.object_id) + .is_some_and(|object| { + ObjectIncarnationRef::from_object(object) == attacker + }) + }) + }) => + { + StaticMode::MustBlockAttacker { attacker } } - } else { - // CR 509.1c: Generic "blocks this turn if able" effects only require - // blocking some legal attacker. - StaticMode::MustBlock + // An unavailable named referent cannot become a generic requirement. + // The instruction has no attacker it can require a block against. + Some(_) => return Ok(()), + None if has_named_attacker => return Ok(()), + None => StaticMode::MustBlock, }; for obj_id in resolved_object_ids_for_filter(state, ability, target_filter) { @@ -59,14 +80,16 @@ pub fn resolve( continue; } - state.add_transient_continuous_effect( + let recipient = ObjectIncarnationRef::from_object(&state.objects[&obj_id]); + let effect_id = state.add_transient_continuous_effect( ability.source_id, ability.controller, - Duration::UntilEndOfTurn, + duration.clone(), TargetFilter::SpecificObject { id: obj_id }, vec![ContinuousModification::AddStaticMode { mode: mode.clone() }], None, ); + state.set_transient_affected_recipient(effect_id, recipient); } events.push(GameEvent::EffectResolved { @@ -82,8 +105,10 @@ mod tests { use super::*; use crate::game::combat::{AttackerInfo, CombatState}; use crate::game::zones::create_object; - use crate::types::ability::{ControllerRef, Effect, TargetRef, TypedFilter}; - use crate::types::identifiers::{CardId, ObjectId}; + use crate::types::ability::{ + ControllerRef, Effect, ForceBlockAttackerRef, TargetRef, TypedFilter, + }; + use crate::types::identifiers::{CardId, ObjectId, ObjectIncarnationRef}; use crate::types::player::PlayerId; use crate::types::zones::Zone; @@ -91,6 +116,8 @@ mod tests { ResolvedAbility::new( Effect::ForceBlock { target: TargetFilter::Any, + attacker: None, + duration: crate::types::ability::Duration::UntilEndOfTurn, }, vec![TargetRef::Object(target)], source, @@ -166,7 +193,14 @@ mod tests { ..Default::default() }); - let ability = make_force_block_ability(source, target); + let mut ability = make_force_block_ability(source, target); + ability.effect = Effect::ForceBlock { + target: TargetFilter::Any, + attacker: Some(ForceBlockAttackerRef::Source), + duration: crate::types::ability::Duration::UntilEndOfTurn, + }; + ability.force_block_attacker = + Some(ObjectIncarnationRef::from_object(&state.objects[&source])); let mut events = Vec::new(); resolve(&mut state, &ability, &mut events).unwrap(); @@ -177,7 +211,7 @@ mod tests { m, ContinuousModification::AddStaticMode { mode: StaticMode::MustBlockAttacker { attacker }, - } if *attacker == source + } if attacker.object_id == source ) }) }), @@ -185,6 +219,83 @@ mod tests { ); } + #[test] + fn tolsimir_attack_trigger_binds_the_wolf_not_tolsimir() { + // Regression: the old resolver inferred the named attacker from the + // triggered ability's source. Tolsimir is not the attacker named by + // "that Wolf"; the event Wolf is. Reverting the pending-ability + // provenance binding makes this assertion select Tolsimir or generic + // MustBlock instead. + let mut state = GameState::new_two_player(42); + let tolsimir = create_object( + &mut state, + CardId(9), + PlayerId(0), + "Tolsimir, Midnight's Light".to_string(), + Zone::Battlefield, + ); + let wolf = create_object( + &mut state, + CardId(418), + PlayerId(0), + "Voja Fenstalker".to_string(), + Zone::Battlefield, + ); + let blocker = create_object( + &mut state, + CardId(3), + PlayerId(1), + "Opponent Bear".to_string(), + Zone::Battlefield, + ); + state.combat = Some(CombatState { + attackers: vec![AttackerInfo::attacking_player(wolf, PlayerId(1))], + ..Default::default() + }); + + let mut ability = ResolvedAbility::new( + Effect::ForceBlock { + target: TargetFilter::Any, + attacker: Some(ForceBlockAttackerRef::EventSource), + duration: crate::types::ability::Duration::UntilEndOfCombat, + }, + vec![TargetRef::Object(blocker)], + tolsimir, + PlayerId(0), + ); + ability.bind_force_block_attacker_recursive(Some(ObjectIncarnationRef::from_object( + &state.objects[&wolf], + ))); + + let mut events = Vec::new(); + resolve(&mut state, &ability, &mut events).unwrap(); + + let effect = state + .transient_continuous_effects + .iter() + .find(|effect| effect.affected_recipient.is_some()) + .cloned() + .expect("targeted force block installs an exact-recipient TCE"); + assert_eq!( + effect.affected_recipient, + Some(ObjectIncarnationRef::from_object(&state.objects[&blocker])) + ); + assert!(effect.modifications.iter().any(|modification| { + matches!( + modification, + ContinuousModification::AddStaticMode { + mode: StaticMode::MustBlockAttacker { attacker }, + } if attacker.object_id == wolf + ) + })); + + state.objects.get_mut(&blocker).unwrap().incarnation += 1; + assert!( + !crate::game::layers::transient_effect_is_live(&state, &effect), + "the targeted force-block TCE must not apply to a later blocker incarnation" + ); + } + #[test] fn force_block_multiple_targets() { let mut state = GameState::new_two_player(42); @@ -213,6 +324,8 @@ mod tests { let ability = ResolvedAbility::new( Effect::ForceBlock { target: TargetFilter::Any, + attacker: None, + duration: crate::types::ability::Duration::UntilEndOfTurn, }, vec![TargetRef::Object(target1), TargetRef::Object(target2)], source, @@ -285,6 +398,8 @@ mod tests { target: TargetFilter::Typed( TypedFilter::creature().controller(ControllerRef::Opponent), ), + attacker: None, + duration: crate::types::ability::Duration::UntilEndOfTurn, }, vec![], source, diff --git a/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs b/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs index 7ff475faec..761be2b674 100644 --- a/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs +++ b/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs @@ -93,6 +93,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, targets: vec![], kind: AbilityKind::Activated, sub_ability: None, diff --git a/crates/engine/src/game/effects/player_counter.rs b/crates/engine/src/game/effects/player_counter.rs index 7b6674444a..bd7130cacc 100644 --- a/crates/engine/src/game/effects/player_counter.rs +++ b/crates/engine/src/game/effects/player_counter.rs @@ -336,6 +336,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, targets: vec![], kind: AbilityKind::Spell, sub_ability: None, @@ -529,6 +530,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, targets: vec![], kind: AbilityKind::Spell, sub_ability: None, diff --git a/crates/engine/src/game/effects/reverse_turn_order.rs b/crates/engine/src/game/effects/reverse_turn_order.rs index 267d1491c7..0eea7a4426 100644 --- a/crates/engine/src/game/effects/reverse_turn_order.rs +++ b/crates/engine/src/game/effects/reverse_turn_order.rs @@ -48,6 +48,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, targets: vec![], kind: AbilityKind::Spell, sub_ability: None, diff --git a/crates/engine/src/game/effects/skip_next_step.rs b/crates/engine/src/game/effects/skip_next_step.rs index c357f10349..f7407178a5 100644 --- a/crates/engine/src/game/effects/skip_next_step.rs +++ b/crates/engine/src/game/effects/skip_next_step.rs @@ -111,6 +111,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, targets: vec![], kind: AbilityKind::Spell, sub_ability: None, diff --git a/crates/engine/src/game/effects/skip_next_turn.rs b/crates/engine/src/game/effects/skip_next_turn.rs index 66715c058b..83ef4a5629 100644 --- a/crates/engine/src/game/effects/skip_next_turn.rs +++ b/crates/engine/src/game/effects/skip_next_turn.rs @@ -89,6 +89,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, targets: vec![], kind: AbilityKind::Spell, sub_ability: None, diff --git a/crates/engine/src/game/effects/vote.rs b/crates/engine/src/game/effects/vote.rs index 5602ab2948..86b87fd47d 100644 --- a/crates/engine/src/game/effects/vote.rs +++ b/crates/engine/src/game/effects/vote.rs @@ -355,6 +355,7 @@ pub fn resolve_tally( source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, controller, original_controller: None, scoped_player: None, @@ -421,6 +422,7 @@ pub fn resolve_tally( source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, controller, original_controller: None, scoped_player: None, @@ -661,6 +663,7 @@ fn resolved_from_def( source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, controller, original_controller: None, scoped_player: None, @@ -922,6 +925,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, controller, original_controller: None, scoped_player: None, @@ -1029,6 +1033,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, controller, original_controller: None, scoped_player: None, @@ -1461,6 +1466,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, controller, original_controller: None, scoped_player: None, @@ -1625,6 +1631,7 @@ mod tests { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, controller, original_controller: None, scoped_player: None, diff --git a/crates/engine/src/game/engine_combat.rs b/crates/engine/src/game/engine_combat.rs index 6473a13d63..e3ad6c20b4 100644 --- a/crates/engine/src/game/engine_combat.rs +++ b/crates/engine/src/game/engine_combat.rs @@ -1,5 +1,3 @@ -use std::collections::HashSet; - use crate::game::combat::{AttackTarget, DamageAssignment, DamageTarget, TrampleKind}; use crate::types::ability::{CostPaidObjectSnapshot, TargetRef}; use crate::types::events::GameEvent; @@ -389,7 +387,12 @@ pub(super) fn handle_declare_blockers( } } - // CR 509.1c + CR 509.1d: Enumerate UnlessPay block-tax static abilities before + // CR 509.1a-c: Validate the submitted whole declaration before quoting a + // cost. A tax prompt must never bless an illegal or incomplete proposal. + super::combat::validate_blockers_for_player(state, player, assignments) + .map_err(EngineError::InvalidAction)?; + + // CR 509.1c + CR 509.1d: Enumerate UnlessPay block-tax static abilities after // finalizing the blocker declaration. Defending player pays or declines the // locked-in total; on decline, taxed blockers are dropped from the assignment // list (CR 509.1c: "that player is not required to pay that cost"). @@ -400,7 +403,7 @@ pub(super) fn handle_declare_blockers( total_cost, per_creature, pending: CombatTaxPending::Block { - assignments: assignments.to_vec(), + assignments: super::combat::snapshot_block_declaration(state, assignments), }, }); } @@ -416,10 +419,9 @@ pub(super) fn handle_declare_blockers( /// - `accept = true`: deduct the locked-in total via the shared mana-payment pipeline, /// then run the pending declaration with every creature intact (CR 508.1i–k: /// mana-abilities chance → pay costs → become attacking). -/// - `accept = false`: drop the taxed creatures from the declaration and submit the -/// remaining untaxed subset. If no creatures remain on the attack side, the engine -/// ends combat via `handle_empty_attackers` (CR 508.8); on the block side, submit -/// the filtered assignments. +/// - `accept = false`: discard the proposal and rebuild the appropriate declaration +/// prompt. The player may make a different legal declaration with a newly computed +/// tax quote. pub(super) fn handle_pay_combat_tax( state: &mut GameState, waiting_for: WaitingFor, @@ -440,6 +442,23 @@ pub(super) fn handle_pay_combat_tax( }; if accept { + let accepted_block_assignments = match &pending { + CombatTaxPending::Block { assignments } => { + let restored = super::combat::block_declaration_from_snapshot(state, assignments); + if restored.len() != assignments.len() { + return Err(EngineError::InvalidAction( + "Block declaration changed while its tax payment was pending".to_string(), + )); + } + // CR 509.1d: the declaration and its cost were fixed before + // the mana-ability/payment window. Do not rescore, revalidate, + // or reprice this exact snapshot here; only reject a stale + // incarnation atomically before any payment is taken. + Some(restored) + } + CombatTaxPending::Attack { .. } => None, + }; + // CR 508.1i–j / CR 509.1e–f: pay the locked-in total through the shared // unless-cost mana path. Failures bubble up to the caller. super::casting::pay_unless_cost(state, player, &total_cost, events)?; @@ -464,7 +483,9 @@ pub(super) fn handle_pay_combat_tax( events, ); } - CombatTaxPending::Block { assignments } => { + CombatTaxPending::Block { assignments: _ } => { + let assignments = accepted_block_assignments + .expect("block tax snapshots are restored and validated before payment"); return resume_declare_blockers(state, player, &assignments, events); } } @@ -481,19 +502,20 @@ pub(super) fn handle_pay_combat_tax( let _ = context; Ok(super::combat::build_declare_attackers_waiting_for(state)) } - // Block path is unchanged: filter the taxed blockers and submit the rest. + // CR 509.1d: declining discards the whole block proposal. Rebuild the + // live prompt; do not commit a stale filtered subset or reuse its price. CombatTaxPending::Block { assignments } => { - let taxed: HashSet = per_creature.iter().map(|(id, _)| *id).collect(); - let filtered: Vec<(ObjectId, ObjectId)> = assignments - .into_iter() - .filter(|(blocker, _)| !taxed.contains(blocker)) - .collect(); events.push(GameEvent::CombatTaxDeclined { player, - dropped: taxed.iter().copied().collect(), + dropped: assignments + .iter() + .map(|(blocker, _)| blocker.object_id) + .collect(), }); - let _ = context; // suppresses unused in this branch; kept for symmetry - resume_declare_blockers(state, player, &filtered, events) + let _ = (context, per_creature); + Ok(super::combat::build_declare_blockers_waiting_for( + state, player, + )) } } } diff --git a/crates/engine/src/game/layers.rs b/crates/engine/src/game/layers.rs index b23496db00..b7d90d360e 100644 --- a/crates/engine/src/game/layers.rs +++ b/crates/engine/src/game/layers.rs @@ -4604,6 +4604,17 @@ fn expand_granted_triggered_abilities( /// consults it too, so a display projection can never claim an effect is live /// after the layer engine has stopped applying it. pub(crate) fn transient_effect_is_live(state: &GameState, tce: &TransientContinuousEffect) -> bool { + // CR 400.7: a recipient that has changed zones is a new object, so a + // continuous effect tied to its prior incarnation cannot keep applying. + if let Some(recipient) = tce.affected_recipient { + if !state + .objects + .get(&recipient.object_id) + .is_some_and(|object| ObjectIncarnationRef::from_object(object) == recipient) + { + return false; + } + } // UntilHostLeavesPlay: skip if source is no longer on the battlefield if tce.duration == Duration::UntilHostLeavesPlay && !state @@ -13802,6 +13813,7 @@ mod tests { condition: StaticCondition::SourceIsTapped, }, affected: TargetFilter::SelfRef, + affected_recipient: None, modifications: vec![ContinuousModification::AddKeyword { keyword: Keyword::Flying, }], @@ -13834,6 +13846,7 @@ mod tests { condition: StaticCondition::SourceIsTapped, }, affected: TargetFilter::SelfRef, + affected_recipient: None, modifications: vec![ContinuousModification::AddKeyword { keyword: Keyword::Flying, }], diff --git a/crates/engine/src/game/meld_tests.rs b/crates/engine/src/game/meld_tests.rs index 93883d1144..ce0517d866 100644 --- a/crates/engine/src/game/meld_tests.rs +++ b/crates/engine/src/game/meld_tests.rs @@ -475,6 +475,7 @@ fn meld_renamed_non_meld_partner_is_noop() { timestamp: ts, duration: Duration::Permanent, affected: TargetFilter::SelfRef, + affected_recipient: None, modifications: vec![ContinuousModification::SetName { name: "Bruna, the Fading Light".to_string(), }], diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index f61e4d5098..c29debc471 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -25,6 +25,10 @@ use super::zone_pipeline::{self, ZoneMoveRequest, ZoneMoveResult}; /// CR 405.1: Add an object to the stack. pub fn push_to_stack(state: &mut GameState, mut entry: StackEntry, events: &mut Vec) { + let source_ref = state + .objects + .get(&entry.source_id) + .map(crate::types::identifiers::ObjectIncarnationRef::from_object); // CR 701.27f: an activated or triggered ability of a permanent may // transform that permanent only if it has not transformed/converted since // the ability was put onto the stack. Spells and keyword actions do not @@ -52,6 +56,12 @@ pub fn push_to_stack(state: &mut GameState, mut entry: StackEntry, events: &mut } } } + // CR 400.7 + CR 509.1c: source-referential force-block instructions bind + // their exact source at the common stack boundary, covering activated and + // other nontriggered stack abilities as well as normal triggered paths. + if let Some(ability) = entry.ability_mut() { + ability.bind_force_block_source_recursive(source_ref); + } events.push(GameEvent::StackPushed { object_id: entry.id, }); @@ -2258,6 +2268,7 @@ fn self_counter_ability_is_batch_candidate(ability: &ResolvedAbility) -> bool { source_incarnation, trigger_source, trigger_definition_ref, + force_block_attacker: _, controller: _, original_controller, scoped_player, @@ -2459,6 +2470,7 @@ fn fixed_controller_gain_life_ability_is_batch_candidate(ability: &ResolvedAbili source_incarnation: _, trigger_source: _, trigger_definition_ref: _, + force_block_attacker: _, controller: _, original_controller: _, scoped_player, @@ -2645,6 +2657,7 @@ fn fixed_opponent_lose_life_ability_is_batch_candidate(ability: &ResolvedAbility source_incarnation: _, trigger_source: _, trigger_definition_ref: _, + force_block_attacker: _, controller: _, original_controller: _, scoped_player, @@ -3256,6 +3269,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( source_incarnation: _, trigger_source: _, trigger_definition_ref: _, + force_block_attacker: a_force_block_attacker, controller: a_controller, original_controller: _, scoped_player: a_scoped_player, @@ -3308,6 +3322,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( source_incarnation: _, trigger_source: _, trigger_definition_ref: _, + force_block_attacker: b_force_block_attacker, controller: b_controller, original_controller: _, scoped_player: b_scoped_player, @@ -3356,6 +3371,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( a_effect == b_effect && a_targets == b_targets + && a_force_block_attacker == b_force_block_attacker && a_controller == b_controller && a_scoped_player == b_scoped_player && a_kind == b_kind diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index c5f13f0925..43d6813c84 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -395,6 +395,45 @@ fn matching_batched_trigger_events( .collect() } +/// CR 508.1 + CR 603.2: Split an attack declaration into the singleton event +/// contexts required by an event-referential attacker demonstrative. A plural +/// declaration has no single object for "that Wolf" to bind. +fn singleton_attack_events( + defending_player: PlayerId, + attacks: Vec<(ObjectId, crate::game::combat::AttackTarget)>, +) -> Vec { + attacks + .into_iter() + .map(|(attacker, target)| GameEvent::AttackersDeclared { + attacker_ids: vec![attacker], + defending_player, + attacks: vec![(attacker, target)], + }) + .collect() +} + +fn split_attack_event_into_singletons(event: &GameEvent) -> Option> { + let GameEvent::AttackersDeclared { + defending_player, + attacker_ids, + attacks, + } = event + else { + return None; + }; + let matching_attacks = attacker_ids + .iter() + .map(|attacker| { + let target = attacks + .iter() + .find_map(|(attacker_id, target)| (*attacker_id == *attacker).then_some(*target)) + .unwrap_or(crate::game::combat::AttackTarget::Player(*defending_player)); + (*attacker, target) + }) + .collect(); + Some(singleton_attack_events(*defending_player, matching_attacks)) +} + fn contextual_batched_trigger_event( state: &GameState, event: &GameEvent, @@ -1440,6 +1479,24 @@ fn collect_matching_triggers_inner( .into_iter() .map(|trigger_event| vec![trigger_event]) .collect() + } else if matches!(trig_def.mode, TriggerMode::YouAttack) + && ability.has_event_source_force_block_recursive() + { + let matching = super::trigger_matchers::matching_you_attack_pairs( + event, + trig_def, + &source_context, + state, + ); + match event { + GameEvent::AttackersDeclared { + defending_player, .. + } => singleton_attack_events(*defending_player, matching), + _ => Vec::new(), + } + .into_iter() + .map(|trigger_event| vec![trigger_event]) + .collect() } else if matches!(trig_def.mode, TriggerMode::Blocks) { super::trigger_matchers::matching_block_events( event, @@ -1499,6 +1556,16 @@ fn collect_matching_triggers_inner( .into_iter() .map(|trigger_event| vec![trigger_event]) .collect() + } else if ability.has_event_source_force_block_recursive() { + // CR 603.2 + CR 608.2c: every event-source force-block branch + // owns one unambiguous attacker. The ordinary attack modes + // above narrow by their own matcher; this fallback preserves + // singleton identity for any additional attack trigger mode. + split_attack_event_into_singletons(event) + .unwrap_or_else(|| vec![event.clone()]) + .into_iter() + .map(|trigger_event| vec![trigger_event]) + .collect() } else { vec![vec![event.clone()]] }; @@ -5920,6 +5987,25 @@ pub(crate) fn take_pending_trigger_event_batch( } } +/// CR 603.3d + CR 509.1c: Extract the one unambiguous attacker named by a +/// singleton attack trigger. Trigger construction completes before priority, so +/// this reads the same live incarnation that the stack-push binding observed. +fn event_attacker_from_trigger_event( + state: &GameState, + trigger_event: Option<&GameEvent>, +) -> Option { + let GameEvent::AttackersDeclared { attacker_ids, .. } = trigger_event? else { + return None; + }; + let [attacker_id] = attacker_ids.as_slice() else { + return None; + }; + state + .objects + .get(attacker_id) + .map(ObjectIncarnationRef::from_object) +} + /// CR 603.3 + CR 603.3c + CR 603.3d: Push a pending trigger to the stack with /// its event batch keyed by entry id. Returns the new entry's `ObjectId` so /// callers can stash it in `state.pending_trigger_entry` when the entry is @@ -6026,6 +6112,11 @@ pub(crate) fn push_pending_trigger_to_stack_with_event_batch( if let Some(origin) = may_trigger_origin { ability.set_may_trigger_origin_recursive(origin); } + // CR 400.7 + CR 509.1c: Bind either grammatical named-attacker form at + // stack creation. In particular, "that Wolf" is the exact object from the + // narrowed attack-trigger event, not the triggered ability's source. + let event_attacker = event_attacker_from_trigger_event(state, trigger_event.as_ref()); + ability.bind_force_block_attacker_recursive(event_attacker); seed_batched_attack_parent_targets(&mut ability, trigger_event.as_ref()); seed_event_context_parent_targets(&mut ability, trigger_event.as_ref()); @@ -6193,6 +6284,25 @@ fn assign_pending_trigger_entry_ability( entry_id: ObjectId, source_ability: &ResolvedAbility, ) -> bool { + let trigger_event = state + .stack + .iter() + .rev() + .find(|entry| entry.id == entry_id) + .and_then(|entry| match &entry.kind { + StackEntryKind::TriggeredAbility { trigger_event, .. } => trigger_event.as_ref(), + _ => None, + }); + let mut assigned_ability = source_ability.clone(); + // CR 603.3d: Target/mode construction replaces the provisional stack + // ability before any player receives priority. Rebind event-referential + // force-blocks here so that replacement cannot discard the stack-time + // identity for "that Wolf". + assigned_ability.bind_force_block_attacker_recursive(event_attacker_from_trigger_event( + state, + trigger_event, + )); + let Some(entry) = state .stack .iter_mut() @@ -6204,7 +6314,7 @@ fn assign_pending_trigger_entry_ability( let ability = entry .ability_mut() .expect("pending_trigger_entry must reference a TriggeredAbility stack entry"); - *ability = source_ability.clone(); + *ability = assigned_ability; true } @@ -8107,6 +8217,16 @@ fn evaluate_trigger_condition_with_source( TriggerCondition::SourceEnteredThisTurn => source_context.is_some_and(|source| { source.source_read(state).entered_battlefield_turn() == Some(state.turn_number) }), + // CR 400.7 + CR 508.1 + CR 603.4: the condition is tied to the + // trigger source's observed incarnation, not its storage id. It is + // checked both when the trigger is created and when it resolves. + TriggerCondition::SourceAttackedThisCombat => source_context.is_some_and(|source| { + state.combat.as_ref().is_some_and(|combat| { + combat + .attacking_incarnations_this_combat + .contains(&source.identity.reference) + }) + }), TriggerCondition::EchoDue => { source_context.is_some_and(|source| source.source_read(state).echo_due()) } @@ -10049,6 +10169,11 @@ pub(super) fn build_triggered_ability_from_context( resolved.set_scoped_player_recursive(state.active_player); } resolved.set_trigger_source_recursive(source_context.clone()); + // CR 400.7 + CR 509.1c: Source-referential force-block instructions + // latch their source incarnation as soon as the triggered ability is + // instantiated. EventSource remains intentionally unbound until the + // individual attack event is attached below. + resolved.bind_force_block_attacker_recursive(None); if let Some(definition_ref) = definition_ref { resolved.set_trigger_definition_ref_recursive(definition_ref.clone()); } @@ -12083,6 +12208,170 @@ pub mod tests { ); } + /// CR 603.3 + CR 508.1 + CR 509.1c: Tolsimir's source-referential + /// intervening-if and its event-referential "that Wolf" are different + /// identities. The parsed trigger must bind the attacking Wolf at stack + /// creation and force the chosen opposing creature to block that Wolf, not + /// Tolsimir. If Tolsimir did not attack, no trigger is created. + #[test] + fn tolsimir_attack_trigger_runs_from_parser_through_stack_to_block_legality() { + use crate::game::combat::{validate_blockers, AttackTarget, AttackerInfo, CombatState}; + use crate::types::actions::GameAction; + + const ORACLE: &str = "Whenever a Wolf you control attacks, if Tolsimir, Midnight's Light attacked this combat, target creature an opponent controls blocks that Wolf this combat if able."; + + let parsed = + crate::parser::oracle_trigger::parse_trigger_line(ORACLE, "Tolsimir, Midnight's Light"); + + let mut no_tolsimir_attack = setup(); + let inactive_tolsimir = make_creature( + &mut no_tolsimir_attack, + PlayerId(0), + "Tolsimir, Midnight's Light", + 3, + 4, + ); + let inactive_wolf = make_creature(&mut no_tolsimir_attack, PlayerId(0), "Wolf", 2, 2); + { + let wolf = no_tolsimir_attack + .objects + .get_mut(&inactive_wolf) + .expect("wolf exists"); + wolf.card_types.subtypes.push("Wolf".to_string()); + wolf.base_card_types = wolf.card_types.clone(); + } + let inactive = no_tolsimir_attack + .objects + .get_mut(&inactive_tolsimir) + .expect("Tolsimir exists"); + inactive.trigger_definitions.push(parsed.clone()); + std::sync::Arc::make_mut(&mut inactive.base_trigger_definitions).push(parsed.clone()); + no_tolsimir_attack.layers_dirty.mark_full(); + no_tolsimir_attack.combat = Some(CombatState { + attackers: vec![AttackerInfo::new( + inactive_wolf, + AttackTarget::Player(PlayerId(1)), + PlayerId(1), + )], + ..CombatState::default() + }); + let inactive_event = GameEvent::AttackersDeclared { + attacker_ids: vec![inactive_wolf], + defending_player: PlayerId(1), + attacks: vec![(inactive_wolf, AttackTarget::Player(PlayerId(1)))], + }; + assert!( + collect_pending_triggers(&mut no_tolsimir_attack, &[inactive_event]).is_empty(), + "the intervening-if prevents the trigger when Tolsimir did not attack" + ); + + let mut state = setup(); + let tolsimir = make_creature(&mut state, PlayerId(0), "Tolsimir, Midnight's Light", 3, 4); + let wolf = make_creature(&mut state, PlayerId(0), "First Wolf", 2, 2); + let second_wolf = make_creature(&mut state, PlayerId(0), "Second Wolf", 2, 2); + let blocker = make_creature(&mut state, PlayerId(1), "Opponent Bear", 2, 2); + let second_blocker = make_creature(&mut state, PlayerId(1), "Opponent Elk", 2, 2); + for wolf_id in [wolf, second_wolf] { + let wolf = state.objects.get_mut(&wolf_id).expect("wolf exists"); + wolf.card_types.subtypes.push("Wolf".to_string()); + wolf.base_card_types = wolf.card_types.clone(); + } + let source = state.objects.get_mut(&tolsimir).expect("Tolsimir exists"); + source.trigger_definitions.push(parsed.clone()); + std::sync::Arc::make_mut(&mut source.base_trigger_definitions).push(parsed); + state.layers_dirty.mark_full(); + let attacking_incarnations_this_combat = [tolsimir, wolf, second_wolf] + .into_iter() + .map(|id| ObjectIncarnationRef::from_object(&state.objects[&id])) + .collect(); + state.combat = Some(CombatState { + attackers: vec![ + AttackerInfo::new(tolsimir, AttackTarget::Player(PlayerId(1)), PlayerId(1)), + AttackerInfo::new(wolf, AttackTarget::Player(PlayerId(1)), PlayerId(1)), + AttackerInfo::new(second_wolf, AttackTarget::Player(PlayerId(1)), PlayerId(1)), + ], + attacking_incarnations_this_combat, + ..CombatState::default() + }); + let attack_event = GameEvent::AttackersDeclared { + attacker_ids: vec![wolf, second_wolf], + defending_player: PlayerId(1), + attacks: vec![ + (wolf, AttackTarget::Player(PlayerId(1))), + (second_wolf, AttackTarget::Player(PlayerId(1))), + ], + }; + assert!( + check_trigger_condition( + &state, + &TriggerCondition::SourceAttackedThisCombat, + PlayerId(0), + Some(tolsimir), + Some(&attack_event), + ), + "Tolsimir's observed incarnation attacked during this combat" + ); + let pending = collect_pending_triggers(&mut state, std::slice::from_ref(&attack_event)); + assert_eq!( + pending.len(), + 2, + "each matching Wolf receives an individual event-bound trigger" + ); + for (attacker, target) in [(wolf, blocker), (second_wolf, second_blocker)] { + let singleton_event = GameEvent::AttackersDeclared { + attacker_ids: vec![attacker], + defending_player: PlayerId(1), + attacks: vec![(attacker, AttackTarget::Player(PlayerId(1)))], + }; + process_triggers(&mut state, &[singleton_event]); + let waiting = crate::game::engine::begin_pending_trigger_target_selection(&mut state) + .expect("target selection setup must succeed") + .expect("Tolsimir trigger must request its target"); + let WaitingFor::TriggerTargetSelection { target_slots, .. } = &waiting else { + panic!("expected trigger target selection, got {waiting:?}"); + }; + assert_eq!( + target_slots.len(), + 1, + "the parsed target survives to stack setup" + ); + assert!( + target_slots[0] + .legal_targets + .contains(&TargetRef::Object(target)), + "production target selection must expose the chosen opposing creature" + ); + state.waiting_for = waiting; + crate::game::engine::apply( + &mut state, + PlayerId(0), + GameAction::ChooseTarget { + target: Some(TargetRef::Object(target)), + }, + ) + .expect("production target choice must succeed"); + } + let mut resolution_events = Vec::new(); + crate::game::stack::resolve_top(&mut state, &mut resolution_events); + crate::game::stack::resolve_top(&mut state, &mut resolution_events); + // CR 613.1: ForceBlock installs a transient continuous effect; blocker + // validation reads its layer-applied static definition. + crate::game::layers::flush_layers(&mut state); + + assert!( + validate_blockers(&state, &[]).is_err(), + "the selected creature must block the event Wolf if able" + ); + assert!( + validate_blockers(&state, &[(blocker, wolf), (second_blocker, second_wolf)]).is_ok(), + "each selected creature must block its own event Wolf" + ); + assert!( + validate_blockers(&state, &[(blocker, second_wolf), (second_blocker, wolf)]).is_err(), + "the two event-attacker bindings cannot cross" + ); + } + /// Places a battlefield commander object with the given owner/controller. fn make_commander(state: &mut GameState, owner: PlayerId, controller: PlayerId) -> ObjectId { let id = make_creature(state, owner, "Test Commander", 3, 3); @@ -27592,6 +27881,7 @@ pub mod tests { timestamp: 1, duration: Duration::Permanent, affected: TargetFilter::SpecificObject { id: recipient }, + affected_recipient: None, modifications: vec![ContinuousModification::AddKeyword { keyword: Keyword::Suspend { count: 0, @@ -27687,6 +27977,7 @@ pub mod tests { affected: TargetFilter::SpecificObject { id: exile_recipient, }, + affected_recipient: None, modifications: vec![ContinuousModification::AddKeyword { keyword: Keyword::Suspend { count: 0, @@ -27827,6 +28118,7 @@ pub mod tests { timestamp: 1 + i as u64, duration: Duration::Permanent, affected: TargetFilter::SpecificObject { id: card }, + affected_recipient: None, modifications: vec![ContinuousModification::AddKeyword { keyword: Keyword::Suspend { count: 0, diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index 36a6fcf6f0..f7645c1526 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -25,6 +25,7 @@ use crate::parser::oracle_ir::diagnostic::OracleDiagnostic; use crate::parser::oracle_nom::bridge::{nom_on_lower, nom_parse_lower, split_once_on_lower}; use crate::parser::oracle_nom::primitives as nom_primitives; use crate::parser::oracle_nom::quantity as nom_quantity; +use crate::parser::oracle_nom::target as nom_target; use crate::parser::oracle_static::parse_activated_ability_cost_head; use crate::parser::oracle_static::{ parse_continuous_modifications, parse_may_look_at_face_down_filter, @@ -34,11 +35,11 @@ use crate::types::ability::{ AbilityCost, AbilityDefinition, AbilityKind, BounceSelection, CardSelectionMode, CategoryChooserScope, ChoiceType, Chooser, ContinuousModification, ControlWindow, ControllerRef, CopyRetargetPermission, CounterAdjustment, DigSource, DoorLockOp, Duration, - Effect, EffectScope, FaceDownProfile, FilterProp, GrantedAbilityScope, LibraryPosition, - MultiTargetSpec, OutsideGameSourcePool, PlayerScope, PreventionAmount, PreventionScope, PtStat, - PtValue, QuantityExpr, QuantityRef, ReassembleControlMode, SearchSelectionConstraint, - StaticDefinition, StickerTicketCostPayment, TapStateChange, TargetFilter, TargetSelectionMode, - TypeFilter, TypedFilter, ZoneOwner, + Effect, EffectScope, FaceDownProfile, FilterProp, ForceBlockAttackerRef, GrantedAbilityScope, + LibraryPosition, MultiTargetSpec, OutsideGameSourcePool, PlayerScope, PreventionAmount, + PreventionScope, PtStat, PtValue, QuantityExpr, QuantityRef, ReassembleControlMode, + SearchSelectionConstraint, StaticDefinition, StickerTicketCostPayment, TapStateChange, + TargetFilter, TargetSelectionMode, TypeFilter, TypedFilter, ZoneOwner, }; use crate::types::card_type::CoreType; use crate::types::phase::Phase; @@ -10343,18 +10344,9 @@ pub(super) fn parse_imperative_family_ast( // ── Combat-related ── - // CR 509.1g: "block [object] this turn/combat if able" - // Handles: "block this turn if able", "blocks ~ this turn if able", - // "blocks it this combat if able", "blocks this creature this turn if able" - "block" | "blocks" => { - if nom_primitives::scan_contains(lower, "this turn if able") - || nom_primitives::scan_contains(lower, "this combat if able") - { - Some(ImperativeFamilyAst::ForceBlock) - } else { - None - } - } + // CR 509.1c: the named attacker is a semantic referent, not a source-id + // inference. "that Wolf" means the narrowed attacking event subject. + "block" | "blocks" => parse_force_block_ast(lower, ctx), // CR 509.1c: "must be blocked [this turn] [if able]" "must" => { if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("must be blocked").parse(lower) { @@ -11847,6 +11839,87 @@ pub(super) fn lower_imperative_family_ast(ast: ImperativeFamilyAst) -> ParsedEff } } +/// CR 509.1c: Parse the force-block grammar after preserving the named attacker +/// axis. This is intentionally a small nom production rather than a text +/// substring check: the referent and duration survive lowering independently. +fn parse_force_block_ast(input: &str, ctx: &ParseContext) -> Option { + let (rest, _) = alt((tag::<_, _, OracleError<'_>>("block "), tag("blocks "))) + .parse(input) + .ok()?; + let parse_duration = |tail| { + all_consuming(terminated( + alt(( + value( + Duration::UntilEndOfTurn, + tag::<_, _, OracleError<'_>>("this turn if able"), + ), + value(Duration::UntilEndOfCombat, tag("this combat if able")), + )), + opt(tag(".")), + )) + .parse(tail) + .ok() + .map(|(_, duration)| duration) + }; + + if let Some(duration) = parse_duration(rest) { + return Some(ImperativeFamilyAst::ForceBlock { + attacker: None, + duration, + }); + } + if let Ok((tail, _)) = + alt((tag::<_, _, OracleError<'_>>("~ "), tag("this creature "))).parse(rest) + { + if let Some(duration) = parse_duration(tail) { + return Some(ImperativeFamilyAst::ForceBlock { + attacker: Some(ForceBlockAttackerRef::Source), + duration, + }); + } + } + if let Ok((tail, _)) = tag::<_, _, OracleError<'_>>("it ").parse(rest) { + if ctx.in_trigger && matches!(ctx.subject.as_ref(), Some(TargetFilter::Typed(_))) { + if let Some(duration) = parse_duration(tail) { + return Some(ImperativeFamilyAst::ForceBlock { + attacker: Some(ForceBlockAttackerRef::EventSource), + duration, + }); + } + } + } + // CR 603.2 + CR 506.3: an event-referential demonstrative needs a typed + // attacking-creature antecedent from a trigger. The shared nom production + // rejects arbitrary text and non-attacker nouns before this effect is built. + if ctx.in_trigger && matches!(ctx.subject.as_ref(), Some(TargetFilter::Typed(_))) { + if let Ok((tail, _referent)) = nom_target::parse_demonstrative_attacker_referent(rest) { + if let Some(duration) = parse_duration(tail) { + return Some(ImperativeFamilyAst::ForceBlock { + attacker: Some(ForceBlockAttackerRef::EventSource), + duration, + }); + } + } + } + + // Preserve the existing generic grammar: modifiers between "block(s)" and + // the duration still produce an attacker-agnostic requirement. Exact + // source/event referents above take precedence when their typed forms match. + if nom_primitives::scan_contains(rest, "this turn if able") { + Some(ImperativeFamilyAst::ForceBlock { + attacker: None, + duration: Duration::UntilEndOfTurn, + }) + } else if nom_primitives::scan_contains(rest, "this combat if able") { + Some(ImperativeFamilyAst::ForceBlock { + attacker: None, + duration: Duration::UntilEndOfCombat, + }) + } else { + None + } +} + fn lower_imperative_family_effect(ast: ImperativeFamilyAst) -> Effect { match ast { ImperativeFamilyAst::Structured(ast) => lower_imperative_ast(ast), @@ -11857,8 +11930,10 @@ fn lower_imperative_family_effect(ast: ImperativeFamilyAst) -> Effect { target: TargetFilter::Any, count: QuantityExpr::Fixed { value: 1 }, }, - ImperativeFamilyAst::ForceBlock => Effect::ForceBlock { + ImperativeFamilyAst::ForceBlock { attacker, duration } => Effect::ForceBlock { target: TargetFilter::Any, + attacker, + duration, }, ImperativeFamilyAst::ForceAttack { duration, diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index bd99b7ecd4..d8a40831aa 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -17885,7 +17885,7 @@ fn replace_target_with_parent(effect: &mut Effect) { // parent-bound PhaseIn ("untap target permanent, then phase it in") // is not silently skipped. | Effect::PhaseIn { target } - | Effect::ForceBlock { target } + | Effect::ForceBlock { target, .. } | Effect::ForceAttack { target, .. } if !matches!(target, TargetFilter::ParentTargetController) => { @@ -20061,7 +20061,7 @@ fn inject_subject_target(effect: &mut Effect, subject: &SubjectPhraseAst) { Effect::Connive { target, .. } | Effect::PhaseOut { target } | Effect::PhaseIn { target } - | Effect::ForceBlock { target } + | Effect::ForceBlock { target, .. } | Effect::ForceAttack { target, .. } | Effect::Suspect { target, .. } | Effect::Unsuspect { target, .. } @@ -24275,7 +24275,7 @@ fn rewrite_parent_targets_to_tracked_set(effect: &mut Effect) { // CR 702.26c: PhaseIn mirrors PhaseOut; expose its target to tracked-set // rewrites for symmetry so a parent-bound PhaseIn is not skipped. | Effect::PhaseIn { target } - | Effect::ForceBlock { target } + | Effect::ForceBlock { target, .. } | Effect::ForceAttack { target, .. } | Effect::CastCopyOfCard { target, .. } | Effect::CopyTokenOf { target, .. } @@ -24537,7 +24537,7 @@ pub(crate) fn each_target_filter_mut(effect: &mut Effect, f: &mut impl FnMut(&mu // its target filter too so generic anaphor/scope rewrites and the // self-disjunctive delayed-trigger rebind (CR 603.7c) reach it. | Effect::PhaseIn { target } - | Effect::ForceBlock { target } + | Effect::ForceBlock { target, .. } | Effect::ForceAttack { target, .. } | Effect::Draw { target, .. } | Effect::Discard { target, .. } diff --git a/crates/engine/src/parser/oracle_effect/subject.rs b/crates/engine/src/parser/oracle_effect/subject.rs index a74bf1f96d..07ca55d7ad 100644 --- a/crates/engine/src/parser/oracle_effect/subject.rs +++ b/crates/engine/src/parser/oracle_effect/subject.rs @@ -6803,7 +6803,12 @@ mod tests { "mass force-block must not request a target, got {:?}", def.target_prompt ); - let Effect::ForceBlock { target } = &*def.effect else { + let Effect::ForceBlock { + target, + attacker, + duration, + } = &*def.effect + else { panic!("expected ForceBlock, got {:?}", def.effect); }; let TargetFilter::Typed(filter) = target else { @@ -6811,6 +6816,8 @@ mod tests { }; assert_eq!(filter.controller, Some(ControllerRef::Opponent)); assert!(filter.type_filters.contains(&TypeFilter::Creature)); + assert_eq!(*attacker, None); + assert_eq!(*duration, Duration::UntilEndOfTurn); } /// CR 702.3b: the subjectless conjunct recognizer accepts every grammatical diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index 42728c7399..04bccb7427 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -19890,7 +19890,8 @@ fn force_block_targeted() { matches!( e, Effect::ForceBlock { - target: TargetFilter::Typed(_) + target: TargetFilter::Typed(_), + .. } ), "Expected ForceBlock with typed target, got {:?}", @@ -19898,6 +19899,25 @@ fn force_block_targeted() { ); } +#[test] +fn force_block_with_named_target_preserves_generic_requirement() { + // Hunt Down — retain the pre-existing generic grammar when the target + // creature is named between the block verb and duration. + let e = parse_effect("Target creature blocks target creature this turn if able."); + assert!( + matches!( + e, + Effect::ForceBlock { + target: TargetFilter::Typed(_), + attacker: None, + duration: Duration::UntilEndOfTurn, + } + ), + "Expected generic ForceBlock with turn duration, got {:?}", + e + ); +} + // ----------------------------------------------------------------------- // Item 3b: MustBeBlocked imperative (CR 509.1c) // ----------------------------------------------------------------------- @@ -20855,12 +20875,14 @@ fn static_must_be_blocked_still_routes_to_static_parser() { #[test] fn force_block_with_self_ref() { // "Target creature blocks ~ this turn if able" (e.g., Auriok Siege Sled) - let e = parse_effect("Target creature blocks ~ this turn if able"); + let e = parse_effect("Target creature blocks ~ this turn if able."); assert!( matches!( e, Effect::ForceBlock { - target: TargetFilter::Typed(_) + target: TargetFilter::Typed(_), + attacker: Some(crate::types::ability::ForceBlockAttackerRef::Source), + duration: Duration::UntilEndOfTurn, } ), "Expected ForceBlock with typed target, got {:?}", @@ -20888,12 +20910,19 @@ fn force_attack_you_this_combat_targets_creature() { #[test] fn force_block_blocks_it_this_combat() { // "target creature blocks it this combat if able" (e.g., Avalanche Tusker) - let e = parse_effect("Target creature blocks it this combat if able"); + let mut ctx = ParseContext { + in_trigger: true, + subject: Some(TargetFilter::Typed(TypedFilter::creature())), + ..ParseContext::default() + }; + let e = parse_effect_clause("Target creature blocks it this combat if able.", &mut ctx).effect; assert!( matches!( e, Effect::ForceBlock { - target: TargetFilter::Typed(_) + target: TargetFilter::Typed(_), + attacker: Some(crate::types::ability::ForceBlockAttackerRef::EventSource), + duration: Duration::UntilEndOfCombat, } ), "Expected ForceBlock with typed target, got {:?}", @@ -20901,6 +20930,32 @@ fn force_block_blocks_it_this_combat() { ); } +#[test] +fn force_block_that_triggered_creature_this_combat_preserves_event_referent() { + let mut ctx = ParseContext { + in_trigger: true, + subject: Some(TargetFilter::Typed(TypedFilter::creature())), + ..ParseContext::default() + }; + let e = parse_effect_clause( + "Target creature an opponent controls blocks that Wolf this combat if able", + &mut ctx, + ) + .effect; + assert!( + matches!( + e, + Effect::ForceBlock { + target: TargetFilter::Typed(_), + attacker: Some(crate::types::ability::ForceBlockAttackerRef::EventSource), + duration: Duration::UntilEndOfCombat, + } + ), + "Expected an event-bound combat force block, got {:?}", + e + ); +} + #[test] fn mass_forced_block_target_creature() { // "All creatures able to block target creature this turn do so" (Alluring Scent) diff --git a/crates/engine/src/parser/oracle_ir/ast.rs b/crates/engine/src/parser/oracle_ir/ast.rs index ca1a2a7a98..7337a906c1 100644 --- a/crates/engine/src/parser/oracle_ir/ast.rs +++ b/crates/engine/src/parser/oracle_ir/ast.rs @@ -4,10 +4,11 @@ use crate::types::ability::MultiTargetSpec; use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, ActivationRestriction, BounceSelection, CastingPermission, ControlWindow, ControllerRef, CopyRetargetPermission, CounterAdjustment, - CounterSourceRider, DoorLockOp, Duration, Effect, FaceDownProfile, LibraryPosition, - ManaProduction, ManaSpendRestriction, ModalSelectionConstraint, OutsideGameSourcePool, - PlayerFilter, PtStat, PtValue, QuantityExpr, SearchDestinationSplit, SearchSelectionConstraint, - SpellStackToGraveyardReplacement, StaticCondition, StaticDefinition, TargetFilter, + CounterSourceRider, DoorLockOp, Duration, Effect, FaceDownProfile, ForceBlockAttackerRef, + LibraryPosition, ManaProduction, ManaSpendRestriction, ModalSelectionConstraint, + OutsideGameSourcePool, PlayerFilter, PtStat, PtValue, QuantityExpr, SearchDestinationSplit, + SearchSelectionConstraint, SpellStackToGraveyardReplacement, StaticCondition, StaticDefinition, + TargetFilter, }; use crate::types::card_type::Supertype; use crate::types::counter::CounterType; @@ -531,8 +532,11 @@ pub(crate) enum ImperativeFamilyAst { Explore, /// CR 702.162a: Connive. Connive, - /// CR 509.1g: Block this turn if able. - ForceBlock, + /// CR 509.1c: Block this turn/combat if able. + ForceBlock { + attacker: Option, + duration: Duration, + }, /// CR 508.1d: Attack a required player this turn/combat if able. The /// `required_player` filter selects whom the forced attacker must attack — /// `TargetFilter::Controller` for "attacks you", or diff --git a/crates/engine/src/parser/oracle_nom/target.rs b/crates/engine/src/parser/oracle_nom/target.rs index 623ff4b3d8..1fd322b0cf 100644 --- a/crates/engine/src/parser/oracle_nom/target.rs +++ b/crates/engine/src/parser/oracle_nom/target.rs @@ -87,6 +87,52 @@ pub fn parse_type_phrase(input: &str) -> OracleResult<'_, TargetFilter> { Ok((rest, filter)) } +/// Parse a demonstrative that names the attacking object of a trigger event. +/// +/// This intentionally accepts only creature-compatible type phrases: "that +/// creature" and "that Wolf" can refer to an attacker, while "that artifact" +/// cannot be silently treated as the event attacker. Callers still own the +/// surrounding trigger-context gate; this combinator owns the typed noun phrase +/// and its required separator before the following grammar production. +pub fn parse_demonstrative_attacker_referent(input: &str) -> OracleResult<'_, TargetFilter> { + let (rest, _) = tag("that ").parse(input)?; + let (rest, filter) = parse_type_phrase(rest)?; + if !target_filter_can_name_attacker(&filter) { + return Err(oracle_err(input)); + } + let (rest, _) = space1.parse(rest)?; + Ok((rest, filter)) +} + +/// CR 506.3: an attacking object must be a creature. A printed creature +/// subtype ("Wolf") is creature-compatible even when the noun phrase omits +/// the word "creature". +fn target_filter_can_name_attacker(filter: &TargetFilter) -> bool { + let TargetFilter::Typed(typed) = filter else { + return false; + }; + typed.type_filters.iter().any(type_filter_can_name_attacker) +} + +fn type_filter_can_name_attacker(filter: &TypeFilter) -> bool { + match filter { + TypeFilter::Creature | TypeFilter::Subtype(_) => true, + TypeFilter::AnyOf(filters) => filters.iter().any(type_filter_can_name_attacker), + TypeFilter::Land + | TypeFilter::Artifact + | TypeFilter::Enchantment + | TypeFilter::Instant + | TypeFilter::Sorcery + | TypeFilter::Planeswalker + | TypeFilter::Battle + | TypeFilter::Kindred + | TypeFilter::Permanent + | TypeFilter::Card + | TypeFilter::Any + | TypeFilter::Non(_) => false, + } +} + /// Parse a "non" prefix: "non" or "non-" followed by implicit word boundary. fn parse_non_prefix(input: &str) -> OracleResult<'_, &str> { alt((tag("non-"), tag("non"))).parse(input) @@ -1475,4 +1521,16 @@ mod tests { "both ability spellings must fold to exactly one StackAbility: {filters:?}" ); } + + #[test] + fn demonstrative_attacker_referent_is_typed_and_creature_compatible() { + let (rest, wolf) = parse_demonstrative_attacker_referent("that Wolf this combat if able") + .expect("creature subtype can name an attacker"); + assert_eq!(rest, "this combat if able"); + assert!(matches!(wolf, TargetFilter::Typed(_))); + assert!( + parse_demonstrative_attacker_referent("that artifact this combat if able").is_err(), + "a noncreature noun cannot become an event attacker" + ); + } } diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index db4d715539..1d50203730 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -5174,6 +5174,20 @@ fn extract_if_condition_with_card_name( ); } + // CR 400.7 + CR 508.1 + CR 603.4: source-combat history is a distinct + // grammar production, not a card-text table entry. The scanner applies the + // nom production at word boundaries so a leading intervening-if is retained + // while later sentence-local conditionals remain outside this function. + if let Some((prefix, _, rest)) = scan_preceded(&lower, |i| { + tag::<_, _, OracleError<'_>>("if ~ attacked this combat").parse(i) + }) { + let clause_len = lower.len() - prefix.len() - rest.len(); + return ( + strip_condition_clause(text, prefix.len(), clause_len), + Some(TriggerCondition::SourceAttackedThisCombat), + ); + } + // Simple pattern→condition extractions (no dynamic parsing or guards needed). if let Some(result) = try_extract_simple_condition( &tp, diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index a2c78fd3bc..57cadbb3cb 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -548,6 +548,28 @@ fn intervening_if_source_attacked_this_turn_populates_condition() { assert!(taigam.execute.is_some()); } +#[test] +fn tolsimir_midnights_light_preserves_combat_source_and_event_attacker_axes() { + let trigger = parse_trigger_line( + "Whenever a Wolf you control attacks, if Tolsimir, Midnight's Light attacked this combat, \ + target creature an opponent controls blocks that Wolf this combat if able.", + "Tolsimir, Midnight's Light", + ); + assert_eq!( + trigger.condition, + Some(TriggerCondition::SourceAttackedThisCombat), + "the intervening-if is combat-scoped and source-incarnation-bound" + ); + assert!(matches!( + trigger.execute.as_deref().map(|ability| &*ability.effect), + Some(Effect::ForceBlock { + attacker: Some(crate::types::ability::ForceBlockAttackerRef::EventSource), + duration: Duration::UntilEndOfCombat, + .. + }) + )); +} + #[test] fn intervening_if_source_attacked_or_blocked_this_turn_populates_condition() { // CR 508.1 + CR 509.1 + CR 603.4: the "attacked or blocked" sibling of the diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 0b53a26cd1..5a703cc27b 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -2465,6 +2465,20 @@ pub enum Duration { Permanent, } +/// The attacker named by a force-block instruction. +/// +/// This intentionally has only exact single-object referents. A filter would +/// allow resolution to reselect a different attacker after the trigger event; +/// CR 400.7 requires the event object, not a later object sharing its id. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "type")] +pub enum ForceBlockAttackerRef { + /// "blocks this creature if able" / Provoke's source-referential form. + Source, + /// "blocks that Wolf if able" from a narrowed attack trigger event. + EventSource, +} + // --------------------------------------------------------------------------- // Game restriction system — composable runtime restrictions // --------------------------------------------------------------------------- @@ -11501,10 +11515,18 @@ pub enum Effect { #[serde(default = "default_target_filter_any")] target: TargetFilter, }, - /// CR 509.1g: Target creature must block this turn if able. + /// CR 509.1c: Target creature must block this turn/combat if able. ForceBlock { #[serde(default = "default_target_filter_any")] target: TargetFilter, + /// The exact grammatical attacker referent, if the instruction names + /// one. `None` remains the generic "blocks this turn if able" form. + #[serde(default, skip_serializing_if = "Option::is_none")] + attacker: Option, + /// `this combat` expires at end of combat; legacy payloads without a + /// duration retain the established `this turn` lifetime. + #[serde(default = "default_duration_until_end_of_turn")] + duration: Duration, }, /// CR 508.1d: Target creature must attack the required player this turn/combat if able. ForceAttack { @@ -18073,6 +18095,9 @@ pub enum TriggerCondition { /// CR 400.7 + CR 603.4: True when the source permanent entered the /// battlefield this turn. SourceEnteredThisTurn, + /// CR 400.7 + CR 508.1 + CR 603.4: True only when this exact source + /// incarnation attacked during the current combat. + SourceAttackedThisCombat, /// CR 702.30a: Echo intervening-if for a permanent that has not yet had /// its next-controller-upkeep echo payment handled. EchoDue, @@ -21320,6 +21345,12 @@ pub struct ResolvedAbility { /// `ability_index` remains presentation/compatibility only. #[serde(default, skip_serializing_if = "Option::is_none")] pub trigger_definition_ref: Option, + /// CR 400.7 + CR 509.1c: Exact attacker selected by a source- or + /// event-source-referential force-block instruction. This is bound when the + /// triggered ability is put on the stack, before targets are chosen; it is + /// deliberately separate from the parsed grammatical selector on `Effect`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub force_block_attacker: Option, pub controller: PlayerId, /// CR 109.5: The controller of the spell or ability before any /// resolution-time player-scope iteration rebinds the acting player. @@ -21607,6 +21638,7 @@ impl ResolvedAbility { source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, modal: None, mode_abilities: Vec::new(), parent_target_missing_reason: None, @@ -21670,6 +21702,79 @@ impl ResolvedAbility { } } + /// Preserve the exact named attacker through target selection and every + /// continuation of a triggered force-block instruction. The parser carries + /// only its grammatical selector; this stores the one event-time object + /// identity that resolution is permitted to use. + pub fn bind_force_block_attacker_recursive( + &mut self, + event_attacker: Option, + ) { + if let Effect::ForceBlock { + attacker: Some(selector), + .. + } = &self.effect + { + self.force_block_attacker = match selector { + ForceBlockAttackerRef::Source => self + .trigger_source + .as_ref() + .map(|source| source.identity.reference), + ForceBlockAttackerRef::EventSource => event_attacker, + }; + } + if let Some(sub) = self.sub_ability.as_mut() { + sub.bind_force_block_attacker_recursive(event_attacker); + } + if let Some(else_branch) = self.else_ability.as_mut() { + else_branch.bind_force_block_attacker_recursive(event_attacker); + } + } + + /// Whether any resolution branch names the attacker from its triggering + /// attack event. Such an ability must receive one singleton attack event + /// per pending trigger; a plural event has no single "that Wolf" referent. + pub fn has_event_source_force_block_recursive(&self) -> bool { + matches!( + &self.effect, + Effect::ForceBlock { + attacker: Some(ForceBlockAttackerRef::EventSource), + .. + } + ) || self + .sub_ability + .as_ref() + .is_some_and(|sub| sub.has_event_source_force_block_recursive()) + || self + .else_ability + .as_ref() + .is_some_and(|else_branch| else_branch.has_event_source_force_block_recursive()) + } + + /// Bind only source-referential force-block effects at the shared stack + /// boundary. Event-source effects are intentionally left untouched because + /// their authority is the singleton triggering event, not the stack entry's + /// source object. + pub fn bind_force_block_source_recursive(&mut self, source: Option) { + if source.is_some() + && matches!( + &self.effect, + Effect::ForceBlock { + attacker: Some(ForceBlockAttackerRef::Source), + .. + } + ) + { + self.force_block_attacker = source; + } + if let Some(sub) = self.sub_ability.as_mut() { + sub.bind_force_block_source_recursive(source); + } + if let Some(else_branch) = self.else_ability.as_mut() { + else_branch.bind_force_block_source_recursive(source); + } + } + /// Clears provenance that distinguishes otherwise identical triggered /// abilities for structural comparison. This deliberately clears the /// complete owned authorities together; retaining either a source context @@ -21679,6 +21784,7 @@ impl ResolvedAbility { self.source_incarnation = None; self.trigger_source = None; self.trigger_definition_ref = None; + self.force_block_attacker = None; if let Some(sub) = self.sub_ability.as_mut() { sub.clear_trigger_identity_recursive(); } @@ -23628,6 +23734,30 @@ mod tests { assert_eq!(effect, deserialized); } + #[test] + fn force_block_serde_preserves_named_attacker_and_legacy_defaults() { + let named = Effect::ForceBlock { + target: TargetFilter::Typed(TypedFilter::creature()), + attacker: Some(ForceBlockAttackerRef::EventSource), + duration: Duration::UntilEndOfCombat, + }; + let json = serde_json::to_string(&named).expect("serialize named force block"); + assert_eq!( + serde_json::from_str::(&json).expect("deserialize named force block"), + named + ); + + let legacy = r#"{"type":"ForceBlock","target":{"type":"Any"}}"#; + assert_eq!( + serde_json::from_str::(legacy).expect("deserialize legacy force block"), + Effect::ForceBlock { + target: TargetFilter::Any, + attacker: None, + duration: Duration::UntilEndOfTurn, + } + ); + } + #[test] fn effect_cleanup_typed_fields_roundtrip() { let effect = Effect::Cleanup { diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 5efa4f71d6..c94d3a817a 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -6263,8 +6263,11 @@ pub enum CombatTaxPending { /// band for blocking (CR 702.22h). bands: Vec>, }, + /// CR 400.7 + CR 509.1d: Both sides of each proposed block pair are + /// snapshotted. A blocker or attacker that leaves and re-enters during the + /// payment pause is a different object and cannot receive the locked quote. Block { - assignments: Vec<(ObjectId, ObjectId)>, + assignments: Vec<(ObjectIncarnationRef, ObjectIncarnationRef)>, }, } @@ -12915,6 +12918,11 @@ pub struct TransientContinuousEffect { pub timestamp: u64, pub duration: Duration, pub affected: TargetFilter, + /// CR 400.7: A one-shot effect that resolved on a specific target must not + /// apply to a later incarnation that reuses the same storage id. `None` + /// preserves the dynamic affected-set semantics required by general TCEs. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub affected_recipient: Option, pub modifications: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] pub condition: Option, @@ -16608,6 +16616,7 @@ impl GameState { timestamp, duration, affected, + affected_recipient: None, modifications, condition, duration_subject: None, @@ -16617,6 +16626,19 @@ impl GameState { id } + /// Bind a transient effect to the exact recipient resolved by a one-shot + /// instruction. Dynamic effects intentionally leave this unset. + pub fn set_transient_affected_recipient(&mut self, id: u64, recipient: ObjectIncarnationRef) { + if let Some(tce) = self + .transient_continuous_effects + .iter_mut() + .find(|tce| tce.id == id) + { + tce.affected_recipient = Some(recipient); + self.layers_dirty.mark_full(); + } + } + /// CR 611.2b + CR 110.5d: bind a target-relative `ForAsLongAs` duration to a /// concrete object resolved at effect-resolution time, on the TCE that /// [`Self::add_transient_continuous_effect`] just created (addressed by its diff --git a/crates/engine/src/types/statics.rs b/crates/engine/src/types/statics.rs index b272858180..e2f8f748ec 100644 --- a/crates/engine/src/types/statics.rs +++ b/crates/engine/src/types/statics.rs @@ -10,7 +10,7 @@ use super::ability::{ QuantityRef, TargetFilter, }; use super::events::ActivatedAbilityKind; -use super::identifiers::ObjectId; +use super::identifiers::ObjectIncarnationRef; use super::keywords::{Keyword, KeywordKind}; use super::mana::{ManaColor, ManaCost, SpecialAction, StepEndManaAction}; use super::phase::Phase; @@ -1158,9 +1158,10 @@ pub enum StaticMode { /// of the attacker that must be blocked. Data-carrying variant — not /// registry-registered (see `coverage::is_data_carrying_static`); enforced by /// direct pattern-match in `combat.rs` declare-blockers validation. The - /// `ObjectId` is stable for the end-of-turn lifetime of the granting effect. + /// incarnation reference prevents an attacker that left and re-entered from + /// inheriting the old combat requirement (CR 400.7). MustBlockAttacker { - attacker: ObjectId, + attacker: ObjectIncarnationRef, }, CantDraw { who: ProhibitionScope, diff --git a/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs b/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs index fa4ddab48b..c3cb0345ab 100644 --- a/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs +++ b/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs @@ -155,6 +155,7 @@ fn make_grant_ability(controller: PlayerId, source: ObjectId) -> ResolvedAbility source_incarnation: None, trigger_source: None, trigger_definition_ref: None, + force_block_attacker: None, targets: vec![], kind: AbilityKind::Activated, sub_ability: None, diff --git a/crates/phase-ai/src/search.rs b/crates/phase-ai/src/search.rs index e3ad186de5..523e20ef9c 100644 --- a/crates/phase-ai/src/search.rs +++ b/crates/phase-ai/src/search.rs @@ -2787,7 +2787,11 @@ pub(crate) fn deterministic_choice( &config.profile, Some(valid_block_targets), ); - return Some(GameAction::DeclareBlockers { assignments }); + return Some(engine::game::combat::complete_blocker_proposal( + state, + ai_player, + &assignments, + )); } return Some(GameAction::DeclareBlockers { assignments: Vec::new(), @@ -2844,7 +2848,11 @@ fn deterministic_combat_choice( profile, Some(valid_block_targets), ); - return Some(GameAction::DeclareBlockers { assignments }); + return Some(engine::game::combat::complete_blocker_proposal( + state, + ai_player, + &assignments, + )); } return Some(GameAction::DeclareBlockers { assignments: Vec::new(),