diff --git a/client/src/adapter/types.ts b/client/src/adapter/types.ts index 1eb37bf59d..cff6659dda 100644 --- a/client/src/adapter/types.ts +++ b/client/src/adapter/types.ts @@ -949,6 +949,14 @@ export type SearchDestinationSplit = { rest_destination: Zone; }; +// CR 107.1a/b: the engine-published contract for a choice whose answer the +// player types instead of picking from `options`. Mirrored from Rust +// `ability::FreeEntry`. `min`/`max` are INCLUSIVE and are the same bounds +// `ChoiceType::accepts_free_entry_answer` enforces — the client renders and +// bounds its input from these values and must never restate them, or it becomes +// a second authority that can reject what the engine accepts. +export type FreeEntry = { kind: "Number"; min: number; max: number }; + // ── Game Object ────────────────────────────────────────────────────────── /** @@ -1721,7 +1729,7 @@ export type WaitingFor = | { type: "TriggerTargetSelection"; data: { player: PlayerId; trigger_controller?: PlayerId; trigger_event?: GameEvent; trigger_events?: GameEvent[]; target_slots: TargetSelectionSlot[]; mode_labels?: (string | null)[]; target_constraints?: TargetSelectionConstraint[]; selection: TargetSelectionProgress; source_id?: ObjectId; description?: string } } | { type: "BetweenGamesSideboard"; data: { player: PlayerId; game_number: number; score: MatchScore; min_main_deck_size: number; max_sideboard_size: number | null } } | { type: "BetweenGamesChoosePlayDraw"; data: { player: PlayerId; game_number: number; score: MatchScore } } - | { type: "NamedChoice"; data: { player: PlayerId; choice_type: string | Record; options: string[]; source?: { prompt: { identity: unknown; controller: PlayerId; display_name: string }; binding: "ResolutionContext" | "ExactObjectAndResolution" }; persist_player?: PlayerId } } + | { type: "NamedChoice"; data: { player: PlayerId; choice_type: string | Record; options: string[]; source?: { prompt: { identity: unknown; controller: PlayerId; display_name: string }; binding: "ResolutionContext" | "ExactObjectAndResolution" }; persist_player?: PlayerId; free_entry?: FreeEntry } } | { type: "OpponentGuess"; data: { player: PlayerId; options: string[]; choice_type: string | Record; source: { prompt: { identity: unknown; controller: PlayerId; display_name: string } }; proposition_truth?: boolean } } | { type: "SpellbookDraft"; data: { player: PlayerId; source_id: ObjectId; options: string[]; destination: Zone; tapped?: boolean } } | { type: "DamageSourceChoice"; data: { player: PlayerId; source_filter: TargetFilter; options: ObjectId[] } } diff --git a/client/src/components/modal/NamedChoiceModal.tsx b/client/src/components/modal/NamedChoiceModal.tsx index edad299bc2..252569429f 100644 --- a/client/src/components/modal/NamedChoiceModal.tsx +++ b/client/src/components/modal/NamedChoiceModal.tsx @@ -11,7 +11,7 @@ import { getPlayerDisplayName, useMultiplayerStore, } from "../../stores/multiplayerStore.ts"; -import type { PlayerId, WaitingFor } from "../../adapter/types.ts"; +import type { FreeEntry, PlayerId, WaitingFor } from "../../adapter/types.ts"; type OptionChoice = Extract< WaitingFor, @@ -54,9 +54,75 @@ export function NamedChoiceModal({ data }: { data: OptionChoice["data"] }) { if (typeKey === "CardName") { return ; } + // CR 107.1a/b: the engine publishes a free-entry contract when the answer is + // typed rather than picked from `options`. Its PRESENCE — not any reading of + // `choice_type`'s serialized shape — selects the numeric form; an enumerated + // choice carries no contract and keeps its grid. + const freeEntry = "free_entry" in data ? data.free_entry : undefined; + if (freeEntry?.kind === "Number") { + return ; + } return ; } +/** CR 107.1a/b: free-entry numeric prompt, rendered and bounded entirely from + * the engine's published contract. + * + * The engine validates the submitted answer authoritatively + * (`ChoiceType::accepts_free_entry_answer`) against the same bounds it + * published here, so this check can only ever agree with it. Deriving the + * bounds locally instead — from the choice type's shape, or from a hard-coded + * numeric ceiling — would make the client a second authority, free to reject a + * value the engine accepts and to drift when the engine's domain changes. */ +function NumberEntry({ contract }: { contract: FreeEntry }) { + const { min, max } = contract; + const { t } = useTranslation("game"); + const dispatch = useGameDispatch(); + const [value, setValue] = useState(String(min)); + const inputRef = useRef(null); + + useEffect(() => { + inputRef.current?.focus(); + inputRef.current?.select(); + }, []); + + const parsed = /^\d+$/.test(value.trim()) ? Number(value.trim()) : null; + const valid = + parsed !== null && + Number.isSafeInteger(parsed) && + parsed >= min && + parsed <= max; + + const confirm = useCallback(() => { + if (valid) { + dispatch({ type: "ChooseOption", data: { choice: String(parsed) } }); + } + }, [dispatch, parsed, valid]); + + return ( + } + > + setValue(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter" && valid) { + e.preventDefault(); + confirm(); + } + }} + className="w-40 rounded-lg border-2 border-gray-600 bg-gray-900/90 px-4 py-3 text-center text-xl text-white outline-none transition focus:border-cyan-400" + /> + + ); +} + function CardNameSearch() { const { t } = useTranslation("game"); const dispatch = useGameDispatch(); diff --git a/client/src/components/modal/__tests__/NamedChoiceModal.test.tsx b/client/src/components/modal/__tests__/NamedChoiceModal.test.tsx index 2be6af908b..9773750606 100644 --- a/client/src/components/modal/__tests__/NamedChoiceModal.test.tsx +++ b/client/src/components/modal/__tests__/NamedChoiceModal.test.tsx @@ -38,4 +38,75 @@ describe("NamedChoiceModal", () => { data: { choice: "Blue" }, }); }); + + // CR 107.1a/b. The engine publishes `free_entry` for a choice whose answer is + // typed rather than picked, and enforces exactly those bounds. These pin that + // the modal RENDERS the contract rather than re-deriving one: the numeric form + // is selected by the contract's presence, and the bounds it enforces are the + // ones it was handed. + describe("free-entry number contract", () => { + const numberChoice = (max: number): NamedChoiceData => ({ + player: 0, + choice_type: { NumberRange: { min: 2 } }, + options: [], + free_entry: { kind: "Number", min: 2, max }, + }); + + it("submits a value inside the published range", () => { + render(); + + fireEvent.change(screen.getByRole("textbox"), { + target: { value: "1000000" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Confirm" })); + + expect(dispatchMock).toHaveBeenCalledWith({ + type: "ChooseOption", + data: { choice: "1000000" }, + }); + }); + + // The bound comes from the contract, not from a constant in the component. + // A modal that hard-coded the i32 ceiling would accept 500 here. + it("refuses a value past the published maximum, whatever that maximum is", () => { + render(); + + fireEvent.change(screen.getByRole("textbox"), { + target: { value: "500" }, + }); + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); + + // ...and the same component accepts a value the contract does allow, so + // the assertion above is about the bound and not about the input being + // inert. + fireEvent.change(screen.getByRole("textbox"), { + target: { value: "99" }, + }); + expect(screen.getByRole("button", { name: "Confirm" })).toBeEnabled(); + }); + + it("refuses a value below the published minimum", () => { + render(); + + fireEvent.change(screen.getByRole("textbox"), { + target: { value: "1" }, + }); + expect(screen.getByRole("button", { name: "Confirm" })).toBeDisabled(); + }); + + // Without a contract there is nothing to type into: the choice is enumerated + // and keeps its button grid, even though its choice type is still a range. + it("renders the option grid when the engine publishes no contract", () => { + const data: NamedChoiceData = { + player: 0, + choice_type: { NumberRange: { min: 0, max: 2 } }, + options: ["0", "1", "2"], + }; + + render(); + + expect(screen.queryByRole("textbox")).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "1" })).toBeInTheDocument(); + }); + }); }); diff --git a/client/src/i18n/__tests__/localeParity.test.ts b/client/src/i18n/__tests__/localeParity.test.ts new file mode 100644 index 0000000000..94f7287fcb --- /dev/null +++ b/client/src/i18n/__tests__/localeParity.test.ts @@ -0,0 +1,130 @@ +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +/** + * Every locale must carry the same keys as the English source, with the same + * interpolation placeholders. + * + * The test suite renders in English only (`test-setup.ts` loads `en`), so a key + * added to `en` and forgotten elsewhere, or a translation whose `{{placeholder}}` + * was dropped or renamed, produces no failing test — it produces a raw key or a + * missing value in front of a player who does not read English, which nobody + * running the suite will see. This closes that gap. + * + * The placeholder half is the one that catches real damage: a translation that + * drops `{{min}}` still renders as fluent prose, so it reads as correct while + * silently omitting the value the sentence exists to communicate. + */ + +const LOCALES_DIR = join(__dirname, "..", "locales"); +const SOURCE = "en"; + +/** + * Known pre-existing divergences, each with the reason it is tolerated. + * + * This is a list of DEFECTS, not of exemptions: an entry here means the string + * is wrong and has not been fixed yet, so keep it short and remove entries as + * they are fixed rather than adding to it. + */ +const KNOWN_PLACEHOLDER_GAPS: ReadonlyArray<{ + ns: string; + key: string; + why: string; +}> = [ + { + ns: "draft.json", + key: "intro.quick.step1", + why: + "All six translations hard-code the default '3 packs of 14 cards' instead " + + "of interpolating {{packCount}}/{{cardsPerPack}}, so a non-default draft " + + "shows wrong numbers. Pre-existing; tracked separately.", + }, +]; + +type Flat = Record; + +function flatten(value: unknown, prefix = "", out: Flat = {}): Flat { + if (value && typeof value === "object" && !Array.isArray(value)) { + for (const [k, v] of Object.entries(value as Record)) { + flatten(v, prefix ? `${prefix}.${k}` : k, out); + } + } else { + out[prefix] = value; + } + return out; +} + +function load(locale: string, ns: string): Flat { + return flatten(JSON.parse(readFileSync(join(LOCALES_DIR, locale, ns), "utf8"))); +} + +/** The `{{name}}` placeholders a string interpolates, sorted for comparison. */ +function placeholders(value: unknown): string[] { + if (typeof value !== "string") return []; + return [...value.matchAll(/\{\{\s*([\w.]+)/g)].map((m) => m[1]).sort(); +} + +const namespaces = readdirSync(join(LOCALES_DIR, SOURCE)).filter((f) => + f.endsWith(".json"), +); +const locales = readdirSync(LOCALES_DIR).filter((d) => d !== SOURCE); + +const isKnownGap = (ns: string, key: string) => + KNOWN_PLACEHOLDER_GAPS.some((g) => g.ns === ns && g.key === key); + +describe("locale parity", () => { + // Guards the guard: if the layout changes and these come back empty, every + // assertion below passes over nothing. + it("discovers the locales and namespaces it is meant to check", () => { + expect(namespaces.length).toBeGreaterThan(0); + expect(locales.length).toBeGreaterThan(0); + expect(locales).toContain("de"); + }); + + describe.each(locales)("%s", (locale) => { + it.each(namespaces)("%s has exactly the English key set", (ns) => { + const source = load(SOURCE, ns); + const target = load(locale, ns); + + expect(Object.keys(source).filter((k) => !(k in target))).toEqual([]); + // Extra keys are dead weight: nothing reads them, and they hide the fact + // that the English source dropped a string. + expect(Object.keys(target).filter((k) => !(k in source))).toEqual([]); + }); + + it.each(namespaces)("%s interpolates the same placeholders", (ns) => { + const source = load(SOURCE, ns); + const target = load(locale, ns); + + const diverged = Object.keys(source) + .filter((k) => k in target && !isKnownGap(ns, k)) + .filter( + (k) => + placeholders(source[k]).join() !== placeholders(target[k]).join(), + ) + .map( + (k) => + `${k}: en=[${placeholders(source[k])}] ${locale}=[${placeholders(target[k])}]`, + ); + + expect(diverged).toEqual([]); + }); + }); + + // Without this, a fixed defect could sit in the list forever, quietly + // exempting a key that no longer needs it. + it("has no stale entries in the known-gap list", () => { + const stale = KNOWN_PLACEHOLDER_GAPS.filter(({ ns, key }) => { + const source = load(SOURCE, ns); + return locales.every( + (locale) => + placeholders(source[key]).join() === + placeholders(load(locale, ns)[key]).join(), + ); + }).map(({ ns, key }) => `${ns}:${key}`); + + expect(stale).toEqual([]); + }); +}); diff --git a/client/src/i18n/locales/de/common.json b/client/src/i18n/locales/de/common.json index 172ffac8a2..bbd17401c4 100644 --- a/client/src/i18n/locales/de/common.json +++ b/client/src/i18n/locales/de/common.json @@ -5,6 +5,11 @@ "close": "Schließen", "closeNamed": "{{name}} schließen" }, + "quantityRef": { + "highestNumber": "die höchste Zahl", + "lowestNumber": "die niedrigste Zahl", + "chosenNumber": "die gewählte Zahl" + }, "scryOutcome": { "title": "Spähen abgeschlossen", "you": "Du", diff --git a/client/src/i18n/locales/de/game.json b/client/src/i18n/locales/de/game.json index f38d465729..9bdb47a100 100644 --- a/client/src/i18n/locales/de/game.json +++ b/client/src/i18n/locales/de/game.json @@ -2051,6 +2051,7 @@ "searchPlaceholder": "Nach Name suchen...", "noCardsFound": "Keine Karten gefunden", "buttonSubtitle": "Wähle eine Option", + "numberSubtitle": "Gib eine ganze Zahl größer oder gleich {{min}} ein", "filterPlaceholder": "Optionen filtern...", "noOptionsMatch": "Keine Option passt" }, diff --git a/client/src/i18n/locales/en/common.json b/client/src/i18n/locales/en/common.json index bbdd363ee7..a4f2fe343a 100644 --- a/client/src/i18n/locales/en/common.json +++ b/client/src/i18n/locales/en/common.json @@ -5,6 +5,11 @@ "close": "Close", "closeNamed": "Close {{name}}" }, + "quantityRef": { + "highestNumber": "the highest number", + "lowestNumber": "the lowest number", + "chosenNumber": "the chosen number" + }, "scryOutcome": { "title": "Scry complete", "you": "You", diff --git a/client/src/i18n/locales/en/game.json b/client/src/i18n/locales/en/game.json index 0024802419..d3285c72ac 100644 --- a/client/src/i18n/locales/en/game.json +++ b/client/src/i18n/locales/en/game.json @@ -2095,6 +2095,7 @@ "searchPlaceholder": "Search by name...", "noCardsFound": "No cards found", "buttonSubtitle": "Select one option", + "numberSubtitle": "Enter a whole number {{min}} or greater", "filterPlaceholder": "Filter options...", "noOptionsMatch": "No options match" }, diff --git a/client/src/i18n/locales/es/common.json b/client/src/i18n/locales/es/common.json index 9b97b9d202..8850356803 100644 --- a/client/src/i18n/locales/es/common.json +++ b/client/src/i18n/locales/es/common.json @@ -5,6 +5,11 @@ "close": "Cerrar", "closeNamed": "Cerrar {{name}}" }, + "quantityRef": { + "highestNumber": "el número más alto", + "lowestNumber": "el número más bajo", + "chosenNumber": "el número elegido" + }, "scryOutcome": { "title": "Adivinación completada", "you": "Tú", diff --git a/client/src/i18n/locales/es/game.json b/client/src/i18n/locales/es/game.json index 3dc2d1c128..67ede2a701 100644 --- a/client/src/i18n/locales/es/game.json +++ b/client/src/i18n/locales/es/game.json @@ -2051,6 +2051,7 @@ "searchPlaceholder": "Buscar por nombre...", "noCardsFound": "No se encontraron cartas", "buttonSubtitle": "Selecciona una opción", + "numberSubtitle": "Introduce un número entero mayor o igual que {{min}}", "filterPlaceholder": "Filtrar opciones...", "noOptionsMatch": "Ninguna opción coincide" }, diff --git a/client/src/i18n/locales/fr/common.json b/client/src/i18n/locales/fr/common.json index 72e09485cc..4792083a86 100644 --- a/client/src/i18n/locales/fr/common.json +++ b/client/src/i18n/locales/fr/common.json @@ -5,6 +5,11 @@ "close": "Fermer", "closeNamed": "Fermer {{name}}" }, + "quantityRef": { + "highestNumber": "le nombre le plus élevé", + "lowestNumber": "le nombre le plus bas", + "chosenNumber": "le nombre choisi" + }, "scryOutcome": { "title": "Regard terminé", "you": "Vous", diff --git a/client/src/i18n/locales/fr/game.json b/client/src/i18n/locales/fr/game.json index 8058d01020..e0bbb54da9 100644 --- a/client/src/i18n/locales/fr/game.json +++ b/client/src/i18n/locales/fr/game.json @@ -2051,6 +2051,7 @@ "searchPlaceholder": "Rechercher par nom...", "noCardsFound": "Aucune carte trouvée", "buttonSubtitle": "Sélectionnez une option", + "numberSubtitle": "Entrez un nombre entier supérieur ou égal à {{min}}", "filterPlaceholder": "Filtrer les options...", "noOptionsMatch": "Aucune option ne correspond" }, diff --git a/client/src/i18n/locales/it/common.json b/client/src/i18n/locales/it/common.json index 0ef9a2cdfa..1ba976ccdf 100644 --- a/client/src/i18n/locales/it/common.json +++ b/client/src/i18n/locales/it/common.json @@ -5,6 +5,11 @@ "close": "Chiudi", "closeNamed": "Chiudi {{name}}" }, + "quantityRef": { + "highestNumber": "il numero più alto", + "lowestNumber": "il numero più basso", + "chosenNumber": "il numero scelto" + }, "scryOutcome": { "title": "Scry completato", "you": "Tu", diff --git a/client/src/i18n/locales/it/game.json b/client/src/i18n/locales/it/game.json index 2d5409f1dc..0b0f514cbd 100644 --- a/client/src/i18n/locales/it/game.json +++ b/client/src/i18n/locales/it/game.json @@ -2051,6 +2051,7 @@ "searchPlaceholder": "Cerca per nome...", "noCardsFound": "Nessuna carta trovata", "buttonSubtitle": "Seleziona un'opzione", + "numberSubtitle": "Inserisci un numero intero maggiore o uguale a {{min}}", "filterPlaceholder": "Filtra opzioni...", "noOptionsMatch": "Nessuna opzione corrisponde" }, diff --git a/client/src/i18n/locales/pl/common.json b/client/src/i18n/locales/pl/common.json index f4b89710d3..031ab743bd 100644 --- a/client/src/i18n/locales/pl/common.json +++ b/client/src/i18n/locales/pl/common.json @@ -5,6 +5,11 @@ "close": "Zamknij", "closeNamed": "Zamknij {{name}}" }, + "quantityRef": { + "highestNumber": "najwyższa liczba", + "lowestNumber": "najniższa liczba", + "chosenNumber": "wybrana liczba" + }, "scryOutcome": { "title": "Wróżenie zakończone", "you": "Ty", diff --git a/client/src/i18n/locales/pl/game.json b/client/src/i18n/locales/pl/game.json index 1c0ca16756..4f1cedd660 100644 --- a/client/src/i18n/locales/pl/game.json +++ b/client/src/i18n/locales/pl/game.json @@ -2051,6 +2051,7 @@ "searchPlaceholder": "Szukaj po nazwie...", "noCardsFound": "Nie znaleziono kart", "buttonSubtitle": "Wybierz jedną opcję", + "numberSubtitle": "Wpisz liczbę całkowitą nie mniejszą niż {{min}}", "filterPlaceholder": "Filtruj opcje...", "noOptionsMatch": "Brak pasujących opcji" }, diff --git a/client/src/i18n/locales/pt/common.json b/client/src/i18n/locales/pt/common.json index 1e521286fd..2586a22fe3 100644 --- a/client/src/i18n/locales/pt/common.json +++ b/client/src/i18n/locales/pt/common.json @@ -5,6 +5,11 @@ "close": "Fechar", "closeNamed": "Fechar {{name}}" }, + "quantityRef": { + "highestNumber": "o número mais alto", + "lowestNumber": "o número mais baixo", + "chosenNumber": "o número escolhido" + }, "scryOutcome": { "title": "Vidência concluída", "you": "Você", diff --git a/client/src/i18n/locales/pt/game.json b/client/src/i18n/locales/pt/game.json index 576a31dedf..4e347985e6 100644 --- a/client/src/i18n/locales/pt/game.json +++ b/client/src/i18n/locales/pt/game.json @@ -2051,6 +2051,7 @@ "searchPlaceholder": "Buscar por nome...", "noCardsFound": "Nenhuma carta encontrada", "buttonSubtitle": "Selecione uma opção", + "numberSubtitle": "Digite um número inteiro maior ou igual a {{min}}", "filterPlaceholder": "Filtrar opções...", "noOptionsMatch": "Nenhuma opção corresponde" }, diff --git a/client/src/viewmodel/__tests__/costLabel.test.ts b/client/src/viewmodel/__tests__/costLabel.test.ts index 3842b782dd..2ecf911c13 100644 --- a/client/src/viewmodel/__tests__/costLabel.test.ts +++ b/client/src/viewmodel/__tests__/costLabel.test.ts @@ -443,6 +443,44 @@ describe("additionalCostChoices — repeatable additional cost", () => { }); describe("formatAbilityCost", () => { + // CR 101.4: `QuantityRef::PlayerChosenNumber` renders the cross-player fold the + // engine supplies — "the highest number" for `Max`, "the lowest number" for + // `Min` — and falls back to the bare noun for a single-player scope, which + // carries no fold. All three go through the i18n boundary, so the assertions + // read the `en` catalog rather than frontend-authored literals. + it.each([ + ["Max", "Pay the highest number life"], + ["Min", "Pay the lowest number life"], + ])("formats a chosen-number cost for the %s fold", (aggregate, expected) => { + expect( + formatAbilityCost({ + type: "PayLife", + amount: { + type: "Ref", + qty: { + type: "PlayerChosenNumber", + player: { type: "AllPlayers", aggregate }, + }, + }, + }), + ).toBe(expected); + }); + + it("falls back to the bare noun for a scoped chosen number", () => { + expect( + formatAbilityCost({ + type: "PayLife", + amount: { + type: "Ref", + qty: { + type: "PlayerChosenNumber", + player: { type: "ScopedPlayer" }, + }, + }, + }), + ).toBe("Pay the chosen number life"); + }); + it("formats disjunctive activation cost branches", () => { expect(formatAbilityCost({ type: "OneOf", diff --git a/client/src/viewmodel/costLabel.ts b/client/src/viewmodel/costLabel.ts index c957cfaf6f..3fe2f2578e 100644 --- a/client/src/viewmodel/costLabel.ts +++ b/client/src/viewmodel/costLabel.ts @@ -7,6 +7,7 @@ import type { SerializedAbility, SerializedAbilityCost, } from "../adapter/types.ts"; +import i18n from "../i18n"; import { getCrewPower, getSaddlePower } from "./keywordProps.ts"; import { renderDescription } from "../utils/description.ts"; @@ -187,6 +188,20 @@ function formatQuantityRef(ref: { type: string; [key: string]: unknown }): strin case "ExiledFromHandThisResolution": return "cards exiled from hand"; case "Speed": return "your speed"; case "ChosenNumber": return "the chosen number"; + // CR 101.4: the number a player secretly chose. The engine supplies the + // player scope (and, for the cross-player scopes, the fold); this only + // renders it — "the highest number" / "the lowest number". Routed through + // the i18n boundary; the surrounding labels in this file are legacy raw + // English and are tracked separately. + case "PlayerChosenNumber": { + const aggregate = + ref.player != null && typeof ref.player === "object" && "aggregate" in ref.player + ? (ref.player as { aggregate?: string }).aggregate + : undefined; + if (aggregate === "Max") return i18n.t("quantityRef.highestNumber"); + if (aggregate === "Min") return i18n.t("quantityRef.lowestNumber"); + return i18n.t("quantityRef.chosenNumber"); + } case "PreviousEffectAmount": return "the previous amount"; case "EventContextAmount": return "the amount"; case "EventContextSourcePower": return "the source's power"; diff --git a/crates/engine/src/ai_support/candidates.rs b/crates/engine/src/ai_support/candidates.rs index ee7ef04ccf..539dcb0c19 100644 --- a/crates/engine/src/ai_support/candidates.rs +++ b/crates/engine/src/ai_support/candidates.rs @@ -4738,6 +4738,52 @@ fn named_choice_actions( choice_type: &ChoiceType, source_display_name: Option<&str>, ) -> Vec { + // CR 107.1a/b: an unbounded number choice ("a number 0 or greater") offers no + // option list, so the AI must SAMPLE a domain it cannot enumerate. The sample + // is deliberately small and game-relevant rather than a truncated 0..=N walk: + // on Wheel of Misfortune the meaningful decisions are "bid nothing", "bid just + // enough to wheel", and "bid past a life total", so the ladder is anchored to + // the live life totals rather than to an arbitrary ceiling. + // + // Every candidate is filtered through `accepts_free_entry_answer`, the same + // authority the answer seam uses, so the AI can never propose a value the + // engine would then reject. + if options.is_empty() { + if let ChoiceType::NumberRange { min, max: None, .. } = choice_type { + let highest_life = state + .players + .iter() + .map(|p| p.life.max(0) as u32) + .max() + .unwrap_or(0); + let mut sample: Vec = vec![ + *min, + min.saturating_add(1), + min.saturating_add(2), + highest_life, + highest_life.saturating_add(1), + ]; + sample.sort_unstable(); + sample.dedup(); + return sample + .into_iter() + .map(|n| n.to_string()) + .filter(|choice| { + choice_type + .accepts_free_entry_answer(choice) + .unwrap_or(false) + }) + .map(|choice| { + candidate( + GameAction::ChooseOption { choice }, + TacticalClass::Selection, + Some(player), + ) + }) + .collect(); + } + } + if options.is_empty() && matches!(choice_type, ChoiceType::CardName) { return card_name_choice_candidates(state, player, source_display_name) .into_iter() @@ -6423,6 +6469,7 @@ mod tests { names.extend((0..10_000).map(|i| format!("Bulk Card {i}"))); state.all_card_names = names.into(); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: ChoiceType::CardName, options: Vec::new(), diff --git a/crates/engine/src/ai_support/mod.rs b/crates/engine/src/ai_support/mod.rs index f8cd1fea9a..0d62fdb000 100644 --- a/crates/engine/src/ai_support/mod.rs +++ b/crates/engine/src/ai_support/mod.rs @@ -6023,6 +6023,7 @@ mod tests { // A "choose a player" prompt (CR 601.2b) with NO offered options — the // engine can produce no legal `ChooseOption`, so no submitter can act. state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: ChoiceType::Labeled { options: vec![] }, options: vec![], diff --git a/crates/engine/src/analysis/ability_graph.rs b/crates/engine/src/analysis/ability_graph.rs index 19c31dedc6..6c1f8eceaa 100644 --- a/crates/engine/src/analysis/ability_graph.rs +++ b/crates/engine/src/analysis/ability_graph.rs @@ -918,6 +918,7 @@ fn effect_projection(effect: &Effect) -> Projection { | Effect::TargetOnly { .. } | Effect::Choose { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::ChooseDamageSource { .. } | Effect::Suspect { .. } | Effect::Unsuspect { .. } diff --git a/crates/engine/src/database/synthesis.rs b/crates/engine/src/database/synthesis.rs index 5a3d2bbcea..d14792ef3a 100644 --- a/crates/engine/src/database/synthesis.rs +++ b/crates/engine/src/database/synthesis.rs @@ -9011,7 +9011,9 @@ pub fn synthesize_read_ahead(face: &mut CardFace) { Effect::Choose { choice_type: ChoiceType::NumberRange { min: 1, - max: final_chapter.min(u8::MAX as u32) as u8, + // CR 702.155b: the Saga states its own upper bound (the final + // chapter), so this range is genuinely bounded. + max: Some(final_chapter), distinctness: crate::types::ability::NumberDistinctness::Repeatable, }, persist: true, @@ -23010,7 +23012,7 @@ mod devour_synthesis_tests { panic!("read-ahead ETB should choose a number"); }; // CR 702.155b + CR 714.2d: between one and the final chapter number (3). - assert_eq!((*min, *max), (1, 3)); + assert_eq!((*min, *max), (1, Some(3))); assert!(*persist, "chosen number must persist for ChosenNumber"); let sub = execute diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index 4c33dff067..60187cd1dd 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -2096,6 +2096,7 @@ fn legacy_quantity_ref(x: &QuantityRef) -> bool { | QuantityRef::TurnsTaken | QuantityRef::CrimesCommittedThisTurn | QuantityRef::ChosenNumber + | QuantityRef::PlayerChosenNumber { .. } | QuantityRef::AttackedThisTurn { .. } | QuantityRef::DescendedThisTurn // CR 701.65b/701.66b/701.67c: controller-scoped per-turn bend accumulator @@ -3066,6 +3067,12 @@ fn legacy_effect(x: &Effect) -> bool { first: _, second: _, } => false, + // CR 101.4: unlike its `SwapChosenLabels` neighbour this variant DOES + // carry a `PlayerFilter`, so it must be traversed rather than answered + // `false` outright — `legacy_player_filter` detects `TriggeringPlayer` + // and recurses through the nested `ControlsCount` / `PlayerAttribute` / + // `AllExcept` forms, any of which a future reveal could name. + Effect::RevealChosenNumbers { players } => legacy_player_filter(players), Effect::Attach { attachment, target } | Effect::UnattachAll { attachment, target } => { legacy_target_filter(attachment) || legacy_target_filter(target) } @@ -5637,6 +5644,12 @@ fn rw_effect( first: _, second: _, } => (ext_write(StateKind::Other), None), + // CR 101.4 + CR 603.3b: publishes per-player chosen numbers. The write is + // to the same per-player chosen-attribute storage `Effect::Choose` + // produces, so it is classified with it (`StateKind::Other`, the + // unclassifiable/fail-closed kind) rather than given a narrower kind that + // no profiled read would conflict with. + Effect::RevealChosenNumbers { players: _ } => (ext_write(StateKind::Other), None), // ---- Histogram-absent ⇒ fail-closed conservative ---- Effect::StartYourEngines { .. } @@ -5932,6 +5945,12 @@ fn rw_quantity_ref(x: &QuantityRef) -> RwProfile { | QuantityRef::TrackedSetSize | QuantityRef::FilteredTrackedSetSize { .. } | QuantityRef::ChosenNumber + // CR 101.4 + CR 608.2d: the player-axis chosen-number read. Its producer + // is a persisting `Effect::Choose`, whose own arm below already declares + // `reads_member_bound`; classifying the reader the same way keeps the + // CR 603.3b same-event ordering gate fail-closed for the producer/consumer + // pair, exactly as for the object-axis `ChosenNumber` sibling. + | QuantityRef::PlayerChosenNumber { .. } | QuantityRef::CostXPaid | QuantityRef::KickerCount | QuantityRef::AdditionalCostPaymentCount diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index 912d6b2e72..f69b026385 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -473,6 +473,11 @@ fn scan_effect(x: &Effect, mode: ScanMode) -> Axes { first: _, second: _, } => Axes::CONSERVATIVE, + // CR 101.4: publishes an already-committed per-player number. Writes only + // the visibility half of the chosen-number ledger (`Number` -> + // `RevealedNumber`), never a value, so it perturbs no scanned axis; the + // player set it names is the only thing to descend into. + Effect::RevealChosenNumbers { players } => scan_player_filter(players, mode), Effect::EachSourceDealsDamage { sources, amount, @@ -2397,6 +2402,13 @@ fn scan_quantity_ref(x: &QuantityRef, mode: ScanMode) -> Axes { acc } QuantityRef::ChosenNumber => Axes::NONE, + // CR 101.4 + CR 608.2d: the number a player chose this resolution. Like + // its object-axis sibling `ChosenNumber` this is a bounded one-shot + // answer, not an accumulating projected resource — a re-choose REPLACES + // the stored value rather than adding to it (`bind_named_choice`), so it + // cannot grow across loop iterations. The only axis it can contribute is + // whatever its player scope carries. + QuantityRef::PlayerChosenNumber { player } => scan_player_scope(player), QuantityRef::AttackedThisTurn { scope, filter } => { let mut acc = Axes::NONE; acc = acc.or(scan_count_scope(scope)); @@ -5412,6 +5424,7 @@ fn effect_target_ctx(e: &Effect, mode: ScanMode) -> FilterReadContext { | Effect::ApplyPostReplacementDamage { .. } | Effect::OpponentGuess { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::Draw { .. } | Effect::Pump { .. } | Effect::PairWith { .. } @@ -5823,6 +5836,7 @@ fn effect_census_role(e: &Effect) -> CensusRole { | Effect::ApplyPostReplacementDamage { .. } | Effect::OpponentGuess { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::Draw { .. } | Effect::Pump { .. } | Effect::PairWith { .. } @@ -6042,6 +6056,7 @@ pub(crate) fn effect_is_randomness_bearing(e: &Effect) -> bool { | Effect::EachDealsDamageEqualToPower { .. } | Effect::OpponentGuess { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::Draw { .. } | Effect::Pump { .. } | Effect::PairWith { .. } diff --git a/crates/engine/src/game/casting.rs b/crates/engine/src/game/casting.rs index 7c3eb781e4..6e68071f6a 100644 --- a/crates/engine/src/game/casting.rs +++ b/crates/engine/src/game/casting.rs @@ -602,10 +602,15 @@ pub(crate) fn begin_variable_speed_payment( player, options: (min..=max).map(|value| value.to_string()).collect(), choice_type: ChoiceType::NumberRange { - min, - max, + min: u32::from(min), + // CR 702.179: a speed payment is bounded by the player's current + // speed, so this range states a real maximum. + max: Some(u32::from(max)), distinctness: crate::types::ability::NumberDistinctness::Repeatable, }, + // A stated maximum means the options above enumerate the domain; there + // is no free entry to contract for. + free_entry: None, source: None, persist_player: None, } diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 2b4248e04c..4efd2ce08e 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -1722,6 +1722,9 @@ fn fmt_quantity_ref(qty: &QuantityRef) -> String { } QuantityRef::TurnsTaken => "turns taken".into(), QuantityRef::ChosenNumber => "chosen number".into(), + QuantityRef::PlayerChosenNumber { player } => { + format!("secretly chosen number ({})", fmt_player_scope(player)) + } QuantityRef::AttackedThisTurn { .. } => "attacked this turn".into(), QuantityRef::DescendedThisTurn => "descended this turn".into(), QuantityRef::LoyaltyAbilitiesActivatedThisTurn { player } => { @@ -2078,7 +2081,13 @@ fn fmt_choice_type(ct: &ChoiceType) -> String { } } ChoiceType::CardName => "card name", - ChoiceType::NumberRange { min, max, .. } => return format!("number ({min}-{max})"), + // CR 107.1a/b: an unbounded range has no ceiling to print. + ChoiceType::NumberRange { min, max, .. } => { + return match max { + Some(max) => format!("number ({min}-{max})"), + None => format!("number ({min} or greater)"), + } + } ChoiceType::Labeled { options } => return format!("one of: {}", options.join(", ")), ChoiceType::LandType => "land type", ChoiceType::CardPredicate { .. } => "card predicate", @@ -2948,6 +2957,9 @@ fn effect_details(effect: &Effect) -> Vec<(String, String)> { Effect::SwapChosenLabels { first, second } => { d.push(("swap".into(), format!("{first} <-> {second}"))); } + Effect::RevealChosenNumbers { players } => { + d.push(("reveal chosen numbers".into(), format!("{players:?}"))); + } Effect::ChooseDamageSource { source_filter } => { d.push(("source".into(), fmt_target(source_filter))); } @@ -6516,6 +6528,7 @@ fn visit_direct_effect_ability_payloads<'a>( | Effect::Choose { .. } | Effect::OpponentGuess { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::ChooseDamageSource { .. } | Effect::Suspect { .. } | Effect::Unsuspect { .. } @@ -8131,6 +8144,9 @@ fn quantity_ref_feature(qref: &QuantityRef) -> (&'static str, FeatureSupport) { // strict-failure marker anywhere, so it is genuinely handled. QuantityRef::TurnsTaken => ("TurnsTaken", Handled), QuantityRef::ChosenNumber => ("ChosenNumber", Unhandled), + // CR 101.4 + CR 608.2d: resolved live in `quantity::resolve_quantity` + // over `Player::chosen_attributes` (per-candidate and aggregate scopes). + QuantityRef::PlayerChosenNumber { .. } => ("PlayerChosenNumber", Handled), QuantityRef::AttackedThisTurn { .. } => ("AttackedThisTurn", Handled), QuantityRef::DescendedThisTurn => ("DescendedThisTurn", Unhandled), QuantityRef::LoyaltyAbilitiesActivatedThisTurn { .. } => { diff --git a/crates/engine/src/game/effects/choose.rs b/crates/engine/src/game/effects/choose.rs index 1a73994d35..4c60de68c0 100644 --- a/crates/engine/src/game/effects/choose.rs +++ b/crates/engine/src/game/effects/choose.rs @@ -78,6 +78,9 @@ pub fn resolve( state.waiting_for = WaitingFor::NamedChoice { player: ability.controller, + // CR 107.1a/b: publish the free-entry contract alongside the choice it + // belongs to, so a client never has to re-derive it from `choice_type`. + free_entry: choice_type.free_entry(), choice_type, options, source, @@ -172,6 +175,10 @@ pub(crate) fn resolve_random_in_chain( ) { ability.update_trigger_source_context_in_resolution_segment(context); } + // CR 101.4 + CR 608.2d: mirror the interactive answer handler so a + // game-selected number is readable per-player too (CR 608.2d override — the + // game makes the choice, but it is still THIS player's chosen number). + record_player_chosen_number(state, ability.controller, &choice_type, &chosen); // CR 608.2c + CR 109.4: A `Choose(Player)`/`Choose(Opponent)` answer binds a // resolution-scoped chosen player. Append it to the resolving ability's @@ -251,16 +258,20 @@ pub(crate) fn bind_named_choice( .filter(|source| source.is_exact_object_and_resolution()) .cloned(); if let Some(pid) = persist_player { - // CR 607.2d / CR 607.2m (by analogy): per-player anchor label. The - // `Player` axis only ever stores `ChosenAttribute::Label`, so no - // multi-keyword split is needed here. Replace-on-rechoose (retain-drop - // any existing `Label`, then push) mirrors the object-branch Keyword - // replace so "last chose" holds exactly one anchor per player. + // CR 607.2d / CR 607.2m (by analogy): per-player anchor. Unlike an + // object's `chosen_attributes` (which accumulates a history — The + // Toymaker's Trap reads every number it has committed), a PLAYER anchor + // answers "what did this player choose", so re-choosing REPLACES the + // prior answer of the same kind. Replace-on-rechoose is keyed on the + // attribute's own discriminant, which is byte-identical to the previous + // `Label`-only retain for the one kind routed here today and keeps any + // other kind a different effect recorded on the player untouched. if let Some(attr) = ChosenAttribute::from_choice(choice_type.clone(), choice) { if let Some(player) = state.players.iter_mut().find(|p| p.id == pid) { + let replaced = std::mem::discriminant(&attr); player .chosen_attributes - .retain(|a| !matches!(a, ChosenAttribute::Label(_))); + .retain(|a| std::mem::discriminant(a) != replaced); player.chosen_attributes.push(attr); } // CR 613.1: per-player labels feed statics/filters — re-run layers. @@ -335,6 +346,20 @@ pub(crate) fn named_choice_authority( persist: bool, choice_type: &ChoiceType, ) -> (Option, Option) { + // CR 607.2d / CR 607.2m (by analogy): a persisting `Labeled` answer chosen + // during a per-player iteration is the planar anchor (Two Streams Facility), + // recorded on the choosing player instead of the source object. + // + // NOTE for future axes: `scoped_player` is NOT a reliable "this is a + // per-player fan-out" marker — it is also set for a plain triggered ability + // resolving for its own controller (measured: The Toymaker's Trap's upkeep + // trigger arrives here with `scoped_player == controller == Some(P0)`, + // indistinguishable from the first iteration of a real fan-out). Adding a + // choice kind to this routing therefore MOVES the answer off the source for + // single-chooser cards too, which breaks any object-scoped reader. The + // per-player secret number (CR 101.4) is instead recorded ADDITIVELY by + // `record_player_chosen_number`, leaving every existing source binding + // intact. let persist_player = (persist && matches!(choice_type, ChoiceType::Labeled { .. })) .then_some(ability.scoped_player) .flatten(); @@ -361,6 +386,54 @@ pub(crate) fn named_choice_authority( ) } +/// CR 101.4 + CR 608.2d: Record the number a PLAYER chose onto that player, as +/// the per-resolution ledger [`crate::types::ability::QuantityRef::PlayerChosenNumber`] +/// folds into "the highest / lowest number" (Wheel of Misfortune, Menacing Ogre, +/// Life at Stake). +/// +/// ADDITIVE, not a reroute: the source-object binding that `bind_named_choice` +/// performs is left exactly as it was, so a single-chooser card whose reader is +/// object-scoped (The Toymaker's Trap's committed number, read through +/// `QuantityRef::ChosenNumber`) is unaffected. The two axes answer different +/// questions — "what number is committed on this permanent" versus "what number +/// did this player choose" — and a card may legitimately want either. +/// +/// Recording it for EVERY number choice rather than only for a detected +/// per-player fan-out is deliberate: `ResolvedAbility::scoped_player` is set for +/// a plain triggered ability resolving for its own controller as well as for a +/// real fan-out iteration, so there is no reliable runtime marker to gate on. +/// The write is harmless where nothing reads it — the ledger is cleared at every +/// top-level resolution entry (`effects::resolve_ability_chain`, depth 0), and +/// `game::visibility` keeps a player's number private to that player, so an +/// unread copy can neither leak nor survive into a later resolution. +/// +/// Replace-on-rechoose: a player holds exactly one chosen number, so a second +/// choice in the same resolution supersedes the first. +pub(crate) fn record_player_chosen_number( + state: &mut GameState, + chooser: PlayerId, + choice_type: &ChoiceType, + choice: &str, +) { + if !matches!(choice_type, ChoiceType::NumberRange { .. }) { + return; + } + let Ok(value) = choice.parse::() else { + return; + }; + if let Some(player) = state.players.iter_mut().find(|p| p.id == chooser) { + player.chosen_attributes.retain(|attribute| { + !matches!( + attribute, + ChosenAttribute::Number(_) | ChosenAttribute::RevealedNumber(_) + ) + }); + player + .chosen_attributes + .push(ChosenAttribute::Number(value)); + } +} + fn register_exact_named_choice_source(state: &mut GameState, source: Option<&NamedChoiceSource>) { let Some(source) = source.filter(|source| source.is_exact_object_and_resolution()) else { return; @@ -570,9 +643,16 @@ fn compute_options( max, distinctness, } => match distinctness { - crate::types::ability::NumberDistinctness::Repeatable => { - (*min..=*max).map(|n| n.to_string()).collect() - } + // CR 107.1a/b: an UNBOUNDED range has no list to enumerate. Return + // empty and let `options_supplied_by_player` route it to the + // free-entry path (the same one `CardName` uses) — the client renders + // a numeric input and the answer seam validates it. Materializing a + // stand-in ceiling here is exactly the bug that made a legal choice + // illegal on Wheel of Misfortune. + _ if max.is_none() => Vec::new(), + crate::types::ability::NumberDistinctness::Repeatable => (*min..=max.unwrap_or(*min)) + .map(|n| n.to_string()) + .collect(), // CR 609.3 + "...that hasn't been chosen": each successive COMMIT // excludes numbers already committed on this source across prior // resolutions. Chosen numbers persist as `ChosenAttribute::Number` @@ -587,7 +667,7 @@ fn compute_options( // offers a separate DistinctFromSourceHistory NumberRange on the same // source, scope the read to a per-choice tag. crate::types::ability::NumberDistinctness::DistinctFromSourceHistory => { - let used: Vec = state + let used: Vec = state .objects .get(&source_id) .map(|o| { @@ -600,7 +680,7 @@ fn compute_options( .collect() }) .unwrap_or_default(); - (*min..=*max) + (*min..=max.unwrap_or(*min)) .filter(|n| !used.contains(n)) .map(|n| n.to_string()) .collect() @@ -924,6 +1004,7 @@ mod tests { match &state.waiting_for { WaitingFor::NamedChoice { + free_entry: _, player, choice_type, options, @@ -970,6 +1051,7 @@ mod tests { match &state.waiting_for { WaitingFor::NamedChoice { + free_entry: None, choice_type, options, .. @@ -1084,6 +1166,7 @@ mod tests { match &state.waiting_for { WaitingFor::NamedChoice { + free_entry: None, choice_type, options, .. @@ -1121,7 +1204,7 @@ mod tests { Effect::Choose { choice_type: ChoiceType::NumberRange { min: 0, - max: 5, + max: Some(5), distinctness: crate::types::ability::NumberDistinctness::Repeatable, }, persist: false, @@ -1162,7 +1245,7 @@ mod tests { let distinct = ChoiceType::NumberRange { min: 1, - max: 5, + max: Some(5), distinctness: crate::types::ability::NumberDistinctness::DistinctFromSourceHistory, }; assert_eq!( @@ -1174,7 +1257,7 @@ mod tests { // Same history under Repeatable: full range is still offered. let repeatable = ChoiceType::NumberRange { min: 1, - max: 5, + max: Some(5), distinctness: crate::types::ability::NumberDistinctness::Repeatable, }; assert_eq!( @@ -1240,6 +1323,7 @@ mod tests { match &state.waiting_for { WaitingFor::NamedChoice { + free_entry: _, player, choice_type, options, diff --git a/crates/engine/src/game/effects/choose_counter_kind.rs b/crates/engine/src/game/effects/choose_counter_kind.rs index 49d8ad3f5f..84b987ffc2 100644 --- a/crates/engine/src/game/effects/choose_counter_kind.rs +++ b/crates/engine/src/game/effects/choose_counter_kind.rs @@ -107,6 +107,7 @@ pub fn resolve( let options: Vec = kinds.iter().map(|k| k.as_str().into_owned()).collect(); state.waiting_for = WaitingFor::NamedChoice { player: ability.controller, + free_entry: choice_type.free_entry(), choice_type, options, source, diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 7da525dd54..6947a1cfec 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -10,12 +10,13 @@ use crate::game::filter; use crate::game::speed::has_max_speed; use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, CardPlayMode, CardTypeSetSource, - ControllerRef, CopyRetargetPermission, CostPaidObjectSnapshot, EachDamageRecipient, Effect, - EffectError, EffectKind, EffectOutcomeSignal, EffectResolutionResult, EffectScope, FilterProp, - ManaProduction, OpponentMayScope, PlayerFilter, PlayerScope, QuantityExpr, QuantityRef, - RepeatContinuation, ResolvedAbility, RevealUntilDisposition, SacrificeCost, - SacrificeRequirement, SharedQuality, SharedQualityRelation, SiblingCondition, SubAbilityLink, - TapStateChange, TargetChoiceTiming, TargetFilter, TargetRef, ThisWayCause, + ChosenAttribute, ControllerRef, CopyRetargetPermission, CostPaidObjectSnapshot, + EachDamageRecipient, Effect, EffectError, EffectKind, EffectOutcomeSignal, + EffectResolutionResult, EffectScope, FilterProp, ManaProduction, OpponentMayScope, + PlayerFilter, PlayerScope, QuantityExpr, QuantityRef, RepeatContinuation, ResolvedAbility, + RevealUntilDisposition, SacrificeCost, SacrificeRequirement, SharedQuality, + SharedQualityRelation, SiblingCondition, SubAbilityLink, TapStateChange, TargetChoiceTiming, + TargetFilter, TargetRef, ThisWayCause, }; #[cfg(test)] use crate::types::ability::{AttackScope, AttackSubject}; @@ -185,6 +186,7 @@ pub mod remove_from_combat; pub mod renown; pub mod return_as_aura; pub mod reveal; +pub mod reveal_chosen_numbers; pub mod reveal_from_hand; pub mod reveal_hand; pub mod reveal_top; @@ -645,6 +647,13 @@ pub(crate) fn candidate_player_scalar(p: &Player, attr: &QuantityRef) -> Option< QuantityRef::CardsDrawnThisTurn { .. } => { Some(u32_to_i32_saturating(p.cards_drawn_this_turn)) } + // CR 101.4 + CR 608.2d: the number this candidate secretly chose during + // the current resolution. `None` for a player who chose nothing, which + // fails the candidate predicate CLOSED — "each player who didn't choose + // the lowest number" must not sweep in a player who never chose at all. + QuantityRef::PlayerChosenNumber { .. } => p + .chosen_number() + .map(crate::game::arithmetic::u32_to_i32_saturating), _ => None, } } @@ -3044,6 +3053,10 @@ fn quantity_ref_counts_population_matching( | QuantityRef::PlayerCount { .. } | QuantityRef::CountersOn { .. } | QuantityRef::PlayerCounter { .. } + // CR 101.4: reads a per-player scalar off `Player::chosen_attributes`. + // It counts no POPULATION of objects — its `PlayerScope` selects which + // players contribute and how they fold, never a `TargetFilter`. + | QuantityRef::PlayerChosenNumber { .. } | QuantityRef::TargetControllerCounter { .. } | QuantityRef::Variable { .. } | QuantityRef::Power { .. } @@ -4685,6 +4698,9 @@ pub fn resolve_effect( Effect::RevealHand { .. } => reveal_hand::resolve(state, ability, events), Effect::RevealFromHand { .. } => reveal_from_hand::resolve(state, ability, events), Effect::Reveal { .. } => reveal::resolve(state, ability, events), + Effect::RevealChosenNumbers { .. } => { + reveal_chosen_numbers::resolve(state, ability, events) + } Effect::RevealTop { .. } => reveal_top::resolve(state, ability, events), Effect::ExileTop { .. } => exile_top::resolve(state, ability, events), Effect::ExileFaceDownPile { .. } => exile_face_down_pile::resolve(state, ability, events), @@ -8730,6 +8746,25 @@ pub fn resolve_ability_chain( // alongside `last_zone_changed_ids` so cross-resolution leakage is // impossible. state.last_vote_ballots = crate::im::Vector::new(); + // CR 101.4 + CR 608.2d: Per-resolution secret-number ledger. A per-player + // `Effect::Choose { NumberRange }` fan-out records each answer as + // `ChosenAttribute::Number` on the chooser (`bind_named_choice`), and + // `QuantityRef::PlayerChosenNumber` folds those into "the highest/lowest + // number". `Player::chosen_attributes` is otherwise DURABLE (players never + // change zones), so without this reset a later card whose choosers are a + // SUBSET of the table — Life at Stake's "you and target creature's + // controller" — would fold in bystanders' numbers left over from an + // earlier Wheel of Misfortune. Cleared alongside `last_vote_ballots`, the + // sibling per-player choice ledger, for the same reason. The player axis + // stores no other `Number`, so nothing else is disturbed. + for player in state.players.iter_mut() { + player.chosen_attributes.retain(|attribute| { + !matches!( + attribute, + ChosenAttribute::Number(_) | ChosenAttribute::RevealedNumber(_) + ) + }); + } state.last_effect_amount = None; // CR 120.10: resolution-local excess channel resets with its total twin. state.last_effect_excess_amount = None; @@ -22942,6 +22977,50 @@ mod tests { ); } + /// CR 101.4 + CR 608.2d: the per-player secret-number ledger is cleared at + /// chain depth 0 too, for the same reason as `last_vote_ballots`. Players + /// never change zones, so `Player::chosen_attributes` is otherwise durable: + /// without the reset, a card whose choosers are a SUBSET of the table (Life + /// at Stake's two choosers) would fold bystanders' numbers left over from an + /// earlier Wheel of Misfortune into its "highest number". + /// + /// Fail-on-revert: drop the reset and the stale `Number(9)` below survives, + /// so the extremum reads 9 instead of 0. + #[test] + fn player_chosen_numbers_clear_at_chain_boundary() { + use crate::types::ability::{AggregateFunction, ChosenAttribute, PlayerScope}; + + let mut state = GameState::new_two_player(42); + // A number left behind by an earlier resolution. + state.players[1].chosen_attributes = vec![ChosenAttribute::Number(9)]; + + let ability = ResolvedAbility::new(Effect::NoOp, vec![], ObjectId(100), PlayerId(0)); + let mut events = Vec::new(); + resolve_ability_chain(&mut state, &ability, &mut events, 0).unwrap(); + + assert!( + state.players[1].chosen_attributes.is_empty(), + "a fresh top-level resolution must not inherit a prior one's secret numbers" + ); + assert_eq!( + crate::game::quantity::resolve_quantity( + &state, + &QuantityExpr::Ref { + qty: QuantityRef::PlayerChosenNumber { + player: PlayerScope::AllPlayers { + aggregate: AggregateFunction::Max, + exclude: None, + }, + }, + }, + PlayerId(0), + ObjectId(100), + ), + 0, + "with no numbers chosen this resolution the extremum is empty" + ); + } + /// CR 608.2c + CR 109.5: "for each opponent who searched their library /// this way" relies on `player_actions_this_way` accumulating across /// player_scope iterations. diff --git a/crates/engine/src/game/effects/opponent_guess.rs b/crates/engine/src/game/effects/opponent_guess.rs index 5bf1be1bae..a58b83cb4b 100644 --- a/crates/engine/src/game/effects/opponent_guess.rs +++ b/crates/engine/src/game/effects/opponent_guess.rs @@ -47,9 +47,17 @@ pub fn resolve( // domain directly from `choice_type` (ignoring `distinctness`), NOT // via the history-subtracting `compute_options`. let options = match choice_type { - ChoiceType::NumberRange { min, max, .. } => { - (*min..=*max).map(|n| n.to_string()).collect::>() - } + // CR 107.1a/b: only a BOUNDED committed domain can be enumerated + // for the guesser. An unbounded one has no printed list to guess + // from, so it falls through to the empty set and the CR 609.3 + // no-op guard below — the same outcome as any other committed + // domain the guess machinery cannot enumerate. No printed card + // pairs an unbounded choice with a guess. + ChoiceType::NumberRange { + min, + max: Some(max), + .. + } => (*min..=*max).map(|n| n.to_string()).collect::>(), // Other committed-choice domains route through the shared option // enumerator's printed-domain semantics. No printed card uses a // non-number committed guess yet; fall back to an empty set so the @@ -249,7 +257,7 @@ pub(crate) fn guess_is_correct( // CR 607.2d link between two distinct printed abilities. let committed = match committed_choice { Some(crate::types::ability::ChosenAttribute::Number(number)) => { - Some(i32::from(*number)) + Some(crate::game::arithmetic::u32_to_i32_saturating(*number)) } _ => None, }; diff --git a/crates/engine/src/game/effects/reveal_chosen_numbers.rs b/crates/engine/src/game/effects/reveal_chosen_numbers.rs new file mode 100644 index 0000000000..5a025a3ef0 --- /dev/null +++ b/crates/engine/src/game/effects/reveal_chosen_numbers.rs @@ -0,0 +1,79 @@ +use crate::types::ability::{Effect, EffectError, EffectKind, PlayerFilter, ResolvedAbility}; +use crate::types::events::GameEvent; +use crate::types::game_state::GameState; +use crate::types::player::PlayerId; + +/// CR 101.4 + CR 608.2c: Publish the numbers `players` secretly chose earlier in +/// this resolution — the runtime half of "then all players reveal those numbers +/// simultaneously" (Wheel of Misfortune), "then you reveal the number you chose" +/// (The Toymaker's Trap), "Then those numbers are revealed" (Menacing Ogre). +/// +/// The transition is typed, not a visibility flag: each named player's +/// `ChosenAttribute::Number` (private — `game::visibility` redacts it from every +/// other viewer) becomes `ChosenAttribute::RevealedNumber` (public). Because +/// privacy is a property of the attribute kind, this single conversion is what +/// makes the card's reveal instruction observable, and a card that never reveals +/// keeps its numbers secret with no extra bookkeeping. +/// +/// CR 101.4: the reveal is SIMULTANEOUS, so every player is converted before the +/// event is emitted and one event carries the whole set. Iteration is in APNAP +/// order purely so the event's contents are deterministic (CR 101.4 fixes that +/// order for the choices themselves); no game action is sequenced by it. +/// +/// CR 609.3: naming a player who chose no number does as much as possible — +/// nothing. That is what lets Wheel of Misfortune's `players: All` be correct on +/// a table where a card's choosers were only a subset (Life at Stake). +pub fn resolve( + state: &mut GameState, + ability: &ResolvedAbility, + events: &mut Vec, +) -> Result<(), EffectError> { + let players = match &ability.effect { + Effect::RevealChosenNumbers { players } => players.clone(), + _ => { + return Err(EffectError::InvalidParam( + "expected RevealChosenNumbers effect".to_string(), + )) + } + }; + + let candidates: Vec = crate::game::players::apnap_order_from( + state, + ability.starting_with.clone(), + ability.controller, + ) + .into_iter() + .filter(|pid| { + super::matches_player_scope(state, *pid, &players, ability.controller, ability.source_id) + }) + .collect(); + + let mut numbers: Vec<(PlayerId, u32)> = Vec::new(); + for pid in candidates { + if let Some(player) = state.players.iter_mut().find(|p| p.id == pid) { + if let Some(value) = player.reveal_chosen_number() { + numbers.push((pid, value)); + } + } + } + + // CR 613.1: a per-player published value can gate statics/filters that read + // it, so re-run layers for the same reason the per-player anchor bind does. + if !numbers.is_empty() { + crate::game::layers::mark_layers_full(state); + events.push(GameEvent::ChosenNumbersRevealed { numbers }); + } + + events.push(GameEvent::EffectResolved { + kind: EffectKind::from(&ability.effect), + source_id: ability.source_id, + subject: None, + }); + Ok(()) +} + +/// The default population when a card does not name one. "Reveal those numbers" +/// with no subject means every player who chose (CR 101.4). +pub(crate) fn default_players() -> PlayerFilter { + PlayerFilter::All +} diff --git a/crates/engine/src/game/elimination.rs b/crates/engine/src/game/elimination.rs index 7adc2a83e7..f0325bb62e 100644 --- a/crates/engine/src/game/elimination.rs +++ b/crates/engine/src/game/elimination.rs @@ -1179,6 +1179,7 @@ fn retire_pending_zone_change_contexts_owned_by(state: &mut GameState, player: P fn abandon_source_bound_resolution_prompt(state: &mut GameState, player: PlayerId) { let abandon = match &state.waiting_for { WaitingFor::NamedChoice { + free_entry: None, player: chooser, source, persist_player, @@ -1806,6 +1807,7 @@ mod tests { ); let context = source_context(&state, source); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(1), choice_type: crate::types::ability::ChoiceType::Labeled { options: vec!["chosen".to_string()], @@ -1845,6 +1847,7 @@ mod tests { fn leaving_persisted_named_choice_player_abandons_the_whole_family() { let mut state = setup_three_player(); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: crate::types::ability::ChoiceType::Labeled { options: vec!["chosen".to_string()], diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 4302cf2322..04ad0e0156 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -18185,14 +18185,71 @@ mod stage2_injector_tests { // shifts combine with #6958's paid-cast outcome exclusion and // #6976's conditional-branch exclusions. None creates an // `OptionalEffect` prompt. Re-pinned against the merged source. - // Current-main port: #7221's typed player-action completion seam and the - // contemporaneous upstream changes moved these three producers. Re-derived - // in the merged source, still in their named production functions. - // #7382's optional-player routing and pre-entry controller prompt move only - // the third and fifth coordinates; both named mints were re-read in place. - "game/effects/mod.rs:6640".to_string(), - "game/effects/mod.rs:6717".to_string(), - "game/effects/mod.rs:9939".to_string(), + // Wheel of Misfortune (#7266), MEASURED ON THE MERGE TREE. This row's + // own header warns that a fork branch's pins are correct for the branch + // and wrong for `refs/pull//merge`; both sides of this conflict were + // that kind of local-correct. `origin/main` carried `:6306/:6383/:9578` + // and the branch carried `:6261/:6338/:9550`; NEITHER is right here, so + // the merged file was re-measured rather than either side taken: + // `:6306/:6383/:9578 => :6315/:6392/:9606`, i.e. `+9/+9/+28`. + // + // The asymmetry IS the measurement. This branch's non-test additions to + // effects/mod.rs, in file order: + // `pub mod reveal_chosen_numbers;` — 1 line, above all three. + // the `Effect::RevealChosenNumbers` dispatch arm — 3 lines, above all + // three (the dispatch table precedes every producer). + // the `QuantityRef::PlayerChosenNumber` arm in + // `candidate_player_scalar` — 5 lines, above all three. + // 1 + 3 + 5 = the uniform `+9` the first two producers take. The third + // takes a further `+19` from the depth-0 per-player secret-number ledger + // reset in `resolve_ability_chain` (16 lines, plus 3 widening the clear + // to retain both `Number` and `RevealedNumber`), which sits above it and + // below the first two: 9 + 19 = 28. Predicted and observed agree. + // + // Nothing added here raises a `WaitingFor`: the two clears and the scalar + // read are pure state reads/writes, and the dispatch arm delegates to + // `reveal_chosen_numbers::resolve`, which converts + // `ChosenAttribute::Number` to `RevealedNumber` and emits an event. The + // census set is therefore still exactly 5. + // + // NOTE for the next drift: upstream refactored the third producer from a + // `state.waiting_for = …` assignment form into a bare struct-literal value + // inside a returned tuple. It is still one producer and still matches this + // row's assembled needle, but a grep for the old assignment form now finds + // only two — measure with the needle, not with the assignment. + // + // And do NOT spell the needle literally in this comment. It is assembled + // at the top of this row precisely so the row cannot count itself, but the + // walker reads every line of this file: writing the struct-literal form + // out in prose here adds a phantom `in_test` hit per mention. Two such + // mentions in an earlier draft of this very note pushed the partition to + // 27 and reded the row — the instrument working exactly as intended. + // + // SECOND merge with main (#7221's typed player-action completion seam and + // its contemporaries). Same rule, applied again: `main` re-derived these to + // `:6640/:6717/:9922` for ITS tree and the branch carried `:6315/:6392/:9606` + // for its own; the merged file measures `:6653/:6730/:9954`, a uniform `+13` + // over main's coordinates. That `+13` is exactly this branch's four + // additions above all three producers: `pub mod reveal_chosen_numbers;` (1), + // the `Effect::RevealChosenNumbers` dispatch arm (3), the + // `QuantityRef::PlayerChosenNumber` arm in `candidate_player_scalar` (5), + // and its arm in main's new `quantity_ref_counts_population_matching` (4). + // It is uniform this time — unlike the first merge — because main's own + // churn moved the depth-0 ledger reset and the third producer together, so + // the branch's extra offset there is already inside main's baseline rather + // than stacked on top of it. + // + // Measure AFTER the last edit to effects/mod.rs, not during: an earlier + // pass here recorded `+9` from a measurement taken before that fourth arm + // was added, and the row caught the 4-line discrepancy. + // Unbounded-number round (same PR): `:6653/:6730/:9954 => + // `:6655/:6732/:9956`, a uniform `+2` — the unbounded-range arm + // added to `compute_options`' sibling classifier in this file, + // which sits above all three producers. Nothing added raises a + // `WaitingFor`; the census set is still exactly 5. + "game/effects/mod.rs:6656".to_string(), + "game/effects/mod.rs:6733".to_string(), + "game/effects/mod.rs:9974".to_string(), // UNMOVED across the rebase, and that is itself evidence the SET did not // move: a census that had gained or lost a producer would not leave this // entry both byte-identical AND at the same coordinate. diff --git a/crates/engine/src/game/engine_phase_trigger_regression_tests.rs b/crates/engine/src/game/engine_phase_trigger_regression_tests.rs index a42b2918a6..c7a6827261 100644 --- a/crates/engine/src/game/engine_phase_trigger_regression_tests.rs +++ b/crates/engine/src/game/engine_phase_trigger_regression_tests.rs @@ -2016,6 +2016,7 @@ fn card_name_choice_validates_against_all_card_names() { let mut state = GameState::new_two_player(42); state.all_card_names = vec!["Lightning Bolt".to_string(), "Counterspell".to_string()].into(); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: crate::types::ability::ChoiceType::CardName, options: Vec::new(), @@ -2034,6 +2035,7 @@ fn card_name_choice_validates_against_all_card_names() { // Reset state for invalid test state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: crate::types::ability::ChoiceType::CardName, options: Vec::new(), @@ -2056,6 +2058,7 @@ fn card_name_choice_is_case_insensitive() { let mut state = GameState::new_two_player(42); state.all_card_names = vec!["Lightning Bolt".to_string()].into(); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: crate::types::ability::ChoiceType::CardName, options: Vec::new(), @@ -3508,6 +3511,7 @@ fn post_replacement_choose_sets_named_choice_waiting_for() { assert!(matches!( waiting_for, Some(WaitingFor::NamedChoice { + free_entry: None, choice_type: crate::types::ability::ChoiceType::BasicLandType, .. }) @@ -3531,6 +3535,7 @@ fn choose_option_with_exact_source_stores_chosen_attribute() { // Set up an exact-object prompt (simulating a persist=true Choose). state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: ChoiceType::color(), options: vec![ @@ -3607,6 +3612,7 @@ fn glacierwood_siege_resolution_prompts_for_anchor_word_choice() { assert!(state.battlefield.contains(&siege_id)); match resolve.waiting_for { WaitingFor::NamedChoice { + free_entry: None, player, choice_type: crate::types::ability::ChoiceType::Labeled { ref options }, source: Some(source), @@ -3641,6 +3647,7 @@ fn restricted_color_choice_rejects_excluded_color() { let mut state = GameState::new_two_player(42); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: ChoiceType::color_excluding(vec![ManaColor::White]), options: vec![ @@ -3909,6 +3916,7 @@ fn echoing_deeps_copying_sunken_citadel_prompts_for_the_copied_color_choice() { ) .expect("Echoing Deeps should copy Sunken Citadel"); let WaitingFor::NamedChoice { + free_entry: None, player, source: Some(source), options, diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index 581e480b4a..92b2158bf0 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -6126,6 +6126,14 @@ pub(super) fn handle_resolution_choice( choice_type, mut source, persist_player, + // MUST stay a wildcard. The published contract is a projection of + // `choice_type`, and validation below consults that single + // authority directly (`accepts_free_entry_answer`) — so a client + // cannot widen its own domain by echoing back a different one. + // Binding this to a literal instead would make the arm miss every + // prompt that HAS a contract, i.e. every free-entry answer would + // fall through to "action not allowed". + free_entry: _, }, GameAction::ChooseOption { choice }, ) => { @@ -6141,6 +6149,18 @@ pub(super) fn handle_resolution_choice( choice ))); } + } else if let Some(accepted) = choice_type.accepts_free_entry_answer(&choice) { + // CR 107.1a/b + CR 608.2d: a free-entry choice has no option list + // to check membership against, so it is validated by RULE instead + // — "a number 0 or greater" accepts any nonnegative integer the + // engine's `i32` quantity domain can represent. Routed through the + // shared authority on `ChoiceType` so the AI's legal-action + // enumeration cannot disagree with this seam about what is legal. + if !accepted { + return Err(EngineError::InvalidAction(format!( + "Invalid number '{choice}' for this choice" + ))); + } } else if !options.contains(&choice) { return Err(EngineError::InvalidAction(format!( "Invalid choice '{}', must be one of: {:?}", @@ -6171,6 +6191,11 @@ pub(super) fn handle_resolution_choice( source.as_mut(), persist_player, ); + // CR 101.4 + CR 608.2d: additionally record a chosen NUMBER on the + // player who chose it, so a later clause can read every player's + // answer back ("the highest number", "each player who didn't choose + // the lowest number"). Additive to the source binding above. + effects::choose::record_player_chosen_number(state, player, &choice_type, &choice); if let Some(context) = updated_context { if let Some(frame) = state.active_ability_continuation_frame_mut() { frame @@ -10094,6 +10119,7 @@ mod tests { Zone::Battlefield, ); let waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(1), choice_type: ChoiceType::CardPredicateGuess { options: ChoiceType::land_or_nonland_card_predicate_options(), @@ -10145,6 +10171,7 @@ mod tests { Zone::Battlefield, ); let waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: ChoiceType::CardPredicate { options: ChoiceType::land_or_nonland_card_predicate_options(), diff --git a/crates/engine/src/game/game_object.rs b/crates/engine/src/game/game_object.rs index 4b38fc56ac..68e0bdf38b 100644 --- a/crates/engine/src/game/game_object.rs +++ b/crates/engine/src/game/game_object.rs @@ -2820,7 +2820,7 @@ impl GameObject { } /// Look up a stored chosen number (e.g., Talion's "choose a number"). - pub fn chosen_number(&self) -> Option { + pub fn chosen_number(&self) -> Option { self.chosen_attributes.iter().find_map(|a| match a { ChosenAttribute::Number(n) => Some(*n), _ => None, diff --git a/crates/engine/src/game/layers.rs b/crates/engine/src/game/layers.rs index e47d22b9c1..784201f71e 100644 --- a/crates/engine/src/game/layers.rs +++ b/crates/engine/src/game/layers.rs @@ -2832,6 +2832,7 @@ fn quantity_ref_reads_zone(qty: &QuantityRef, zone: Zone) -> bool { // Per-turn bend-type tracking (Avatar Aang) — turn history, not a zone read. | QuantityRef::BendTypesThisTurn | QuantityRef::ChosenNumber + | QuantityRef::PlayerChosenNumber { .. } | QuantityRef::ColorsInCommandersColorIdentity | QuantityRef::CommanderCastFromCommandZoneCount | QuantityRef::ConvokedCreatureCount @@ -3154,6 +3155,7 @@ fn quantity_ref_reads_life(qty: &QuantityRef) -> bool { | QuantityRef::LandsPlayedThisTurn { .. } | QuantityRef::TurnsTaken | QuantityRef::ChosenNumber + | QuantityRef::PlayerChosenNumber { .. } | QuantityRef::DescendedThisTurn | QuantityRef::LoyaltyAbilitiesActivatedThisTurn { .. } | QuantityRef::SpellsCastLastTurn diff --git a/crates/engine/src/game/log.rs b/crates/engine/src/game/log.rs index 94a43e8d6b..800e3ba5fb 100644 --- a/crates/engine/src/game/log.rs +++ b/crates/engine/src/game/log.rs @@ -120,6 +120,7 @@ fn importance(event: &GameEvent) -> LogImportance { | GameEvent::Discarded { .. } | GameEvent::Cycled { .. } | GameEvent::CardsRevealed { .. } + | GameEvent::ChosenNumbersRevealed { .. } | GameEvent::BecomesTarget { .. } | GameEvent::ReplacementApplied { .. } | GameEvent::SpeedChanged { .. } @@ -330,6 +331,7 @@ fn tone(event: &GameEvent) -> LogTone { | GameEvent::TurnedFaceUp { .. } | GameEvent::TurnedFaceDown { .. } | GameEvent::CardsRevealed { .. } + | GameEvent::ChosenNumbersRevealed { .. } | GameEvent::CombatDamageDealtToPlayer { .. } | GameEvent::CrimeCommitted { .. } | GameEvent::Regenerated { .. } @@ -533,6 +535,7 @@ fn categorize(event: &GameEvent) -> LogCategory { | GameEvent::Discarded { .. } | GameEvent::Cycled { .. } | GameEvent::CardsRevealed { .. } + | GameEvent::ChosenNumbersRevealed { .. } | GameEvent::Foretold { .. } | GameEvent::BecameForetold { .. } => LogCategory::Zone, @@ -931,6 +934,22 @@ fn format_segments(event: &GameEvent, state: &GameState) -> Vec { text(&card_names.join(", ")), ], + // CR 101.4: one line for the whole simultaneous reveal — the numbers + // become public together, so rendering them per-player would imply an + // ordering the rules do not have. + GameEvent::ChosenNumbersRevealed { numbers } => { + let mut segments = vec![text("Chosen numbers revealed: ")]; + for (index, (player, value)) in numbers.iter().enumerate() { + if index > 0 { + segments.push(text(", ")); + } + segments.push(player_seg(state, *player)); + segments.push(text(" ")); + segments.push(num(crate::game::arithmetic::u32_to_i32_saturating(*value))); + } + segments + } + GameEvent::LifeChanged { player_id, amount } => { if *amount >= 0 { vec![ diff --git a/crates/engine/src/game/public_state.rs b/crates/engine/src/game/public_state.rs index a4b590a4f1..ae79e59185 100644 --- a/crates/engine/src/game/public_state.rs +++ b/crates/engine/src/game/public_state.rs @@ -444,6 +444,7 @@ pub fn mark_public_state_from_events(state: &mut GameState, events: &[GameEvent] | GameEvent::ReplacementApplied { .. } | GameEvent::DayNightChanged { .. } | GameEvent::CardsRevealed { .. } + | GameEvent::ChosenNumbersRevealed { .. } | GameEvent::CombatDamageDealtToPlayer { .. } | GameEvent::PlayerEliminated { .. } | GameEvent::CrimeCommitted { .. } diff --git a/crates/engine/src/game/quantity.rs b/crates/engine/src/game/quantity.rs index 27360103fc..ac1f4fabf3 100644 --- a/crates/engine/src/game/quantity.rs +++ b/crates/engine/src/game/quantity.rs @@ -741,6 +741,7 @@ fn quantity_ref_uses_unspent_mana(qty: &QuantityRef) -> bool { | QuantityRef::ZoneChangeAggregateThisTurn { .. } | QuantityRef::DamageDealtThisTurn { .. } | QuantityRef::ChosenNumber + | QuantityRef::PlayerChosenNumber { .. } | QuantityRef::AttackedThisTurn { .. } | QuantityRef::DescendedThisTurn | QuantityRef::LoyaltyAbilitiesActivatedThisTurn { .. } @@ -1040,6 +1041,7 @@ fn quantity_ref_uses_object_count(qty: &QuantityRef) -> bool { | QuantityRef::ZoneChangeAggregateThisTurn { .. } | QuantityRef::DamageDealtThisTurn { .. } | QuantityRef::ChosenNumber + | QuantityRef::PlayerChosenNumber { .. } | QuantityRef::AttackedThisTurn { .. } | QuantityRef::DescendedThisTurn | QuantityRef::LoyaltyAbilitiesActivatedThisTurn { .. } @@ -1288,6 +1290,7 @@ fn quantity_ref_characteristic_reads(qty: &QuantityRef, depth: u32) -> Character // CR 120.1: damage records store the amount actually dealt. | QuantityRef::DamageDealtThisTurn { .. } | QuantityRef::ChosenNumber + | QuantityRef::PlayerChosenNumber { .. } // CR 508.1: declaration-time attacker snapshots. | QuantityRef::AttackedThisTurn { .. } | QuantityRef::DescendedThisTurn @@ -1520,6 +1523,7 @@ fn entered_object_perturbs_quantity_ref( | QuantityRef::ZoneChangeAggregateThisTurn { .. } | QuantityRef::DamageDealtThisTurn { .. } | QuantityRef::ChosenNumber + | QuantityRef::PlayerChosenNumber { .. } | QuantityRef::AttackedThisTurn { .. } | QuantityRef::DescendedThisTurn | QuantityRef::LoyaltyAbilitiesActivatedThisTurn { .. } @@ -3111,7 +3115,12 @@ fn resolve_ref( .last_named_choice .as_ref() .and_then(|choice| match choice { - crate::types::ability::ChoiceValue::Number(value) => Some(i32::from(*value)), + // CR 107.1a/b: a chosen number can now be any value the rules + // permit, so the conversion into the engine's `i32` quantity + // domain saturates rather than assuming it fits. + crate::types::ability::ChoiceValue::Number(value) => { + Some(crate::game::arithmetic::u32_to_i32_saturating(*value)) + } _ => None, }) .unwrap_or(0), @@ -4383,6 +4392,18 @@ fn resolve_ref( }) }) .unwrap_or(0), + // CR 101.4 + CR 608.2d: the number a PLAYER secretly chose this + // resolution, read off `Player::chosen_attributes`. `AllPlayers { Max }` + // / `{ Min }` fold to "the highest/lowest number" over the players who + // actually chose one (non-choosers are excluded, not counted as 0); + // `ScopedPlayer` is the per-candidate read `PlayerFilter::PlayerAttribute` + // uses to select "each player who chose the highest number". + QuantityRef::PlayerChosenNumber { player: scope } => { + resolve_per_player_scalar_opt(state, scope, controller, ctx, targets, ability, |p| { + p.chosen_number() + .map(crate::game::arithmetic::u32_to_i32_saturating) + }) + } // CR 508.1a: Count creatures that attacked this turn. Declaration-time // records are the authority for every scoped form so attackers that // left the battlefield still count. @@ -6426,6 +6447,79 @@ where } } +/// CR 101.4 + CR 608.2d: `resolve_per_player_scalar` for a scalar that some +/// players simply DON'T HAVE. `extract` returns `None` for such a player, and +/// the aggregate scopes (`Opponent` / `AllPlayers`) then fold over only the +/// players that do — a non-participant is absent from the population, not a +/// zero in it. +/// +/// This matters for `Min`: reading "the lowest number chosen" over a table where +/// only some players chose must not report 0 because a non-chooser was counted. +/// Single-player scopes keep the family's `map_or(0, …)` convention (an absent +/// value reads as 0), so the two helpers agree wherever both are defined. +fn resolve_per_player_scalar_opt( + state: &GameState, + scope: &PlayerScope, + controller: PlayerId, + ctx: QuantityContext, + targets: &[TargetRef], + ability: Option<&ResolvedAbility>, + mut extract: F, +) -> i32 +where + F: FnMut(&crate::types::player::Player) -> Option, +{ + match scope { + // CR 102.2 / CR 102.1: the aggregate populations, narrowed to the + // players that actually have the scalar. + PlayerScope::Opponent { aggregate } => aggregate_over_present_players( + state.players.iter().filter(|p| p.id != controller), + *aggregate, + &mut extract, + ), + PlayerScope::AllPlayers { aggregate, exclude } => { + let excluded_id = exclude.as_deref().and_then(|ex| { + resolve_single_player_scope(state, ex, controller, ctx, targets, ability) + }); + aggregate_over_present_players( + state.players.iter().filter(|p| Some(p.id) != excluded_id), + *aggregate, + &mut extract, + ) + } + // Single-player scopes: delegate to the shared resolver so the "which + // player does this scope name" logic lives in exactly one place. The two + // arms above are the ONLY aggregate scopes; a future aggregate variant + // must be added there as well, or it would fold non-participants in as + // zeroes through this delegation. + single => { + resolve_per_player_scalar(state, single, controller, ctx, targets, ability, |p| { + extract(p).unwrap_or(0) + }) + } + } +} + +/// CR 107.3e: `aggregate_over_players` for a partially-defined scalar — players +/// whose `extract` yields `None` are dropped before the fold. An empty +/// population reduces to 0, matching the total-scalar helper. +fn aggregate_over_present_players<'a, I, F>( + players: I, + aggregate: AggregateFunction, + mut extract: F, +) -> i32 +where + I: IntoIterator, + F: FnMut(&crate::types::player::Player) -> Option, +{ + let values = players.into_iter().filter_map(&mut extract); + match aggregate { + AggregateFunction::Max => values.max().unwrap_or(0), + AggregateFunction::Min => values.min().unwrap_or(0), + AggregateFunction::Sum => values.sum(), + } +} + fn defending_player_for_quantity_context( state: &GameState, ctx: QuantityContext, diff --git a/crates/engine/src/game/resolution_prompt.rs b/crates/engine/src/game/resolution_prompt.rs index 96449f475c..59479b8f54 100644 --- a/crates/engine/src/game/resolution_prompt.rs +++ b/crates/engine/src/game/resolution_prompt.rs @@ -280,6 +280,12 @@ fn effect_offers_choice(e: &Effect) -> bool { | Effect::EachDealsDamageEqualToPower { .. } | Effect::OpponentGuess { .. } | Effect::SwapChosenLabels { .. } + // CR 101.4: `RevealChosenNumbers` publishes an ALREADY-made choice and + // raises no `WaitingFor` of its own. It is nonetheless left in the + // fail-closed group: claiming choice-free is a soundness claim that + // requires a resolver trace and a pinned-guard update, and the only cost + // of `MayPrompt` here is a conservative probe verdict. + | Effect::RevealChosenNumbers { .. } | Effect::Pump { .. } | Effect::PairWith { .. } | Effect::Destroy { .. } diff --git a/crates/engine/src/game/trigger_index.rs b/crates/engine/src/game/trigger_index.rs index b067abf8ca..9e9fccab3e 100644 --- a/crates/engine/src/game/trigger_index.rs +++ b/crates/engine/src/game/trigger_index.rs @@ -667,6 +667,11 @@ pub(crate) fn keys_from_event(event: &GameEvent, state: &GameState) -> Keys { GameEvent::Flipped { .. } => {} GameEvent::DayNightChanged { .. } => push(TriggerEventKey::DayNightChanged), GameEvent::CardsRevealed { .. } => push(TriggerEventKey::Revealed), + // CR 101.4: publishing a chosen number is not CR 701.20 "reveal a card", + // and no printed trigger watches for it, so it keys nothing. Listed + // explicitly (not folded into `Revealed`) so a future "whenever a player + // reveals a card" trigger cannot start firing on a number. + GameEvent::ChosenNumbersRevealed { .. } => {} GameEvent::CrimeCommitted { .. } => push(TriggerEventKey::PlayerActionPerformed), GameEvent::Cycled { .. } => {} GameEvent::PlayerPerformedAction { .. } => push(TriggerEventKey::PlayerActionPerformed), diff --git a/crates/engine/src/game/trigger_matchers.rs b/crates/engine/src/game/trigger_matchers.rs index 7b0a2b72ae..8d1701e3b1 100644 --- a/crates/engine/src/game/trigger_matchers.rs +++ b/crates/engine/src/game/trigger_matchers.rs @@ -1034,6 +1034,7 @@ fn count_matching_trigger_event_subjects( | GameEvent::TurnedFaceUp { .. } | GameEvent::TurnedFaceDown { .. } | GameEvent::CardsRevealed { .. } + | GameEvent::ChosenNumbersRevealed { .. } | GameEvent::CombatDamageDealtToPlayer { .. } | GameEvent::PlayerEliminated { .. } | GameEvent::CrimeCommitted { .. } diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index 725c008bc1..84b151041f 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -11721,6 +11721,7 @@ fn quantity_ref_refs_cost_paid_object(qty: &QuantityRef) -> bool { | QuantityRef::LandsPlayedThisTurn { .. } | QuantityRef::TurnsTaken | QuantityRef::ChosenNumber + | QuantityRef::PlayerChosenNumber { .. } | QuantityRef::DescendedThisTurn | QuantityRef::LoyaltyAbilitiesActivatedThisTurn { .. } | QuantityRef::SpellsCastLastTurn diff --git a/crates/engine/src/game/visibility.rs b/crates/engine/src/game/visibility.rs index 5c91ce0d87..7e7642d610 100644 --- a/crates/engine/src/game/visibility.rs +++ b/crates/engine/src/game/visibility.rs @@ -409,6 +409,7 @@ pub fn filter_state_for_viewer(state: &GameState, viewer: PlayerId) -> GameState options, source, persist_player, + free_entry, } = &state.waiting_for { let mut source = source.clone(); @@ -421,9 +422,34 @@ pub fn filter_state_for_viewer(state: &GameState, viewer: PlayerId) -> GameState options: options.clone(), source, persist_player: *persist_player, + // The free-entry contract is what the prompt PUBLISHES; it carries no + // hidden information (it is a function of `choice_type`, which is + // already public here), so the projection forwards it intact. + free_entry: *free_entry, }; } + // CR 101.4 + CR 101.4b + CR 608.2d: A number a player chose but has not yet + // REVEALED is that player's secret. Wheel of Misfortune, Menacing Ogre and + // Life at Stake all say "secretly", and The Toymaker's Trap's committed + // number must survive an opponent's guess unseen — CR 101.4b would otherwise + // let a later chooser read the earlier answers. + // + // The redaction keys on the ATTRIBUTE KIND, not on the current `waiting_for`: + // `ChosenAttribute::Number` is private, `RevealedNumber` is public, and + // `Effect::RevealChosenNumbers` converts one into the other when the card's + // reveal instruction resolves (CR 608.2c, in written order). Making privacy a + // property of the type means no call path can open a window where a still- + // secret value leaks, and no reveal can be forgotten — a value is visible + // exactly when the game has published it. + for player in filtered.players.iter_mut() { + if !can_view_private_for_player(player.id) { + player.chosen_attributes.retain(|attribute| { + !matches!(attribute, crate::types::ability::ChosenAttribute::Number(_)) + }); + } + } + // CR 608.2d: While an `OpponentGuess` is pending, strip the secret the // guesser must not see so the round-trip can't be auto-won. Two redactions: // @@ -2358,6 +2384,7 @@ mod tests { options: vec!["Anchor".to_string()], }; state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: choice_type.clone(), options: vec!["Anchor".to_string()], @@ -2370,6 +2397,7 @@ mod tests { assert!(matches!( source_less_view.waiting_for, WaitingFor::NamedChoice { + free_entry: _, player: PlayerId(0), ref choice_type, ref options, @@ -2397,6 +2425,7 @@ mod tests { ); let expected_prompt = source.prompt.clone(); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type, options: vec!["Anchor".to_string()], @@ -2408,6 +2437,7 @@ mod tests { assert!(source_bound_view.resolution_stack.is_empty()); match source_bound_view.waiting_for { WaitingFor::NamedChoice { + free_entry: None, source: Some(source), persist_player, .. @@ -5810,6 +5840,120 @@ mod tests { )); } + /// CR 101.4b + CR 608.2d: a number a player chose is that player's secret. + /// The per-player ledger behind `QuantityRef::PlayerChosenNumber` (Wheel of + /// Misfortune's "each player secretly chooses a number 0 or greater") is + /// redacted from every other viewer — and, because it is an engine ledger + /// rather than a rendered fact, it stays redacted regardless of what the game + /// is currently waiting on. A window-scoped rule would leak the moment the + /// prompt closed but the secret was still live (The Toymaker's Trap's + /// committed number, guessed at during an `OpponentGuess`). + /// + /// Fail-on-revert: without the redaction the second chooser's client shows + /// the first chooser's number and the "secret" is free information. + #[test] + fn player_chosen_number_is_private_to_that_player() { + use crate::types::ability::{ChoiceType, ChosenAttribute, NumberDistinctness}; + let mut state = GameState::new_two_player(42); + // P0 has already answered; P1 is the pending chooser. + state.players[0].chosen_attributes = vec![ChosenAttribute::Number(4)]; + state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, + player: PlayerId(1), + choice_type: ChoiceType::NumberRange { + min: 0, + max: Some(20), + distinctness: NumberDistinctness::Repeatable, + }, + options: (0..=20u8).map(|n| n.to_string()).collect(), + source: None, + persist_player: None, + }; + + let chooser_view = filter_state_for_viewer(&state, PlayerId(1)); + assert!( + chooser_view.players[0].chosen_attributes.is_empty(), + "the pending chooser must not see the number already chosen by P0" + ); + let owner_view = filter_state_for_viewer(&state, PlayerId(0)); + assert!( + owner_view.players[0] + .chosen_attributes + .contains(&ChosenAttribute::Number(4)), + "a player always sees their own chosen number" + ); + + // Still redacted once the prompt window has closed — the secret can + // outlive the prompt (a pending guess against it, CR 608.2d). + state.waiting_for = WaitingFor::Priority { + player: PlayerId(0), + }; + assert!( + filter_state_for_viewer(&state, PlayerId(1)).players[0] + .chosen_attributes + .is_empty(), + "privacy is a property of the attribute kind, not of the current prompt" + ); + + // CR 101.4 + CR 608.2c: the OTHER side of the contract. Once the card's + // reveal instruction publishes the number (`Number` → `RevealedNumber`, + // performed by `Effect::RevealChosenNumbers`), every viewer sees it — + // otherwise the engine would keep information secret after the + // instruction that makes it public. + state.players[0].reveal_chosen_number(); + assert!( + filter_state_for_viewer(&state, PlayerId(1)).players[0] + .chosen_attributes + .contains(&ChosenAttribute::RevealedNumber(4)), + "a revealed number must be visible to every player" + ); + assert!( + filter_state_for_viewer(&state, PlayerId(0)).players[0] + .chosen_attributes + .contains(&ChosenAttribute::RevealedNumber(4)), + "revealing must not hide the number from its own chooser" + ); + } + + /// CR 101.4: `reveal_chosen_number` is the single typed transition — it + /// preserves the VALUE (every rules read must agree across the reveal), + /// is idempotent, and is a no-op for a player who chose nothing (CR 609.3), + /// which is what lets a card name every player when only some chose. + #[test] + fn revealing_a_chosen_number_preserves_value_and_tolerates_non_choosers() { + use crate::types::ability::ChosenAttribute; + let mut state = GameState::new_two_player(42); + state.players[0].chosen_attributes = vec![ChosenAttribute::Number(7)]; + + assert_eq!(state.players[0].reveal_chosen_number(), Some(7)); + assert_eq!( + state.players[0].chosen_number(), + Some(7), + "the value a rules read sees is unchanged by the reveal" + ); + assert_eq!( + state.players[0].reveal_chosen_number(), + Some(7), + "revealing an already-revealed number is idempotent" + ); + assert_eq!( + state.players[0] + .chosen_attributes + .iter() + .filter(|a| matches!( + a, + ChosenAttribute::Number(_) | ChosenAttribute::RevealedNumber(_) + )) + .count(), + 1, + "a player holds exactly one chosen number, in exactly one state" + ); + + // A player who chose nothing reveals nothing, and gains no attribute. + assert_eq!(state.players[1].reveal_chosen_number(), None); + assert!(state.players[1].chosen_attributes.is_empty()); + } + /// CR 608.2d: For a `GuessSubject::CommittedChoice` (The Toymaker's Trap), /// only the MOST-RECENTLY committed number is hidden from the guesser — it is /// the secret of the pending guess. Numbers chosen on earlier upkeeps were @@ -5840,7 +5984,7 @@ mod tests { options: (1..=5).map(|n| n.to_string()).collect(), choice_type: ChoiceType::NumberRange { min: 1, - max: 5, + max: Some(5), distinctness: NumberDistinctness::DistinctFromSourceHistory, }, source: crate::types::game_state::OpponentGuessSource { diff --git a/crates/engine/src/parser/oracle_effect/assembly.rs b/crates/engine/src/parser/oracle_effect/assembly.rs index e615abb86f..46a4c5d812 100644 --- a/crates/engine/src/parser/oracle_effect/assembly.rs +++ b/crates/engine/src/parser/oracle_effect/assembly.rs @@ -24,9 +24,10 @@ use crate::parser::oracle_ir::effect_chain::{ use crate::parser::oracle_nom::bridge::nom_on_lower; use crate::parser::oracle_nom::error::OracleError; use crate::types::ability::{ - AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, CastFromZoneDriver, - CastingPermission, ControllerRef, Effect, PlayerFilter, QuantityExpr, QuantityRef, - StaticCondition, SubAbilityLink, TapStateChange, TargetFilter, + AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AggregateFunction, + CastFromZoneDriver, CastingPermission, ChoiceType, Comparator, ControllerRef, DamageChannel, + Effect, PlayerFilter, PlayerScope, QuantityExpr, QuantityRef, StaticCondition, SubAbilityLink, + TapStateChange, TargetFilter, }; use crate::types::game_state::TargetSelectionConstraint; use crate::types::zones::Zone; @@ -1329,6 +1330,134 @@ fn damage_amount_reads_event_context(effect: &Effect) -> bool { reads } +/// The recipient of a single-recipient scalar instruction — the position a +/// "… to them" / "… they lose" player anaphor occupies. Paired with +/// [`scalar_amount_mut`], which reads the "that much" position of the same +/// instruction; split in two because the binding below needs the recipient +/// immutably to decide, then the amount mutably to rewrite. +fn scalar_recipient(effect: &Effect) -> Option<&TargetFilter> { + match effect { + Effect::DealDamage { target, .. } => Some(target), + Effect::GainLife { player, .. } => Some(player), + Effect::LoseLife { target, .. } => target.as_ref(), + _ => None, + } +} + +fn scalar_amount_mut(effect: &mut Effect) -> Option<&mut QuantityExpr> { + match effect { + Effect::DealDamage { amount, .. } + | Effect::GainLife { amount, .. } + | Effect::LoseLife { amount, .. } => Some(amount), + _ => None, + } +} + +/// CR 109.4: the chain index of the `Choose(Player)` clause a recipient anaphor +/// names, if it names one. A resolution-time chosen player is carried as a +/// player-only `Typed` filter whose controller is `ChosenPlayer { index }` — +/// the shape `subject::chosen_player_anaphor_filter` is the sole producer of. +fn chosen_player_anaphor_index(filter: &TargetFilter) -> Option { + match filter { + TargetFilter::Typed(tf) => match tf.controller { + Some(ControllerRef::ChosenPlayer { index }) => Some(index), + _ => None, + }, + _ => None, + } +} + +/// CR 101.4 + CR 107.1a: The extremum a "choose a player with the highest/lowest +/// number" restriction selected by, if that is what this player filter is. +/// +/// Recognizes exactly the shape `lower::chosen_number_player_filter` emits, and +/// only under `EQ` — under `NE` ("an opponent who DIDN'T choose the highest +/// number") the extremum is provably *not* the chosen player's number, so there +/// is nothing to bind and this returns `None`. +fn chosen_number_selection_extremum(filter: &PlayerFilter) -> Option { + let PlayerFilter::PlayerAttribute { + attr, + comparator: Comparator::EQ, + value, + .. + } = filter + else { + return None; + }; + if !matches!( + **attr, + QuantityRef::PlayerChosenNumber { + player: PlayerScope::ScopedPlayer + } + ) { + return None; + } + match &**value { + QuantityExpr::Ref { + qty: + QuantityRef::PlayerChosenNumber { + player: + PlayerScope::AllPlayers { + aggregate, + exclude: None, + }, + }, + } => Some(*aggregate), + _ => None, + } +} + +/// CR 608.2c + CR 101.4: Bind the "that much" of a +/// *"Choose an opponent with the highest number. ~ deals that much damage to +/// them."* continuation (Itazura, Lingering Wick) to the number that selection +/// was made by. +/// +/// `EventContextAmount` means "the amount the surrounding event supplies", and a +/// resolving spell supplies none — left alone the instruction silently deals 0. +/// The antecedent is provable from the assembled chain rather than guessable at +/// lowering time, which is why this runs here: the recipient anaphor names a +/// specific `Choose(Player)` clause by index, and that clause's own restriction +/// says which extremum it selected by. Both halves must agree, so an +/// intervening unrestricted player choice (which would shift the index) simply +/// fails to match and nothing is rebound. +/// +/// The bound reference is the chain-wide extremum, not a read of the chosen +/// player's own number: the `EQ` restriction guarantees the chosen player HOLDS +/// that extremum, so the two are equal by construction, and expressing it this +/// way reuses the `PlayerChosenNumber` scalar the restriction itself is built +/// from instead of minting a resolution-scoped `PlayerScope::ChosenPlayer` whose +/// only consumer would be this binding. +fn bind_chosen_number_anaphor(def: &mut AbilityDefinition, prior: &[AbilityDefinition]) { + let Some(index) = scalar_recipient(&def.effect).and_then(chosen_player_anaphor_index) else { + return; + }; + let mut player_choices = prior.iter().filter_map(|d| match &*d.effect { + Effect::Choose { + choice_type: choice_type @ (ChoiceType::Player { .. } | ChoiceType::Opponent { .. }), + .. + } => Some(choice_type), + _ => None, + }); + let Some(ChoiceType::Opponent { + restriction: Some(restriction), + .. + }) = player_choices.nth(index as usize) + else { + return; + }; + let Some(aggregate) = chosen_number_selection_extremum(restriction) else { + return; + }; + if let Some(amount) = scalar_amount_mut(def.effect.as_mut()) { + amount.rebind_event_context_amount(&QuantityRef::PlayerChosenNumber { + player: PlayerScope::AllPlayers { + aggregate, + exclude: None, + }, + }); + } +} + pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition { let kind = ir.kind; let continuation_kind = ir.continuation_kind.unwrap_or(AbilityKind::Spell); @@ -2074,6 +2203,9 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition { // not a different per-recipient event-context amount. Bind only this // explicit continuation relationship; event-fed and per-player-scoped // `EventContextAmount` consumers retain their ordinary meaning. + // CR 608.2c + CR 101.4: bind a "that much … to them" pair back to the + // number the earlier player selection was made by (Itazura). + bind_chosen_number_anaphor(&mut def, &defs); if let Some(prev) = defs.last() { if def.sub_link == SubAbilityLink::ContinuationStep && prev.sub_ability.is_none() @@ -2082,7 +2214,9 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition { && damage_amount_reads_event_context(&def.effect) { if let Effect::DamageEachPlayer { amount, .. } = def.effect.as_mut() { - amount.rebind_event_context_amount_to_previous_effect(); + amount.rebind_event_context_amount(&QuantityRef::PreviousEffectAmount { + channel: DamageChannel::Total, + }); } } } @@ -3088,6 +3222,10 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition { // CR 607.2d: fill every committed-choice guess with the head Choose's domain. super::propagate_committed_choice_type_to_guesses(&mut result); + // CR 607.2d + CR 101.4: persist a secretly-chosen number whenever a later + // clause in the same chain reads it back ("the highest number", "each player + // who didn't choose the lowest number"). + super::promote_chosen_number_persistence(&mut result); // CR 608.2d: gate the whole "if they guessed wrong/right" branch, including // any "and ..." continuation steps. super::propagate_guess_branch_condition_to_continuations(&mut result); diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index e1c4710543..e389828c18 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -4070,7 +4070,17 @@ pub(super) fn parse_choose_ast( } } - if let Some(choice_type) = super::try_parse_named_choice(lower) { + // CR 107.1a/b: a "with the highest number" restriction on an opponent choice + // only means the secretly-chosen number when this ability actually made one. + // `pending_choice_type` is the chunk-loop-threaded record of the last + // `Effect::Choose` domain, the same provenance the quantity path gates on. + let has_number_choice = matches!( + ctx.pending_choice_type, + Some(crate::types::ability::ChoiceType::NumberRange { .. }) + ); + if let Some(choice_type) = + super::try_parse_named_choice_with_provenance(lower, has_number_choice) + { // CR 608.2d (override) + CR 701.9b (analogous): "choose a player at // random" (Strax) — the game selects the referent, not the controller. let selection = if nom_primitives::scan_contains(lower, "at random") { diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index 70137309d8..0fd8850fb9 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -4428,6 +4428,18 @@ pub(super) fn strip_each_player_subject(text: &str) -> (Option, St return (Some(attr_scope), deconjugated); } + // CR 101.4 + CR 608.2d: "who [didn't] chose/choose the highest/lowest + // number" restricts the player set to those whose secretly-chosen number + // matches (or fails to match) the cross-player extremum — Wheel of + // Misfortune's "each player who didn't choose the lowest number discards + // their hand, then draws seven cards", Life at Stake's "each player who + // chose the highest number loses that much life". Sibling of the attribute + // clause above and consumed on the same terms. + if let Some((chosen_scope, after_clause)) = strip_chosen_number_clause(&scope, rest) { + let deconjugated = subject::deconjugate_verb(&after_clause); + return (Some(chosen_scope), deconjugated); + } + // CR 608.2c + CR 109.5: A "who [verb]ed … this way" relative clause after // "each player" / "each opponent" restricts the affected set to the players // who performed the tracked action during THIS resolution (Kwain, Itinerant @@ -4535,13 +4547,21 @@ pub(super) fn strip_each_player_subject(text: &str) -> (Option, St ); } - // CR 608.2c: A leading "also" after a resolved player-scope subject - // ("each opponent also discards a card") is a continuation adverb with no - // semantic weight — the same additive connector handled for self-ref - // subjects in `parse_effect_clause_inner`. Strip it via `tag()` so the - // residual ("discards a card") deconjugates and dispatches normally. + // CR 608.2c: A leading manner/continuation adverb after a resolved + // player-scope subject carries no AST weight — strip it via `tag()` so the + // residual deconjugates and dispatches normally. + // + // - "also" ("each opponent also discards a card") is the additive connector + // also handled for self-ref subjects in `parse_effect_clause_inner`. + // - CR 101.4 + CR 608.2d: "secretly" (Wheel of Misfortune's "each player + // secretly chooses a number 0 or greater"; Menacing Ogre's "each player + // secretly chooses a number") marks the choice as hidden from the other + // choosers. That is a VISIBILITY property, enforced at the state-filtering + // seam (`game::visibility` keeps each player's `ChosenAttribute::Number` + // private to that player), not a distinct effect — so the choice itself + // parses exactly like an open one. let rest = nom_on_lower(rest, &rest_condition_lower, |i| { - value((), tag("also ")).parse(i) + value((), alt((tag("also "), tag("secretly ")))).parse(i) }) .map(|((), after)| after) .unwrap_or(rest); @@ -5086,6 +5106,139 @@ fn strip_player_attribute_clause( )) } +/// CR 101.4 + CR 608.2d: a player-set restriction keyed on the number each +/// player secretly chose during this resolution. Returns the comparator and the +/// extremum to compare against: +/// +/// - `"who chose the highest number"` → `(EQ, Max)` — Life at Stake. +/// - `"who didn't choose the lowest number"` → `(NE, Min)` — Wheel of Misfortune. +/// - `"with the highest number"` → `(EQ, Max)` — Menacing Ogre's participial form. +/// - `"who chose that number"` → `(EQ, anaphor)` — Wheel of Misfortune's damage +/// recipient, where "that number" refers back to the extremum the same clause +/// already named as its amount. +/// +/// Composed by axis (polarity × verb form × extremum), not enumerated as +/// permutations, so a new phrasing on any one axis costs one `tag`. `anaphor` +/// supplies the referent for "that number"; `None` disables that arm, which is +/// correct wherever no extremum is in scope to anaphor back to. +/// A trailing `" of "` disqualifies EVERY arm: "the highest number OF cards in +/// hand" is a counting phrase over a population, not a reference to a chosen +/// number. This is the same guard the quantity-side `parse_extreme_chosen_number_ref` +/// carries — without it here, "choose an opponent with the highest number of +/// cards in hand" binds a chosen-number comparison to a card that has no choice +/// in it, the Custodi Peacekeeper failure one layer over. +pub(crate) fn parse_chosen_number_restriction( + i: &str, + anaphor: Option, +) -> OracleResult<'_, (Comparator, AggregateFunction)> { + terminated( + parse_chosen_number_restriction_body(anaphor), + not(tag(" of ")), + ) + .parse(i) +} + +fn parse_chosen_number_restriction_body( + anaphor: Option, +) -> impl FnMut(&str) -> OracleResult<'_, (Comparator, AggregateFunction)> { + move |i: &str| { + alt(( + map( + ( + tag("who "), + alt(( + value( + Comparator::NE, + alt((tag("didn't "), tag("did not "), tag("doesn't "))), + ), + nom::combinator::success(Comparator::EQ), + )), + alt((tag("chose "), tag("chooses "), tag("choose "))), + tag("the "), + nom_quantity::parse_chosen_number_extremum, + nom_quantity::parse_chosen_number_noun, + ), + |(_, comparator, _, _, aggregate, ())| (comparator, aggregate), + ), + map( + terminated( + preceded(tag("with the "), nom_quantity::parse_chosen_number_extremum), + nom_quantity::parse_chosen_number_noun, + ), + |aggregate| (Comparator::EQ, aggregate), + ), + nom::combinator::map_opt( + terminated( + tag("who chose that"), + nom_quantity::parse_chosen_number_noun, + ), + move |_| anaphor.map(|aggregate| (Comparator::EQ, aggregate)), + ), + )) + .parse(i) + } +} + +/// CR 101.4 + CR 608.2d: the `PlayerFilter` selecting the players whose +/// secretly-chosen number compares (under `comparator`) to the cross-player +/// extremum of the same scalar. +/// +/// Reuses the existing parameterized [`PlayerFilter::PlayerAttribute`] rather +/// than minting a "chose the highest number" variant: the per-candidate scalar +/// is [`QuantityRef::PlayerChosenNumber`] under `ScopedPlayer` (read off each +/// candidate by `effects::candidate_player_scalar`), and the threshold is the +/// same reference under `AllPlayers { aggregate }`. "Didn't choose the lowest" +/// is therefore just `Comparator::NE` — no negation wrapper, no `AllExcept`. +pub(crate) fn chosen_number_player_filter( + relation: crate::types::ability::PlayerRelation, + comparator: Comparator, + aggregate: AggregateFunction, +) -> PlayerFilter { + use crate::types::ability::PlayerScope; + PlayerFilter::PlayerAttribute { + relation, + attr: Box::new(QuantityRef::PlayerChosenNumber { + player: PlayerScope::ScopedPlayer, + }), + comparator, + value: Box::new(QuantityExpr::Ref { + qty: QuantityRef::PlayerChosenNumber { + player: PlayerScope::AllPlayers { + aggregate, + exclude: None, + }, + }, + }), + } +} + +/// CR 101.4 + CR 608.2d + CR 109.5: Strip a chosen-number relative clause after +/// an "each player" / "each opponent" subject ("Each player who didn't choose +/// the lowest number discards their hand"). Returns the narrowed scope and the +/// verb-phrase remainder. Structural sibling of `strip_player_attribute_clause`: +/// same `PlayerAttribute` shape, different per-candidate scalar. Like every +/// relative clause in this dispatcher the clause MUST be consumed and reflected +/// in the scope — dropping it would apply the effect to every player. +fn strip_chosen_number_clause(base: &PlayerFilter, rest: &str) -> Option<(PlayerFilter, String)> { + use crate::types::ability::PlayerRelation; + let relation = match base { + PlayerFilter::Opponent => PlayerRelation::Opponent, + PlayerFilter::All => PlayerRelation::All, + _ => return None, + }; + let lower = rest.to_lowercase(); + let ((comparator, aggregate), remainder) = + nom_on_lower(rest, &lower, |i| parse_chosen_number_restriction(i, None))?; + let verb_phrase = remainder.trim_start(); + if verb_phrase.is_empty() { + return None; + } + Some(( + chosen_number_player_filter(relation, comparator, aggregate), + verb_phrase.to_string(), + )) +} + /// CR 608.2c + CR 109.5: Strip a "who [verb]ed … this way" relative clause after /// an "each opponent"/"each player" subject. Returns /// `PlayerFilter::PerformedActionThisWay` (carrying the base subject's relation @@ -5230,6 +5383,52 @@ pub(crate) fn parse_damage_each_player_scope(text: &str) -> Option .then_some(filter) } +/// CR 101.4 + CR 120.3 + CR 608.2d: an all-consuming damage-recipient scope +/// narrowed by a chosen-number relative clause — "each player who chose that +/// number" (Wheel of Misfortune), "each player who chose the highest number". +/// +/// `anaphor` is the extremum the enclosing clause already named as its damage +/// amount, which is what an anaphoric "that number" refers to; pass `None` where +/// the amount is not a chosen-number extremum, and the anaphoric arm declines +/// (so the clause falls through to the unnarrowed scopes instead of silently +/// binding the wrong referent). +fn parse_damage_each_chosen_number_scope( + text: &str, + anaphor: Option, +) -> Option { + use crate::types::ability::PlayerRelation; + let (rest, base) = preceded(tag("each "), parse_damage_player_scope) + .parse(text) + .ok()?; + let relation = match base { + PlayerFilter::Opponent => PlayerRelation::Opponent, + PlayerFilter::All => PlayerRelation::All, + _ => return None, + }; + let (rest, (comparator, aggregate)) = + preceded(multispace1, |i| parse_chosen_number_restriction(i, anaphor)) + .parse(rest) + .ok()?; + rest.chars() + .all(|c| c.is_ascii_whitespace() || c.is_ascii_punctuation()) + .then(|| chosen_number_player_filter(relation, comparator, aggregate)) +} + +/// CR 608.2c: the cross-player extremum a quantity names, when it is one. This +/// is the referent an anaphoric "that number" in the same clause points back to +/// — read off the already-parsed AST rather than re-matching Oracle text. +fn chosen_number_extremum_of(amount: &QuantityExpr) -> Option { + match amount { + QuantityExpr::Ref { + qty: + QuantityRef::PlayerChosenNumber { + player: crate::types::ability::PlayerScope::AllPlayers { aggregate, .. }, + }, + } => Some(*aggregate), + _ => None, + } +} + /// CR 120.2b + CR 120.3 + CR 102.2: leading "each opponent/player/foe/other /// opponent/other player" damage scope, returning the matched filter AND the /// unconsumed remainder. Unlike `parse_damage_each_player_scope` it is NOT @@ -7903,6 +8102,25 @@ pub(super) fn try_parse_damage_with_remainder<'a>( rem, )); } + // CR 101.4 + CR 120.3: "… to each player who chose that number" (Wheel of + // Misfortune). The recipient set is keyed on the secretly-chosen numbers, + // and the anaphoric "that number" points back to the extremum THIS clause + // already named as its amount — resolved structurally from the parsed + // `amount`, never by re-reading the Oracle phrase. Tried before the plain + // each-player scope so the relative clause is consumed rather than left as + // a remainder that would widen the damage to every player. + if let Some(player_filter) = parse_damage_each_chosen_number_scope( + after_to_for_classification, + chosen_number_extremum_of(&amount), + ) { + return Some(( + Effect::DamageEachPlayer { + amount, + player_filter, + }, + "", + )); + } if let Some(player_filter) = parse_damage_each_player_scope(after_to_for_classification) { return Some(( Effect::DamageEachPlayer { @@ -8205,6 +8423,17 @@ fn resolve_player_anaphor_damage_recipient( { return Some(filter); } + // CR 608.2c + CR 109.4: "Choose an opponent …. ~ deals that much damage to + // them." — the recipient is the player the earlier `Choose(Player)` clause + // selected, carried on `relative_player_scope` across the sentence boundary. + // Same single-authority binding the subject-position "they" anaphor uses + // (`resolve_they_pronoun`), so both pronoun positions in this card class + // (Itazura, Lingering Wick; Gluntch, the Bestower) name the same player. + if let Some(filter) = + super::subject::chosen_player_anaphor_filter(ctx.relative_player_scope.as_ref()) + { + return Some(filter); + } match ctx.relative_player_scope { Some(ControllerRef::ScopedPlayer) => Some(TargetFilter::ScopedPlayer), Some(ControllerRef::ParentTargetController) => Some(TargetFilter::ParentTargetController), diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 10c58150fa..8ca74e9196 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -852,6 +852,162 @@ pub(super) fn propagate_committed_choice_type_to_guesses(ability: &mut AbilityDe } } +/// CR 607.2d + CR 101.4: A `NumberRange` choice must PERSIST whenever a later +/// clause in the same resolution reads the chosen number back. Without +/// persistence the answer is dropped the instant the prompt is answered, and +/// every downstream "the highest number" / "who chose the lowest number" +/// reference silently resolves against nothing. +/// +/// The `ChooseImperativeAst` lowering can't make this call: at that point the +/// clause knows only its own choice type, not whether a LATER chunk consumes it. +/// This post-pass answers it on the ASSEMBLED tree, so the rule the persist +/// decision has always claimed to follow — "persist whenever a later clause +/// refers back to it" — is enforced structurally rather than by a choice-type +/// whitelist. Sibling of `propagate_committed_choice_type_to_guesses`, run from +/// the same chokepoint. +pub(super) fn promote_chosen_number_persistence(ability: &mut AbilityDefinition) { + if definition_reads_player_chosen_number(ability) { + persist_number_choices(ability); + } +} + +fn definition_reads_player_chosen_number(def: &AbilityDefinition) -> bool { + if def + .player_scope + .as_ref() + .is_some_and(player_filter_reads_player_chosen_number) + { + return true; + } + let mut found = false; + def.effect.for_each_quantity_expr(&mut |expr| { + found = found || quantity_expr_reads_player_chosen_number(expr); + }); + if found { + return true; + } + // CR 120.3: `DamageEachPlayer`'s recipient set is a `PlayerFilter` rather + // than a quantity, so it is not reached by `for_each_quantity_expr`. + if let Effect::DamageEachPlayer { player_filter, .. } = def.effect.as_ref() { + if player_filter_reads_player_chosen_number(player_filter) { + return true; + } + } + // CR 608.2d: a `Choose(Opponent)`'s RESTRICTION is a read site too — "choose + // an opponent with the highest number" narrows the option list by comparing + // each candidate's chosen number, so the choice it reads must persist. The + // filter lives inside the `ChoiceType`, which no quantity walk reaches; this + // is the same class of miss as the condition arm below. + if let Effect::Choose { + choice_type: + ChoiceType::Opponent { + restriction: Some(restriction), + .. + }, + .. + } = def.effect.as_ref() + { + if player_filter_reads_player_chosen_number(restriction) { + return true; + } + } + // CR 608.2c: a link's CONDITION is a read site too. "If you are one of those + // players …" gates on the chosen number just as surely as an amount does, and + // the condition is evaluated DURING the same resolution — so a chain whose + // only reference lives in a condition still needs the upstream choice to + // persist, or the answer is cleared before the condition is evaluated and the + // gate silently reads against nothing. + if def + .condition + .as_ref() + .is_some_and(condition_reads_player_chosen_number) + { + return true; + } + def.sub_ability + .as_deref() + .is_some_and(definition_reads_player_chosen_number) + || def + .else_ability + .as_deref() + .is_some_and(definition_reads_player_chosen_number) +} + +/// CR 608.2c: Does this condition read a secretly-chosen number? Recurses +/// through the boolean combinators so a reference nested inside +/// `And`/`Or`/`Not`/`ConditionInstead` is found — a shallow check would miss +/// exactly the compound gates ("if you chose the highest number and …") that +/// make the reference worth having. +fn condition_reads_player_chosen_number(condition: &AbilityCondition) -> bool { + match condition { + AbilityCondition::QuantityCheck { lhs, rhs, .. } => { + quantity_expr_reads_player_chosen_number(lhs) + || quantity_expr_reads_player_chosen_number(rhs) + } + AbilityCondition::ConditionInstead { inner } => condition_reads_player_chosen_number(inner), + AbilityCondition::Not { condition } => condition_reads_player_chosen_number(condition), + AbilityCondition::And { conditions } | AbilityCondition::Or { conditions } => { + conditions.iter().any(condition_reads_player_chosen_number) + } + // Every other condition gates on board/turn/event facts and carries no + // embedded quantity, so it cannot name a chosen number. + _ => false, + } +} + +fn player_filter_reads_player_chosen_number(filter: &PlayerFilter) -> bool { + match filter { + PlayerFilter::PlayerAttribute { attr, value, .. } => { + matches!(**attr, QuantityRef::PlayerChosenNumber { .. }) + || quantity_expr_reads_player_chosen_number(value) + } + PlayerFilter::ControlsCount { count, .. } => { + quantity_expr_reads_player_chosen_number(count) + } + PlayerFilter::AllExcept { exclude } => player_filter_reads_player_chosen_number(exclude), + // Every other player filter selects on board/turn/event facts and + // carries no embedded quantity, so it cannot name a chosen number. + _ => false, + } +} + +fn quantity_expr_reads_player_chosen_number(expr: &QuantityExpr) -> bool { + match expr { + QuantityExpr::Ref { qty } => matches!(qty, QuantityRef::PlayerChosenNumber { .. }), + QuantityExpr::Fixed { .. } => false, + QuantityExpr::DivideRounded { inner, .. } + | QuantityExpr::Offset { inner, .. } + | QuantityExpr::ClampMin { inner, .. } + | QuantityExpr::Multiply { inner, .. } + | QuantityExpr::UpTo { max: inner } => quantity_expr_reads_player_chosen_number(inner), + QuantityExpr::Power { exponent, .. } => quantity_expr_reads_player_chosen_number(exponent), + QuantityExpr::Difference { left, right } => { + quantity_expr_reads_player_chosen_number(left) + || quantity_expr_reads_player_chosen_number(right) + } + QuantityExpr::Sum { exprs } | QuantityExpr::Max { exprs } => { + exprs.iter().any(quantity_expr_reads_player_chosen_number) + } + } +} + +fn persist_number_choices(def: &mut AbilityDefinition) { + if let Effect::Choose { + choice_type: ChoiceType::NumberRange { .. }, + persist, + .. + } = def.effect.as_mut() + { + *persist = true; + } + if let Some(sub) = def.sub_ability.as_deref_mut() { + persist_number_choices(sub); + } + if let Some(els) = def.else_ability.as_deref_mut() { + persist_number_choices(els); + } +} + fn find_head_committed_guess_choice_type(ability: &AbilityDefinition) -> Option { if let Effect::Choose { choice_type, .. } = ability.effect.as_ref() { if choice_type_is_committed_guess_domain(choice_type) { @@ -7855,13 +8011,53 @@ fn try_parse_choose_player_to_verb( // restriction to the `Opponent` choice so it stays a single pick (CR 608.2d // resolves ties) rather than fanning out. Consume the qualifier so it is not // left dangling on the verb tail. + // + // CR 101.4 + CR 608.2c: the same seam carries "choose an opponent WITH THE + // HIGHEST NUMBER" (Itazura, Lingering Wick), which narrows the pick to the + // opponent(s) who chose the cross-player maximum. Dropping that qualifier + // would let the controller pick an opponent who did NOT choose the highest + // and then damage that illegal choice, so it is bound, not discarded. It + // reuses the same restriction grammar and `PlayerFilter` builder as the + // "each player who chose the highest number" subject path, and is gated on + // provenance — with no preceding secret-number choice in this ability there + // is nothing for "the highest number" to refer to. + let has_number_choice = matches!( + ctx.pending_choice_type, + Some(ChoiceType::NumberRange { .. }) + ); let after_player = if let ChoiceType::Opponent { restriction, .. } = &mut choice_type { match parse_opponent_most_life_restriction(after_player) { Ok((rest, filter)) => { *restriction = Some(Box::new(filter)); rest } - Err(_) => after_player, + Err(_) => { + let chosen_number = has_number_choice + .then(|| { + let (after, _) = + tag::<_, _, OracleError<'_>>(" ").parse(after_player).ok()?; + lower::parse_chosen_number_restriction(after, None) + .ok() + .map(|(rest, (comparator, aggregate))| { + ( + rest, + lower::chosen_number_player_filter( + crate::types::ability::PlayerRelation::Opponent, + comparator, + aggregate, + ), + ) + }) + }) + .flatten(); + match chosen_number { + Some((rest, filter)) => { + *restriction = Some(Box::new(filter)); + rest + } + None => after_player, + } + } } } else { after_player @@ -8157,9 +8353,14 @@ fn effect_has_guess_outcome_authority(effect: &Effect, has_choice: bool) -> bool fn is_placeholder_committed_guess_choice(choice_type: &ChoiceType) -> bool { matches!( choice_type, + // The sentinel is `max: Some(0)` — an empty-in-practice range containing + // only 0, which no card text produces. It must NOT be `max: None`: since + // CR 107.1a/b made that the real shape of "choose a number 0 or greater", + // a `None` sentinel would classify every genuine unbounded choice as an + // unfilled placeholder. ChoiceType::NumberRange { min: 0, - max: 0, + max: Some(0), distinctness: NumberDistinctness::Repeatable, } ) @@ -8594,28 +8795,59 @@ fn parse_effect_clause_inner(text: &str, ctx: &mut ParseContext) -> ParsedEffect return clause; } - // CR 608.2d: "[then you] reveal the number you chose" — revealing the - // secretly-committed value. The engine models the secret as a redacted - // `ChosenAttribute::Number` (visibility.rs) that becomes public the moment - // the guess is answered, so the explicit reveal is an engine-level - // consequence with no separate effect — a no-op at the AST layer. + // CR 101.4 + CR 608.2c: "[then you] reveal the number you chose" / "all + // players reveal those numbers simultaneously and determine the highest and + // lowest numbers revealed this way" / "then those numbers are revealed" — + // publishing the secretly-committed value(s). + // + // This is a real state transition, not bookkeeping: a chosen number is + // PRIVATE to its chooser until published, so the instruction that publishes + // it must be modeled or the engine keeps information secret after the card + // made it public. `Effect::RevealChosenNumbers` performs that conversion + // (`ChosenAttribute::Number` → `RevealedNumber`), which `game::visibility` + // reads. The subject selects WHOSE numbers: "you" publishes only the + // controller's (The Toymaker's Trap), an unscoped or "all players" subject + // publishes everyone's. + // + // The trailing "and determine the highest and lowest numbers" IS pure + // bookkeeping and is consumed without effect — the extrema are computed on + // demand by `QuantityRef::PlayerChosenNumber` under an + // `AllPlayers { aggregate }` scope, never stored. { let lower = text.to_ascii_lowercase(); - let body = opt(value((), tag::<_, _, OracleError<'_>>("you "))) - .parse(lower.as_str()) - .map(|(rest, _)| rest) - .unwrap_or(lower.as_str()); - if alt(( + let (body, players) = opt(alt(( value( - (), - tag::<_, _, OracleError<'_>>("reveal the number you chose"), + PlayerFilter::Controller, + tag::<_, _, OracleError<'_>>("you "), ), - value((), tag::<_, _, OracleError<'_>>("reveal the chosen number")), - )) - .parse(body) - .is_ok() + value(PlayerFilter::All, tag("all players ")), + value(PlayerFilter::All, tag("each player ")), + ))) + .parse(lower.as_str()) + .map(|(rest, subject)| { + ( + rest, + subject + .unwrap_or_else(crate::game::effects::reveal_chosen_numbers::default_players), + ) + }) + .unwrap_or_else(|_: nom::Err>| { + ( + lower.as_str(), + crate::game::effects::reveal_chosen_numbers::default_players(), + ) + }); + // COMPLETE-CLAUSE consumption. A prefix match would lower the whole chunk + // to a bare reveal and silently discard whatever followed — the swallow + // this parser is built to avoid. Only trailing punctuation and whitespace + // may remain, so a clause the grammar does not fully model falls through + // to the general dispatcher (and, if nothing claims it, to an honest + // `Unimplemented`) instead of being quietly truncated. + if all_consuming(parse_reveal_chosen_numbers_clause) + .parse(body.trim().trim_end_matches(['.', ',']).trim()) + .is_ok() { - return parsed_clause(Effect::NoOp); + return parsed_clause(Effect::RevealChosenNumbers { players }); } } @@ -25138,7 +25370,7 @@ fn try_parse_guess_clause(text: &str, ctx: &ParseContext) -> Option Option NumberDistinctness { } pub(crate) fn try_parse_named_choice(lower: &str) -> Option { + try_parse_named_choice_with_provenance(lower, false) +} + +/// CR 608.2d + CR 107.1a/b: `try_parse_named_choice`, told whether a +/// secretly-chosen number already exists in this ability. +/// +/// Only the imperative dispatcher can answer that (it holds `ParseContext`), and +/// only one phrase needs it: "choose an opponent WITH THE HIGHEST NUMBER" +/// (Itazura). Without the provenance flag that restriction would be bound by +/// wording alone — the failure mode that rewrote Custodi Peacekeeper — so the +/// default entry point passes `false` and the restriction simply is not offered. +pub(crate) fn try_parse_named_choice_with_provenance( + lower: &str, + has_number_choice: bool, +) -> Option { let (rest, _) = alt(( tag::<_, _, OracleError<'_>>("choose "), nom::sequence::preceded(tag("secretly "), tag("choose ")), )) .parse(lower) .ok()?; - parse_named_choice_object(rest) + parse_named_choice_object_with_provenance(rest, has_number_choice) } /// The object phrase of a named choice, with any leading "choose "/"secretly @@ -25305,6 +25552,13 @@ pub(crate) fn try_parse_named_choice(lower: &str) -> Option { /// Paper) can re-dispatch each conjunct through this same phrase table instead /// of duplicating it or reconstructing a "choose "-prefixed string. pub(crate) fn parse_named_choice_object(rest: &str) -> Option { + parse_named_choice_object_with_provenance(rest, false) +} + +pub(crate) fn parse_named_choice_object_with_provenance( + rest: &str, + has_number_choice: bool, +) -> Option { type E<'a> = OracleError<'a>; if tag::<_, _, E>("a creature type").parse(rest).is_ok() { Some(ChoiceType::creature_type()) @@ -25350,7 +25604,10 @@ pub(crate) fn parse_named_choice_object(rest: &str) -> Option { } else if let Ok((range_rest, _)) = tag::<_, _, E>("a number between ").parse(rest) { // "choose a number between 1 and 5 [that hasn't been chosen]" let mut parts = range_rest.splitn(3, ' '); - let min = parts.next().and_then(|s| s.parse::().ok()).unwrap_or(0); + let min = parts + .next() + .and_then(|s| s.parse::().ok()) + .unwrap_or(0); let and = parts.next(); // Split the leading max digits from any trailing distinctness clause // ("...5 that hasn't been chosen") with a nom digit combinator so the @@ -25358,45 +25615,52 @@ pub(crate) fn parse_named_choice_object(rest: &str) -> Option { // impossible option), not silently dropped. let max_token = parts.next().unwrap_or(""); let (max, tail) = match nom::character::complete::digit1::<_, ()>(max_token) { - Ok((rest_after, digits)) => (digits.parse::().unwrap_or(20), rest_after), - Err(_) => (20, ""), + Ok((rest_after, digits)) => (digits.parse::().ok(), rest_after), + Err(_) => (None, ""), }; let distinctness = parse_number_distinctness(tail); - if and == Some("and") { - Some(ChoiceType::NumberRange { + // CR 107.1a: "between X and Y" states BOTH bounds, so a missing/unparsable + // upper token means the phrase was not actually this shape — decline + // rather than substituting an invented ceiling. + match (and, max) { + (Some("and"), Some(max)) => Some(ChoiceType::NumberRange { min, - max, + max: Some(max), distinctness, - }) - } else { - None + }), + _ => None, } } else if let Ok((gt_rest, _)) = tag::<_, _, E>("a number greater than ").parse(rest) { - // "choose a number greater than 0" — open-ended, cap at 20 + // CR 107.1a/b: "choose a number greater than N" states a lower bound and + // NO upper one, so the range is unbounded above. let (n, tail) = match nom::character::complete::digit1::<_, ()>(gt_rest.trim_start()) { - Ok((rest_after, digits)) => (digits.parse::().unwrap_or(0), rest_after), + Ok((rest_after, digits)) => (digits.parse::().unwrap_or(0), rest_after), Err(_) => (0, ""), }; Some(ChoiceType::NumberRange { - min: n + 1, - max: 20, + min: n.saturating_add(1), + max: None, distinctness: parse_number_distinctness(tail), }) } else if tag::<_, _, E>("a number").parse(rest).is_ok() { - // Generic "choose a number" (default range 0-20). A bare-prefix `tag`, - // mirroring the "a color" branch above, so a sentence-ending clause such - // as "As ~ enters, choose a number." parses (Squall, Gunblade Duelist, - // #722). The previous exact/trailing-space match dropped the period and - // produced no choice. The bounded "a number between" / "a number greater - // than" forms are consumed by the earlier branches, so this arm is reached - // only for a bare "a number". + // CR 107.1a/b: bare "choose a number", and the explicit "a number 0 or + // greater" (Wheel of Misfortune, Menacing Ogre, Itazura). Neither states a + // maximum, so neither gets one — the rules permit any nonnegative integer, + // and on Wheel the size of the number is the entire decision, so an + // invented ceiling would make a legal choice illegal. + // + // A bare-prefix `tag`, mirroring the "a color" branch above, so a + // sentence-ending clause such as "As ~ enters, choose a number." parses + // (Squall, Gunblade Duelist, #722). The bounded "a number between" form is + // consumed by the earlier branch, so this arm is reached only for the + // unbounded shapes. let tail = tag::<_, _, ()>("a number") .parse(rest) .map(|(tail, _)| tail) .unwrap_or(""); Some(ChoiceType::NumberRange { min: 0, - max: 20, + max: None, distinctness: parse_number_distinctness(tail), }) } else if alt((tag::<_, _, E>("a land type"), tag("a nonbasic land type"))) @@ -25404,9 +25668,38 @@ pub(crate) fn parse_named_choice_object(rest: &str) -> Option { .is_ok() { Some(ChoiceType::LandType) - } else if tag::<_, _, E>("an opponent").parse(rest).is_ok() { - // CR 800.4a: Choose an opponent from among players in the game. - Some(ChoiceType::opponent()) + } else if let Ok((after_opponent, _)) = tag::<_, _, E>("an opponent").parse(rest) { + // CR 608.2c + CR 608.2d: "choose an opponent WITH THE HIGHEST NUMBER" + // (Itazura, Lingering Wick). The restriction is part of the instruction + // and cannot be discarded — dropping it lets the controller pick an + // opponent who did not choose the highest number and then damage that + // illegal choice. + // + // Reuses the same restriction grammar and `PlayerFilter` builder the + // "each player who chose the highest number" subject path uses, so the + // two phrasings cannot drift. Gated on provenance: without a preceding + // secret-number choice in this ability there is nothing for "the highest + // number" to refer to, and binding it by wording alone is the Custodi + // Peacekeeper failure. + let restriction = has_number_choice + .then(|| { + let (after, _) = tag::<_, _, E>(" ").parse(after_opponent).ok()?; + lower::parse_chosen_number_restriction(after, None) + .ok() + .map(|(_, (comparator, aggregate))| { + lower::chosen_number_player_filter( + crate::types::ability::PlayerRelation::Opponent, + comparator, + aggregate, + ) + }) + }) + .flatten(); + match restriction { + Some(restriction) => Some(ChoiceType::opponent_with_restriction(restriction)), + // CR 800.4a: Choose an opponent from among players in the game. + None => Some(ChoiceType::opponent()), + } } else if tag::<_, _, E>("a player").parse(rest).is_ok() { Some(ChoiceType::player()) } else if tag::<_, _, E>("two colors").parse(rest).is_ok() { @@ -25442,6 +25735,64 @@ pub(crate) fn parse_named_choice_object(rest: &str) -> Option { } } +/// CR 101.4 + CR 608.2c: The sentence that publishes secretly-chosen numbers — +/// "reveal the number you chose" (The Toymaker's Trap), "reveal the chosen +/// numbers" (Life at Stake), "reveal those numbers simultaneously and determine +/// the highest and lowest numbers revealed this way" (Wheel of Misfortune), +/// "those numbers are revealed" (Menacing Ogre's passive voice). The leading +/// subject ("you " / "all players ") is stripped by the caller, which maps it to +/// the `PlayerFilter` naming whose numbers are published. +/// +/// Composed by axis — voice × object phrase × optional manner adverb × optional +/// "determine" tail, each its own `alt`/`opt` — rather than enumerated as +/// permutations of whole sentences. The recognized forms lower to +/// `Effect::RevealChosenNumbers`, which performs the private→public conversion +/// `game::visibility` reads. Only the "determine the highest/lowest" tail is +/// inert: those extrema are computed on demand by +/// `QuantityRef::PlayerChosenNumber` rather than stored. +fn parse_reveal_chosen_numbers_clause(input: &str) -> OracleResult<'_, ()> { + // Passive voice carries the object first: "those numbers are revealed". + if let Ok((input, _)) = ( + alt(( + tag::<_, _, OracleError<'_>>("those numbers"), + tag("the chosen numbers"), + tag("the numbers"), + )), + tag(" are revealed"), + ) + .parse(input) + { + return Ok((input, ())); + } + // Active voice, both persons: an imperative "reveal …" and a third-person + // "each player revealS …". The `s` is its own `opt`, not a duplicated tag, + // so the person axis costs nothing to extend. + let (input, _) = (tag("reveal"), opt(tag("s")), tag(" ")).parse(input)?; + let (input, _) = alt(( + tag("the number you chose"), + tag("the number they chose"), + tag("the chosen numbers"), + tag("the chosen number"), + tag("those numbers"), + )) + .parse(input)?; + let (input, _) = opt(tag(" simultaneously")).parse(input)?; + let (input, _) = opt(preceded( + (tag(" and determine "), tag("the ")), + ( + crate::parser::oracle_nom::quantity::parse_chosen_number_extremum, + opt(preceded( + tag(" and "), + crate::parser::oracle_nom::quantity::parse_chosen_number_extremum, + )), + alt((tag(" numbers"), tag(" number"))), + opt(tag(" revealed this way")), + ), + )) + .parse(input)?; + Ok((input, ())) +} + /// CR 608.2d + CR 614.1c: A conjunction of named-choice phrases sharing one /// "choose" ("choose a creature card name and a creature type" — Psychic /// Paper). Strips the same "choose "/"secretly choose " prefix diff --git a/crates/engine/src/parser/oracle_effect/sequence.rs b/crates/engine/src/parser/oracle_effect/sequence.rs index 97880a37b0..7437fa9d2e 100644 --- a/crates/engine/src/parser/oracle_effect/sequence.rs +++ b/crates/engine/src/parser/oracle_effect/sequence.rs @@ -6363,6 +6363,7 @@ pub(super) fn clause_is_dig_lookback_transparent(effect: &Effect) -> bool { | Effect::Choose { .. } | Effect::OpponentGuess { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::ChooseDamageSource { .. } | Effect::Suspect { .. } | Effect::Unsuspect { .. } diff --git a/crates/engine/src/parser/oracle_effect/subject.rs b/crates/engine/src/parser/oracle_effect/subject.rs index 3d1f847764..4d67bba0d1 100644 --- a/crates/engine/src/parser/oracle_effect/subject.rs +++ b/crates/engine/src/parser/oracle_effect/subject.rs @@ -2250,6 +2250,25 @@ pub(super) fn enchanted_player_anaphor_filter( matches!(scope, Some(ControllerRef::EnchantedPlayer)).then_some(TargetFilter::DefendingPlayer) } +/// CR 608.2c + CR 109.4: single authority for "the player a `Choose(Player)` +/// clause earlier in this chain selected" as a `TargetFilter`. +/// +/// A resolution-time chosen player has no dedicated `TargetFilter` variant — it +/// is expressed as a player-only `Typed` filter whose `controller` carries the +/// `ChosenPlayer { index }` scope, which is what the runtime filter evaluates +/// against `ability.chosen_players`. Every anaphor that can name that player +/// ("they" as a subject, "them" as a damage recipient) must produce the SAME +/// filter, so the construction lives here rather than being rebuilt per site. +pub(super) fn chosen_player_anaphor_filter(scope: Option<&ControllerRef>) -> Option { + let scope @ ControllerRef::ChosenPlayer { .. } = scope? else { + return None; + }; + Some(TargetFilter::Typed(crate::types::ability::TypedFilter { + controller: Some(scope.clone()), + ..Default::default() + })) +} + /// Which player-subject anaphor a standalone "that/the player" clause names. /// /// Both forms resolve to an event-context `TargetFilter` via @@ -3571,11 +3590,8 @@ fn resolve_they_pronoun(ctx: &mut ParseContext) -> TargetFilter { // CR 608.2c + CR 109.4: "They" after a `Choose(Player)` clause refers to // the chosen player — a player-only `Typed` filter carrying the chosen // scope (Gluntch's "choose a player. They put two +1/+1 counters …"). - if let Some(scope @ ControllerRef::ChosenPlayer { .. }) = &ctx.relative_player_scope { - return TargetFilter::Typed(crate::types::ability::TypedFilter { - controller: Some(scope.clone()), - ..Default::default() - }); + if let Some(filter) = chosen_player_anaphor_filter(ctx.relative_player_scope.as_ref()) { + return filter; } match &ctx.subject { // Player-type trigger subject: no type_filters, has controller ref diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index bc6cfe85d9..99a5f6180f 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -25740,7 +25740,7 @@ fn committed_choice_guess_chooses_single_opponent_before_guess() { Effect::Choose { choice_type: ChoiceType::NumberRange { min: 1, - max: 5, + max: Some(5), distinctness: NumberDistinctness::DistinctFromSourceHistory }, .. @@ -25783,7 +25783,7 @@ fn committed_choice_guess_chooses_single_opponent_before_guess() { GuessSubject::CommittedChoice { choice_type: ChoiceType::NumberRange { min: 1, - max: 5, + max: Some(5), distinctness: NumberDistinctness::DistinctFromSourceHistory } } @@ -27699,6 +27699,662 @@ fn strip_each_scope_who_didnt_discard_filter_this_way_is_exact() { } } +/// CR 101.4 + CR 608.2d: the chosen-number subject restriction is a CLASS, not +/// Wheel of Misfortune's one sentence. Every cell of polarity × extremum × +/// player scope must narrow the subject to a `PlayerAttribute` whose +/// per-candidate scalar is the player's own secretly-chosen number and whose +/// threshold is the cross-player extremum of the same scalar — and the body must +/// survive, deconjugated. +#[test] +fn strip_each_player_subject_chosen_number_matrix() { + use crate::types::ability::{AggregateFunction, PlayerRelation, PlayerScope}; + + fn expect(text: &str) -> (PlayerRelation, Comparator, AggregateFunction, String) { + let (scope, body) = strip_each_player_subject(text); + let Some(PlayerFilter::PlayerAttribute { + relation, + attr, + comparator, + value, + }) = scope + else { + panic!("a chosen-number subject must narrow the player scope: {text}"); + }; + // The per-candidate read is THIS player's own number (CR 608.2d), never + // an aggregate — an aggregate here would compare the table extremum to + // itself and match everyone. + assert_eq!( + *attr, + QuantityRef::PlayerChosenNumber { + player: PlayerScope::ScopedPlayer + }, + "per-candidate scalar for {text}" + ); + let QuantityExpr::Ref { + qty: + QuantityRef::PlayerChosenNumber { + player: PlayerScope::AllPlayers { aggregate, exclude }, + }, + } = *value + else { + panic!("threshold must be a cross-player chosen-number extremum for {text}"); + }; + // CR 101.4: the extremum is over EVERY player who chose, including the + // controller — "the highest number" is the table's highest, not the + // highest among some narrowed subset. This matters most on the opponent + // relation ("each opponent with the highest number"), where `relation` + // narrows WHO IS AFFECTED but must not narrow WHAT IS COMPARED: an + // `exclude` here would silently measure the extremum over opponents only + // and hit an opponent whose number the controller had beaten. + assert!( + exclude.is_none(), + "the extremum population must not be narrowed for {text}, got exclude={exclude:?}" + ); + (relation, comparator, aggregate, body) + } + + // Wheel of Misfortune's wheel clause: negated polarity × lowest × all players. + let (relation, comparator, aggregate, body) = + expect("Each player who didn't choose the lowest number discards their hand"); + assert_eq!(relation, PlayerRelation::All); + assert_eq!(comparator, Comparator::NE); + assert_eq!(aggregate, AggregateFunction::Min); + assert_eq!(body, "discard their hand"); + + // Life at Stake's life-loss clause: positive polarity × highest. + let (relation, comparator, aggregate, body) = + expect("Each player who chose the highest number loses that much life"); + assert_eq!(relation, PlayerRelation::All); + assert_eq!(comparator, Comparator::EQ); + assert_eq!(aggregate, AggregateFunction::Max); + assert_eq!(body, "lose that much life"); + + // Menacing Ogre's participial phrasing, on the opponent relation — the same + // restriction reached through a different surface form. + let (relation, comparator, aggregate, body) = + expect("Each opponent with the highest number loses that much life"); + assert_eq!(relation, PlayerRelation::Opponent); + assert_eq!(comparator, Comparator::EQ); + assert_eq!(aggregate, AggregateFunction::Max); + assert_eq!(body, "lose that much life"); +} + +/// The restriction must NOT fire on shapes it does not model: an unrelated +/// relative clause, or the anaphoric "that number" with no extremum in scope to +/// bind it (passing `anaphor: None`, as the subject path does). Binding "that +/// number" to a guessed extremum would silently invent a referent. +#[test] +fn chosen_number_restriction_rejects_unbound_and_unrelated_clauses() { + assert!( + super::lower::parse_chosen_number_restriction("who chose that number", None).is_err(), + "an anaphor with no referent in scope must decline, not guess an extremum" + ); + // The anaphor must bind to the SUPPLIED referent, not merely succeed. A + // parser that ignored the parameter and hardcoded one extremum would pass an + // `is_ok()` check while resolving "that number" to the wrong value, so assert + // the returned pair — and assert BOTH extrema so the binding is shown to + // track the argument rather than coincide with a default. + for aggregate in [ + crate::types::ability::AggregateFunction::Max, + crate::types::ability::AggregateFunction::Min, + ] { + let (rest, bound) = + super::lower::parse_chosen_number_restriction("who chose that number", Some(aggregate)) + .expect("the anaphor binds once its referent is supplied"); + assert_eq!(rest, "", "the whole clause is consumed"); + assert_eq!( + bound, + (Comparator::EQ, aggregate), + "\"that number\" must resolve to the extremum the caller supplied" + ); + } + assert!( + super::lower::parse_chosen_number_restriction("who didn't discard a card", None).is_err(), + "an unrelated decline tail is not a chosen-number restriction" + ); + assert!( + super::lower::parse_chosen_number_restriction("who controls an artifact", None).is_err(), + "a controls-clause is not a chosen-number restriction" + ); +} + +/// CR 101.4 + CR 120.3: Wheel of Misfortune's whole sentence, end to end at the +/// AST layer. Every clause must lower to a typed effect — the pre-fix parse was +/// four consecutive `Unimplemented` links — and the two extrema must be +/// DIFFERENT (`Max` for the damage, `Min` for the wheel), which a single shared +/// "the chosen number" reading would collapse. +#[test] +fn wheel_of_misfortune_lowers_every_clause() { + use crate::types::ability::{AggregateFunction, PlayerScope}; + + fn collect<'a>(a: &'a AbilityDefinition, out: &mut Vec<&'a AbilityDefinition>) { + out.push(a); + if let Some(s) = a.sub_ability.as_deref() { + collect(s, out); + } + if let Some(e) = a.else_ability.as_deref() { + collect(e, out); + } + } + + let text = "Each player secretly chooses a number 0 or greater, then all players reveal \ + those numbers simultaneously and determine the highest and lowest numbers revealed \ + this way. Wheel of Misfortune deals damage equal to the highest number to each player \ + who chose that number. Each player who didn't choose the lowest number discards their \ + hand, then draws seven cards."; + let parsed = parse_oracle_text( + text, + "Wheel of Misfortune", + &[], + &["Sorcery".to_string()], + &[], + ); + let ability = parsed.abilities.first().expect("expected a spell ability"); + + let mut links = Vec::new(); + collect(ability, &mut links); + assert!( + !links + .iter() + .any(|link| matches!(&*link.effect, Effect::Unimplemented { .. })), + "no clause may fall back to Unimplemented: {:#?}", + links.iter().map(|l| &l.effect).collect::>() + ); + + // CR 608.2d: "each player secretly chooses a number 0 or greater" — the + // per-player choice, which must PERSIST or the later extremum reads have + // nothing to fold. + let Effect::Choose { + choice_type: ChoiceType::NumberRange { min, .. }, + persist, + .. + } = &*ability.effect + else { + panic!("head must be the number choice, got {:#?}", ability.effect); + }; + assert_eq!(*min, 0, "\"a number 0 or greater\" starts at zero"); + assert!( + *persist, + "a chosen number a later clause reads must persist" + ); + assert_eq!(ability.player_scope, Some(PlayerFilter::All)); + + // CR 120.3: the damage amount and its recipient set are BOTH keyed on the + // highest number. + let damage = links + .iter() + .find_map(|link| match &*link.effect { + Effect::DamageEachPlayer { + amount, + player_filter, + } => Some((amount.clone(), player_filter.clone())), + _ => None, + }) + .expect("the damage clause must lower to DamageEachPlayer"); + assert_eq!( + damage.0, + QuantityExpr::Ref { + qty: QuantityRef::PlayerChosenNumber { + player: PlayerScope::AllPlayers { + aggregate: AggregateFunction::Max, + exclude: None, + }, + }, + }, + "\"damage equal to the highest number\"" + ); + let PlayerFilter::PlayerAttribute { + comparator, + value: recipient_threshold, + .. + } = damage.1 + else { + panic!("\"each player who chose that number\" must narrow the recipients"); + }; + assert_eq!(comparator, Comparator::EQ); + assert_eq!( + *recipient_threshold, damage.0, + "\"that number\" must anaphor the SAME extremum the amount named" + ); + + // CR 701.9a + CR 121.1: the wheel half reads the LOWEST number under NE. + let wheel_scope = links + .iter() + .find_map(|link| match (&*link.effect, &link.player_scope) { + (Effect::Discard { .. }, Some(scope)) => Some(scope.clone()), + _ => None, + }) + .expect("the discard clause must carry the narrowed scope"); + assert_eq!( + wheel_scope, + PlayerFilter::PlayerAttribute { + relation: crate::types::ability::PlayerRelation::All, + attr: Box::new(QuantityRef::PlayerChosenNumber { + player: PlayerScope::ScopedPlayer, + }), + comparator: Comparator::NE, + value: Box::new(QuantityExpr::Ref { + qty: QuantityRef::PlayerChosenNumber { + player: PlayerScope::AllPlayers { + aggregate: AggregateFunction::Min, + exclude: None, + }, + }, + }), + }, + "\"each player who didn't choose the lowest number\"" + ); + // CR 608.2c: the ", then draws seven cards" continuation inherits the SAME + // narrowed subject — a plain `All` here would wheel the lowest chooser too. + let draw_scope = links + .iter() + .find_map(|link| match (&*link.effect, &link.player_scope) { + (Effect::Draw { .. }, Some(scope)) => Some(scope.clone()), + _ => None, + }) + .expect("the draw continuation must carry a scope"); + assert_eq!(draw_scope, wheel_scope); +} + +/// CR 101.4 + CR 608.2c: the reveal grammar covers both voices and both persons, +/// and — critically — consumes its WHOLE clause. A prefix-accepting reveal would +/// lower a chunk to a bare publication and silently drop everything after it, +/// which is the swallow class this parser exists to prevent. +#[test] +fn reveal_chosen_numbers_grammar_is_complete_clause_and_covers_both_voices() { + fn lowers_to_reveal(text: &str) -> bool { + matches!( + &*parse_effect_chain(text, AbilityKind::Spell).effect, + Effect::RevealChosenNumbers { .. } + ) + } + + // Voice × person × object phrase, each an independent axis. + for accepted in [ + "Then you reveal the number you chose.", + "Each player reveals the number they chose.", + "All players reveal those numbers simultaneously.", + "Then, reveal the chosen numbers.", + "Then those numbers are revealed.", + "All players reveal those numbers simultaneously and determine the highest and lowest numbers revealed this way.", + ] { + assert!( + lowers_to_reveal(accepted), + "must lower to RevealChosenNumbers: {accepted}" + ); + } + + // ANTI-SWALLOW. A reveal followed by another instruction must keep that + // instruction. The clause splitter separates these before the reveal grammar + // sees them, so the head legitimately IS a reveal — what matters is that the + // TAIL survives as a chained link rather than being discarded. Asserting the + // head "is not a reveal" would be testing the splitter's boundary choice + // instead of the property that matters. + for (text, tail_present) in [ + ( + "Reveal the chosen numbers, then each player loses 3 life.", + "LoseLife", + ), + ( + "Reveal those numbers and sacrifice a creature.", + "Sacrifice", + ), + ] { + let def = parse_effect_chain(text, AbilityKind::Spell); + assert!( + matches!(&*def.effect, Effect::RevealChosenNumbers { .. }), + "the head clause is still the reveal: {text}" + ); + let tail = def + .sub_ability + .as_ref() + .unwrap_or_else(|| panic!("the trailing instruction was swallowed: {text}")); + assert!( + format!("{:?}", tail.effect).contains(tail_present), + "expected a surviving {tail_present} tail for {text}, got {:#?}", + tail.effect + ); + } +} + +/// CR 608.2c: a chosen-number reference in a link's CONDITION must force the +/// upstream `NumberRange` choice to persist. Without the condition walk the +/// answer is cleared before the condition is evaluated, so the gate reads +/// against nothing — a silent wrong-branch rather than a visible failure. +/// +/// Fail-on-revert: drop the `.condition` arm from +/// `definition_reads_player_chosen_number` and the first assertion flips. +#[test] +fn chosen_number_read_from_a_condition_forces_persistence() { + use crate::types::ability::{ + AbilityCondition, AggregateFunction, ChoiceType, Comparator, NumberDistinctness, + PlayerScope, TargetSelectionMode, + }; + + // CR 608.2c: a chain whose ONLY chosen-number reference lives in a link's + // condition. `promote_chosen_number_persistence` must still persist the + // upstream choice, or the answer is cleared before the condition is + // evaluated and the gate reads against nothing. + // + // Built directly rather than via Oracle text: no shipped card reaches this + // shape today, and the point is the walker's coverage, not a parse. The + // reference is buried under Not(And(...)) so a shallow check fails. + fn number_choice() -> AbilityDefinition { + AbilityDefinition::new( + AbilityKind::Spell, + Effect::Choose { + choice_type: ChoiceType::NumberRange { + min: 0, + max: Some(20), + distinctness: NumberDistinctness::Repeatable, + }, + persist: false, + selection: TargetSelectionMode::Chosen, + }, + ) + } + + let extremum = QuantityExpr::Ref { + qty: QuantityRef::PlayerChosenNumber { + player: PlayerScope::AllPlayers { + aggregate: AggregateFunction::Max, + exclude: None, + }, + }, + }; + + let mut gated = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ); + gated.condition = Some(AbilityCondition::Not { + condition: Box::new(AbilityCondition::And { + conditions: vec![AbilityCondition::QuantityCheck { + lhs: extremum, + comparator: Comparator::GE, + rhs: QuantityExpr::Fixed { value: 1 }, + }], + }), + }); + + let mut chain = number_choice(); + chain.sub_ability = Some(Box::new(gated)); + + // Control: the identical chain with no condition must NOT persist, which is + // what proves the assertion below is measuring the condition walk and not + // some unconditional promotion. + let mut unconditional = number_choice(); + unconditional.sub_ability = Some(Box::new(AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ))); + super::promote_chosen_number_persistence(&mut unconditional); + assert!( + matches!( + &*unconditional.effect, + Effect::Choose { persist: false, .. } + ), + "a chain that never reads the number must not persist it" + ); + + super::promote_chosen_number_persistence(&mut chain); + assert!( + matches!(&*chain.effect, Effect::Choose { persist: true, .. }), + "a chosen-number read nested in Not(And(QuantityCheck)) must force the \ + upstream choice to persist, got {:#?}", + chain.effect + ); +} + +/// CR 101.4 + CR 608.2d: sweep of every card the CI parse-diff flagged for this +/// change, asserting the invariant that makes the chosen-number reference safe: +/// a card may only READ a secretly-chosen number if it also CREATES one. +/// +/// This is the class-level answer to "re-check every changed card": the flagged +/// set is all "each player secretly …" cards that previously died at +/// `Unimplemented { secretly }`, plus two controls that contain the same words +/// with no choice behind them. Wording-matched parsing passes the first six and +/// fails the controls; provenance-bound parsing passes all eight. +#[test] +fn secret_number_provenance_invariant_holds_across_the_class() { + // Every card the CI parse-diff flagged for this change, by verbatim Oracle + // text (Scryfall). They are all members of the "each player secretly …" + // class: before the adverb peel each died at `Unimplemented { secretly }`, + // so every one of them is an unlock rather than a reinterpretation. The + // last two are the CONTROLS: they contain the words this grammar keys on + // but no secret choice at all. + const CARDS: &[(&str, &str)] = &[ + ("Círdan the Shipwright", "Vigilance\nSecret council — Whenever Círdan enters or attacks, each player secretly votes for a player, then those votes are revealed. Each player draws a card for each vote they received. Each player who received no votes may put a permanent card from their hand onto the battlefield."), + ("Mob Verdict", "Secret council — Each player secretly votes for another player, then those votes are revealed. For each vote an opponent received, Mob Verdict deals 2 damage to that player and each creature that player controls. For each vote you received, draw a card."), + ("Mana Conference", "When Mana Conference enters the battlefield, each player secretly chooses a basic land type, then those choices are revealed. Mana Conference gains each basic land type that received at least one vote."), + ("Prisoner's Dilemma", "Each opponent secretly chooses silence or snitch, then the choices are revealed. If each opponent chose silence, Prisoner's Dilemma deals 4 damage to each of them. If each opponent chose snitch, Prisoner's Dilemma deals 8 damage to each of them. Otherwise, Prisoner's Dilemma deals 12 damage to each opponent who chose silence."), + ("Menacing Ogre", "Trample, haste\nWhen this creature enters, each player secretly chooses a number. Then those numbers are revealed. Each player with the highest number loses that much life. If you are one of those players, put two +1/+1 counters on this creature."), + ("Itazura, Lingering Wick", "At the beginning of your upkeep, exile the top three cards of your library. Each opponent secretly chooses a number 0 or greater. Then those numbers are revealed. Choose an opponent with the highest number. Itazura deals that much damage to them, then they may cast a spell from among those cards without paying its mana cost. You put a card from among them that wasn't cast this way into your hand."), + // CONTROL: a draft-time NOTED number, no choice anywhere on the card. + ("Custodi Peacekeeper", "Reveal this card as you draft it and note how many cards you've drafted this draft round, including this card.\n{W}, {T}: Tap target creature with power less than or equal to the highest number you noted for cards named Custodi Peacekeeper."), + // CONTROL: "highest"/"number" in a pure counting phrase. + ("Counting Control", "Draw cards equal to the highest number of cards in hand among players."), + ]; + + let mut readers: Vec<&str> = Vec::new(); + for (name, oracle) in CARDS { + let parsed = parse_oracle_text(oracle, name, &[], &["Creature".to_string()], &[]); + let rendered = format!("{parsed:?}"); + let reads_chosen_number = rendered.contains("PlayerChosenNumber"); + let has_number_choice = rendered.contains("NumberRange"); + if reads_chosen_number { + readers.push(name); + } + + // THE INVARIANT: a chosen-number reference may exist only where the same + // card actually creates a secret number to refer to. This is what makes + // the reference provenance-bound instead of wording-matched, and it is + // the property whose absence rewrote Custodi Peacekeeper. + assert!( + !reads_chosen_number || has_number_choice, + "{name} reads a secretly-chosen number with no NumberRange choice to bind it to" + ); + } + + // REACH GUARD. The invariant above is an implication, so it holds vacuously + // for a card that produces no `PlayerChosenNumber` at all — if the grammar + // stopped firing entirely, or every card fell back to `Unimplemented`, all + // eight cases would still "pass" while proving nothing. + // + // Menacing Ogre is the NAMED positive: "Each player with the highest number + // loses that much life" is the one clause in this set that both creates a + // secret number and reads the extremum back, so it must appear here or the + // sweep is measuring nothing. + // + // Itazura joined the readers when "Choose an opponent with the highest + // number" was bound to the restriction seam. It was previously listed here as + // a known gap, and this assertion is what forced that to be closed + // deliberately rather than drifting: the fix made the sweep RED until the + // expected set was updated with the card confirmed correct. + assert_eq!( + readers, + vec!["Menacing Ogre", "Itazura, Lingering Wick"], + "the sweep's positive side changed. If a card gained a chosen-number read, \ + confirm it is correct and add it here; if one stopped reading, the grammar \ + regressed and the implication above is now vacuous" + ); +} + +/// CR 608.2c + CR 101.4: *"Choose an opponent with the highest number. ~ deals +/// that much damage to them."* — the anaphor pair, and the guards that keep it +/// from firing where the antecedent isn't provable. +/// +/// "That much" is `EventContextAmount` — "whatever amount the surrounding event +/// supplies" — and a resolving spell supplies none, so an unbound anaphor here +/// deals 0 rather than failing loudly. `bind_chosen_number_anaphor` rewrites it +/// only when the recipient anaphor names a `Choose(Player)` clause that selected +/// BY the chosen number. +/// +/// The two declines are the point of the test: a chosen player selected without +/// a number restriction, and one selected by NOT holding the extremum, both have +/// a "highest number" in scope but neither makes it that player's number. +/// Fail-on-revert: drop either guard and a decline case starts binding. +#[test] +fn that_much_damage_to_them_binds_only_to_a_provable_chosen_number() { + const SELECT: &str = "Each opponent secretly chooses a number 0 or greater. \ + Then those numbers are revealed. "; + + // BINDS: the selection was made BY the number, so the chosen player holds + // the extremum and "that much" is that extremum. + let bound = format!( + "{SELECT}Choose an opponent with the highest number. \ + Wick deals that much damage to them." + ); + let rendered = format!( + "{:?}", + parse_oracle_text(&bound, "Wick", &[], &["Instant".to_string()], &[]) + ); + assert!( + rendered.contains("PlayerChosenNumber"), + "the damage amount must bind to the chosen number, not stay an \ + unbound event-context amount: {rendered}" + ); + assert!( + rendered.contains("ChosenPlayer"), + "the recipient \"them\" must name the chosen player: {rendered}" + ); + + // DECLINES. Both still parse a chosen player and still say "that much"; what + // they lack is a restriction proving that player's number IS the extremum. + for (label, oracle) in [ + ( + "no restriction — any opponent may be chosen, so no extremum is theirs", + format!("{SELECT}Choose an opponent. Wick deals that much damage to them."), + ), + ( + "negated restriction — the chosen player provably does NOT hold it", + format!( + "{SELECT}Choose an opponent who didn't choose the highest number. \ + Wick deals that much damage to them." + ), + ), + ] { + let rendered = format!( + "{:?}", + parse_oracle_text(&oracle, "Wick", &[], &["Instant".to_string()], &[]) + ); + // Non-vacuity: the decline must be a decline to BIND, not a failure to + // parse the damage clause at all — otherwise the assertion below holds + // for the wrong reason. + assert!( + rendered.contains("DealDamage"), + "{label}: the damage clause must still parse, or the decline below \ + is vacuous: {rendered}" + ); + // Scope the assertion to the damage amount: the SELECTION clause of the + // negated case legitimately carries a `PlayerChosenNumber` in its + // restriction, so a whole-tree "contains" check would be vacuous there. + assert!( + !damage_amount_reads_chosen_number(&rendered), + "{label}: the damage amount must stay unbound: {rendered}" + ); + } +} + +/// Whether a rendered ability's `DealDamage` amount reads a chosen number, +/// ignoring chosen-number references anywhere else in the tree. +fn damage_amount_reads_chosen_number(rendered: &str) -> bool { + rendered.split("DealDamage").skip(1).any(|tail| { + let amount = tail.split("target").next().unwrap_or(tail); + amount.contains("PlayerChosenNumber") + }) +} + +/// CR 608.2d: "the highest number" is only a secretly-chosen number when a +/// preceding choice in the SAME ability created one. Custodi Peacekeeper's +/// "power less than or equal to the highest number you noted for cards named +/// Custodi Peacekeeper" is a draft-time noted value with no choice behind it, so +/// the chosen-number grammar must not touch it. +/// +/// Fail-on-revert: registering the extremum in the context-free +/// `parse_quantity_ref` alt (its original shape) rewrote this card's `Tap` target +/// to `power ≤ secretly chosen number (max of all players)` — a silent +/// reinterpretation of an unrelated card, caught by the CI parse-diff. +#[test] +fn noted_number_is_not_a_secretly_chosen_number() { + const CUSTODI_PEACEKEEPER: &str = "Reveal this card as you draft it and note how many cards you've drafted this draft round, including this card.\n{W}, {T}: Tap target creature with power less than or equal to the highest number you noted for cards named Custodi Peacekeeper."; + let parsed = parse_oracle_text( + CUSTODI_PEACEKEEPER, + "Custodi Peacekeeper", + &[], + &["Creature".to_string()], + &[], + ); + + fn mentions_chosen_number(def: &AbilityDefinition) -> bool { + let mut found = false; + def.effect.for_each_quantity_expr(&mut |expr| { + if let QuantityExpr::Ref { + qty: QuantityRef::PlayerChosenNumber { .. }, + } = expr + { + found = true; + } + }); + let filter_mentions = format!("{:?}", def.effect).contains("PlayerChosenNumber") + || format!("{:?}", def.player_scope).contains("PlayerChosenNumber"); + found + || filter_mentions + || def + .sub_ability + .as_deref() + .is_some_and(mentions_chosen_number) + || def + .else_ability + .as_deref() + .is_some_and(mentions_chosen_number) + } + + // REACH GUARD. A non-empty `abilities` is not enough: `mentions_chosen_number` + // also returns false when the ability lowered to `Effect::Unimplemented`, so a + // regression that DROPPED the "power less than or equal to …" clause entirely + // would satisfy the negative assertion below for the wrong reason. Require the + // real activated ability — a `Tap` whose target survived — so the negative is + // measured against a parse that actually reached the clause under test. + assert!( + !parsed.abilities.is_empty(), + "Custodi Peacekeeper must still produce its activated ability" + ); + fn has_live_tap(def: &AbilityDefinition) -> bool { + matches!( + &*def.effect, + Effect::SetTapState { + state: crate::types::ability::TapStateChange::Tap, + .. + } + ) || def.sub_ability.as_deref().is_some_and(has_live_tap) + || def.else_ability.as_deref().is_some_and(has_live_tap) + } + assert!( + parsed.abilities.iter().any(has_live_tap), + "the tap ability must still lower — without it the negative below passes \ + because the clause vanished, not because it parsed correctly: {:#?}", + parsed + .abilities + .iter() + .map(|a| &a.effect) + .collect::>() + ); + for ability in &parsed.abilities { + assert!( + !mentions_chosen_number(ability), + "a noted number must not be reinterpreted as a secretly-chosen one: {:#?}", + ability.effect + ); + } +} + /// CR 118.12 + CR 608.2d + CR 109.5: `strip_each_scope_who_does_subject` /// covers the full subject-only × scope × positive-"does" matrix (The Second /// Doctor: "each opponent who does can't attack you …"; Step Between Worlds: diff --git a/crates/engine/src/parser/oracle_ir/doc.rs b/crates/engine/src/parser/oracle_ir/doc.rs index 138ee242bb..b8748a354d 100644 --- a/crates/engine/src/parser/oracle_ir/doc.rs +++ b/crates/engine/src/parser/oracle_ir/doc.rs @@ -1528,6 +1528,7 @@ fn stamp_effect_printed_slot(effect: &mut Effect, slot: usize, kind: PrintedItem Effect::Choose { .. } => {} Effect::OpponentGuess { .. } => {} Effect::SwapChosenLabels { .. } => {} + Effect::RevealChosenNumbers { .. } => {} Effect::ChooseDamageSource { .. } => {} Effect::Suspect { .. } => {} Effect::Unsuspect { .. } => {} diff --git a/crates/engine/src/parser/oracle_nom/quantity.rs b/crates/engine/src/parser/oracle_nom/quantity.rs index 8d8e5887ae..af1a55c764 100644 --- a/crates/engine/src/parser/oracle_nom/quantity.rs +++ b/crates/engine/src/parser/oracle_nom/quantity.rs @@ -841,6 +841,78 @@ fn parse_paid_energy_this_way_ref(input: &str) -> OracleResult<'_, QuantityRef> .parse(input) } +/// CR 101.4 + CR 608.2d: which cross-player extremum a "chosen number" phrase +/// names. The two words are the only leaves of this axis; the aggregation is the +/// existing [`AggregateFunction`], so no extremum enum is minted. Shared with the +/// subject-side restriction grammar (`oracle_effect::lower`) so the two sites +/// cannot drift. +pub fn parse_chosen_number_extremum(input: &str) -> OracleResult<'_, AggregateFunction> { + alt(( + value(AggregateFunction::Max, tag("highest")), + value(AggregateFunction::Min, tag("lowest")), + )) + .parse(input) +} + +/// CR 101.4: the singular head noun of a chosen-number phrase, with a +/// word-boundary guard so `" number"` cannot match the prefix of `" numbers"`. +/// The plural ("the highest and lowest numbers revealed this way") is the +/// bookkeeping sentence's noun, not a value reference, and belongs to the +/// reveal-clause combinator instead. +pub fn parse_chosen_number_noun(input: &str) -> OracleResult<'_, ()> { + value( + (), + terminated( + tag(" number"), + nom::combinator::not(nom::character::complete::satisfy(|c: char| { + c.is_ascii_alphanumeric() + })), + ), + ) + .parse(input) +} + +/// CR 101.4 + CR 608.2d: "the highest number" / "the lowest number" — the +/// cross-player extremum of the numbers players secretly chose during this +/// resolution (Wheel of Misfortune, Menacing Ogre, Life at Stake). +/// `QuantityRef::PlayerChosenNumber` under `PlayerScope::AllPlayers { aggregate }` +/// folds `Player::chosen_attributes` over the players who actually chose. +/// +/// DELIBERATELY NOT REGISTERED in the context-free `parse_quantity_ref` alt. +/// The wording alone does not identify the concept: Custodi Peacekeeper's "power +/// less than or equal to the highest number YOU NOTED for cards named Custodi +/// Peacekeeper" is a draft-time noted value with no choice behind it, and a +/// wording-only match silently reinterpreted it as a secretly-chosen number. +/// The only caller is the context-gated arm in +/// `oracle_quantity::parse_cda_quantity_with_context`, which fires solely when +/// `ParseContext::pending_choice_type` proves a preceding `NumberRange` choice in +/// the same ability — the same provenance gate `try_parse_guess_clause` uses for +/// "guesses which number you chose". +/// +/// Two further guards keep it off phrases that only look alike: +/// `parse_chosen_number_noun`'s word boundary rejects the PLURAL bookkeeping noun +/// ("the highest and lowest numberS revealed this way"), and the trailing +/// `not(tag(" of "))` rejects the counting phrase "the highest number OF +/// <things>" ("… of cards in hand among players"). +pub(crate) fn parse_extreme_chosen_number_ref(input: &str) -> OracleResult<'_, QuantityRef> { + map( + terminated( + terminated( + preceded(tag("the "), parse_chosen_number_extremum), + parse_chosen_number_noun, + ), + nom::combinator::not(tag(" of ")), + ), + |aggregate| QuantityRef::PlayerChosenNumber { + player: crate::types::ability::PlayerScope::AllPlayers { + aggregate, + exclude: None, + }, + }, + ) + .parse(input) +} + pub fn parse_quantity_ref(input: &str) -> OracleResult<'_, QuantityRef> { alt(( alt(( @@ -9073,6 +9145,75 @@ mod tests { assert_eq!(controller, Some(ControllerRef::You)); } + /// CR 101.4 + CR 608.2d: "the highest number" / "the lowest number" reads as + /// the cross-player extremum of the secretly-chosen numbers — and NOT any of + /// the look-alike phrases that share its opening words. + #[test] + fn parse_extreme_chosen_number_ref_shape() { + for (text, aggregate) in [ + ("the highest number", AggregateFunction::Max), + ("the lowest number", AggregateFunction::Min), + ] { + let (rest, q) = parse_extreme_chosen_number_ref(text).unwrap(); + assert_eq!(rest, ""); + assert_eq!( + q, + QuantityRef::PlayerChosenNumber { + player: PlayerScope::AllPlayers { + aggregate, + exclude: None, + }, + }, + "{text}" + ); + } + + // The clause continues past the noun — still the same reference, with + // the remainder handed back (Wheel of Misfortune's "… to each player"). + let (rest, q) = + parse_extreme_chosen_number_ref("the highest number to each player").unwrap(); + assert_eq!(rest, " to each player"); + assert!(matches!(q, QuantityRef::PlayerChosenNumber { .. })); + + // A COUNTING phrase ("the highest number OF cards …") belongs to the + // object-count grammar; a PLURAL bookkeeping noun ("the highest and + // lowest numberS revealed this way") is not a value reference at all. + for unrelated in [ + "the highest number of cards in hand among players", + "the highest numbers revealed this way", + "the lowest numbers revealed this way", + ] { + assert!( + parse_extreme_chosen_number_ref(unrelated).is_err(), + "{unrelated} must not read as a chosen-number extremum" + ); + } + } + + /// The extremum reference is NOT reachable from the context-free + /// `parse_quantity_ref` grammar. Wording alone does not identify the concept — + /// Custodi Peacekeeper's "the highest number you noted for cards named …" is a + /// draft-time noted value with no choice behind it — so the only route in is + /// the provenance-gated arm in `parse_cda_quantity_with_context`, which + /// requires a preceding `NumberRange` choice in the same ability. + /// + /// Fail-on-revert: re-registering the combinator in the context-free alt makes + /// every one of these read as a secretly-chosen number. + #[test] + fn context_free_quantity_grammar_never_yields_a_chosen_number_extremum() { + for text in [ + "the highest number", + "the lowest number", + "the highest number you noted for cards named Custodi Peacekeeper", + ] { + let parsed = parse_quantity_ref(text).ok().map(|(_, q)| q); + assert!( + !matches!(parsed, Some(QuantityRef::PlayerChosenNumber { .. })), + "{text} must not resolve to a chosen number without proven provenance, got {parsed:?}" + ); + } + } + #[test] fn parse_quantity_ref_tokens_created_this_turn() { let (rest, q) = parse_quantity_ref("the number of tokens you created this turn").unwrap(); diff --git a/crates/engine/src/parser/oracle_quantity.rs b/crates/engine/src/parser/oracle_quantity.rs index ee645e22e6..d31ebe7396 100644 --- a/crates/engine/src/parser/oracle_quantity.rs +++ b/crates/engine/src/parser/oracle_quantity.rs @@ -785,6 +785,32 @@ pub(crate) fn parse_cda_quantity_with_context( ) -> Option { let text = text.trim().trim_end_matches('.'); + // CR 101.4 + CR 608.2d: "the highest number" / "the lowest number" — the + // cross-player extremum of the numbers players secretly chose earlier in THIS + // ability (Wheel of Misfortune, Menacing Ogre, Life at Stake). + // + // Gated on PROVENANCE, never on wording. The phrase is ambiguous in isolation: + // Custodi Peacekeeper's "power less than or equal to the highest number you + // noted for cards named Custodi Peacekeeper" is a draft-time noted value, and + // reading it as a secretly-chosen number silently reinterpreted a card that + // has no choice in it at all. `pending_choice_type` is the chunk-loop-threaded + // record of the last `Effect::Choose` domain in this ability (set in + // `imperative.rs`, carried across chunks by `chain_pending_choice_type`), so + // requiring it to be a `NumberRange` binds the reference to an actual + // preceding secret-number ledger. Same provenance gate `try_parse_guess_clause` + // applies to "guesses which number you chose". Without a proven choice the arm + // declines and the phrase falls through to the pre-existing grammar unchanged. + if matches!( + ctx.pending_choice_type, + Some(crate::types::ability::ChoiceType::NumberRange { .. }) + ) { + if let Ok((rest, qty)) = nom_quantity::parse_extreme_chosen_number_ref(text) { + if rest.is_empty() { + return Some(QuantityExpr::Ref { qty }); + } + } + } + // CR 107.1a: "half/third/tenth , rounded up/down" fractional // quantities delivered via a "where X is …" binding or a CDA route through // here (Chainer's Torment, Endless Ranks of the Dead, Ghoulcaller's Harvest, diff --git a/crates/engine/src/parser/oracle_replacement.rs b/crates/engine/src/parser/oracle_replacement.rs index 6a733bdc2d..b6d0d0603a 100644 --- a/crates/engine/src/parser/oracle_replacement.rs +++ b/crates/engine/src/parser/oracle_replacement.rs @@ -14740,16 +14740,18 @@ mod tests { matches!( *execute.effect, Effect::Choose { + // CR 107.1a/b: Talion states no maximum, so the range is + // unbounded rather than the old 0-20 stand-in. choice_type: ChoiceType::NumberRange { min: 0, - max: 20, + max: None, .. }, persist: true, .. } ), - "expected a persisted NumberRange(0,20) choice, got {:?}", + "expected a persisted unbounded NumberRange choice, got {:?}", execute.effect ); } diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index d9ade804f7..fea0a41eaf 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -548,9 +548,31 @@ pub enum ChoiceType { }, CardName, /// "Choose a number between X and Y" — generates string options "0", "1", ..., "Y". + /// CR 107.1a/b + CR 608.2d: choose a number from `min` up to `max`. + /// + /// `max: None` is the UNBOUNDED form — "choose a number 0 or greater" (Wheel + /// of Misfortune, Menacing Ogre, Itazura). The rules state no maximum, so the + /// engine must not invent one: a bounded stand-in silently makes a legal + /// choice illegal, and on Wheel the magnitude of the number IS the decision. + /// + /// The practical ceiling on an unbounded choice is `i32::MAX`, enforced at the + /// answer seam rather than here. That is not an arbitrary UI cap but the + /// engine's own arithmetic domain: every quantity resolves through `i32` + /// (`game::quantity`), and damage and life totals are `i32`, so a number the + /// engine could not represent could not be acted on either. Within that + /// domain, every value the rules permit is accepted. + /// + /// Unbounded ranges enumerate no options — `compute_options` returns empty and + /// `options_supplied_by_player` is true, the same free-entry path `CardName` + /// already uses — so the client renders a numeric input instead of a button + /// per value. NumberRange { - min: u8, - max: u8, + min: u32, + /// `None` = no maximum (CR 107.1a/b). Bounded card text ("a number + /// between 1 and 5") keeps `Some`. The serde attributes that keep the + /// bounded form byte-identical on the wire live on the `ChoiceTypeData` + /// mirror, because `ChoiceType` itself has a hand-written `Serialize`. + max: Option, /// CR 609.3: distinctness requirement, parse-detected from "that hasn't /// been chosen". Default `Repeatable` for every existing card. distinctness: NumberDistinctness, @@ -781,10 +803,80 @@ impl ChoiceType { /// predicate is true) from an impossible choice that must resolve as a /// no-op per CR 609.3 (this predicate is false). pub fn options_supplied_by_player(&self) -> bool { - matches!(self, Self::CardName | Self::Word | Self::Artist) + matches!( + self, + Self::CardName + | Self::Word + | Self::Artist + // CR 107.1a/b: an unbounded number choice cannot be enumerated, + // so the player supplies the value. Bounded ranges keep their + // option list and their button-per-value rendering. + | Self::NumberRange { max: None, .. } + ) + } + + /// CR 107.1a/b + CR 608.2d: Is `answer` a legal value for this choice when + /// the engine cannot offer an option list to check it against? + /// + /// The single authority for validating a free-entry answer, shared by the + /// interactive handler and the AI's legal-action enumeration so a value one + /// accepts cannot be rejected by the other. Returns `None` for choice kinds + /// whose answers are validated by membership instead. + /// + /// Delegates to [`ChoiceType::free_entry`] so the rule this enforces and the + /// contract published to clients are the same value, not two statements of + /// the same intent. + pub fn accepts_free_entry_answer(&self, answer: &str) -> Option { + match self.free_entry()? { + FreeEntry::Number { min, max } => { + let parsed = answer.trim().parse::(); + Some(parsed.is_ok_and(|n| n >= min && n <= max)) + } + } + } + + /// CR 107.1a/b: The free-entry contract for this choice, or `None` when the + /// answer is picked from an option list instead. + /// + /// This is the ONE definition of what a free-entry answer may be. It is what + /// [`ChoiceType::accepts_free_entry_answer`] validates against, what the AI's + /// legal-action enumeration samples within, and — published on + /// `WaitingFor::NamedChoice` — what a client renders and bounds its input by. + /// A client that reads this contract cannot reject a value the engine accepts, + /// because there is no second statement of the domain to drift from. + pub fn free_entry(&self) -> Option { + match self { + // CR 107.1a/b: an unbounded number choice cannot be enumerated, so + // the player supplies the value. Bounded ranges keep their option + // list and are validated by membership. + Self::NumberRange { min, max: None, .. } => Some(FreeEntry::Number { + min: *min, + // Not a UI cap, but the engine's own arithmetic domain: every + // quantity resolves through `i32`, so a number beyond this could + // not be dealt as damage or compared against a life total. + // Within that domain every value the rules permit is accepted. + max: i32::MAX as u32, + }), + _ => None, + } } } +/// CR 107.1a/b: A choice whose answer the player supplies rather than picks from +/// an enumerated list, together with the bounds that make an answer legal. +/// +/// Published on the prompt (`WaitingFor::NamedChoice::free_entry`) so a client +/// renders and bounds the input from engine-stated values instead of +/// re-deriving them from the choice's own shape. `Number`'s bounds are both +/// INCLUSIVE. `CardName` and the other unbounded-string choices are deliberately +/// absent: their answers are validated against the card corpus, not a range, so +/// they have no contract of this form to publish. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "kind")] +pub enum FreeEntry { + Number { min: u32, max: u32 }, +} + impl Serialize for ChoiceType { fn serialize(&self, serializer: S) -> Result where @@ -840,7 +932,17 @@ impl Serialize for ChoiceType { } => { // Emit `distinctness` only when non-default so existing // `{min,max}` card-data stays byte-stable. - let field_count = 2 + (*distinctness != NumberDistinctness::Repeatable) as usize; + // + // CR 107.1a/b: emit `max` only when the range HAS one. This is a + // hand-written `Serialize`, so the `skip_serializing_if` on the + // `ChoiceTypeData` deserialize mirror does not apply here and has + // to be mirrored by hand — otherwise an unbounded range writes + // `"max": null`, which round-trips correctly but needlessly + // changes the wire shape and reads as "a null bound" rather than + // "no bound". + let field_count = 1 + + max.is_some() as usize + + (*distinctness != NumberDistinctness::Repeatable) as usize; let mut variant = serializer.serialize_struct_variant( "ChoiceType", 6, @@ -848,7 +950,9 @@ impl Serialize for ChoiceType { field_count, )?; variant.serialize_field("min", min)?; - variant.serialize_field("max", max)?; + if let Some(max) = max { + variant.serialize_field("max", max)?; + } if *distinctness != NumberDistinctness::Repeatable { variant.serialize_field("distinctness", distinctness)?; } @@ -977,8 +1081,13 @@ impl<'de> Deserialize<'de> for ChoiceType { excluded: Vec, }, NumberRange { - min: u8, - max: u8, + min: u32, + /// CR 107.1a/b: absent = no maximum. A bounded range keeps + /// emitting `"max": N` exactly as before, so existing card-data + /// round-trips byte-identically; only the unbounded form omits + /// the key. + #[serde(default, skip_serializing_if = "Option::is_none")] + max: Option, #[serde(default)] distinctness: NumberDistinctness, }, @@ -1380,7 +1489,22 @@ pub enum ChosenAttribute { OddOrEven(Parity), CardName(String), /// Stores a chosen number (e.g., "choose a number" for Talion). - Number(u8), + /// + /// On the PLAYER axis (`Player::chosen_attributes`) this is the SECRET half + /// of the CR 101.4 secret-number ledger: `game::visibility` redacts it from + /// every viewer but its owner. `Effect::RevealChosenNumbers` converts it to + /// [`ChosenAttribute::RevealedNumber`], which is public — that conversion is + /// the card's "reveal" instruction as an observable state transition. + Number(u32), + /// CR 101.4 + CR 608.2c: A chosen number that a reveal instruction has + /// PUBLISHED ("then all players reveal those numbers simultaneously"). + /// Identical in value to [`ChosenAttribute::Number`] and read + /// interchangeably with it by `Player::chosen_number`; the two differ only + /// in visibility, which is exactly what the reveal changes. Kept as a + /// distinct variant rather than a flag so the secret and published states + /// cannot be confused at a read site, and so `game::visibility` redacts on + /// the type rather than on a condition it might forget to check. + RevealedNumber(u32), /// Stores the chosen opponent/player ID (CR 800.4a). Player(PlayerId), /// Stores two chosen colors as a pair. @@ -1464,9 +1588,14 @@ impl ChosenAttribute { Self::CardType(_) => ChoiceType::card_type(), Self::OddOrEven(_) => ChoiceType::OddOrEven, Self::CardName(_) => ChoiceType::CardName, - Self::Number(_) => ChoiceType::NumberRange { + // CR 101.4: the secret and the published number came from the same + // `NumberRange` prompt; revealing changes visibility, not category. + // CR 107.1a/b: recovering the CATEGORY from a stored value cannot + // recover the card's original bounds, so report the widest form the + // rules allow rather than inventing a ceiling this value never had. + Self::Number(_) | Self::RevealedNumber(_) => ChoiceType::NumberRange { min: 0, - max: 20, + max: None, distinctness: NumberDistinctness::Repeatable, }, // Player covers both Player and Opponent choice types @@ -1567,7 +1696,7 @@ pub enum ChoiceValue { CardType(CoreType), OddOrEven(Parity), CardName(String), - Number(u8), + Number(u32), Label(String), CardPredicate(CardPredicateChoice), LandType(String), @@ -1601,7 +1730,7 @@ impl ChoiceValue { } ChoiceType::OddOrEven => value.parse::().ok().map(Self::OddOrEven), ChoiceType::CardName => Some(Self::CardName(value.to_string())), - ChoiceType::NumberRange { .. } => value.parse::().ok().map(Self::Number), + ChoiceType::NumberRange { .. } => value.parse::().ok().map(Self::Number), ChoiceType::Labeled { .. } => Some(Self::Label(value.to_string())), ChoiceType::CardPredicate { options } | ChoiceType::CardPredicateGuess { options } => { let predicate = CardPredicateChoice::from_label(value)?; @@ -6467,6 +6596,32 @@ pub enum QuantityRef { /// A number chosen as the source entered the battlefield (e.g., Talion, the Kindly Lord). /// Resolved from the source object's `ChosenAttribute::Number`. ChosenNumber, + /// CR 101.4 + CR 608.2d: The number a PLAYER chose during this resolution + /// ("each player secretly chooses a number 0 or greater"), read off + /// `Player::chosen_attributes` (`ChosenAttribute::Number`) — the player-axis + /// sibling of the object-axis [`QuantityRef::ChosenNumber`], which reads the + /// SOURCE object's persisted number instead. The two subjects have different + /// runtime resolvers (per-player scalar vs. source LKI), so they stay + /// separate variants rather than one subject-parameterized reference. + /// + /// A member of the per-player-scalar subset (`HandSize` / `LifeTotal` / + /// `GraveyardSize` / `PlayerCounter` / …), so `player` selects both WHICH + /// player is read and — for the aggregate scopes — HOW the per-player values + /// are folded: + /// - `AllPlayers { aggregate: Max }` — "the highest number" (Wheel of + /// Misfortune, Menacing Ogre, Life at Stake). + /// - `AllPlayers { aggregate: Min }` — "the lowest number" (Wheel of + /// Misfortune's discard clause). + /// - `ScopedPlayer` — the per-candidate read used by + /// [`PlayerFilter::PlayerAttribute`] to select "each player who chose the + /// highest number". + /// + /// Players who chose no number this resolution are EXCLUDED from the + /// aggregate populations (rather than contributing 0), so a card whose + /// choosers are a subset of the table — Life at Stake's "you and target + /// creature's controller" — still reads the extremum over the actual + /// choosers. + PlayerChosenNumber { player: PlayerScope }, /// CR 508.1a: Number of creatures that attacked this turn, scoped by /// `scope` and optionally narrowed by `filter` (e.g. "attacked with a /// token / a commander / a Wolf"). `Controller` + `filter: None` counts all @@ -7523,22 +7678,25 @@ impl QuantityExpr { } } - /// CR 608.2c: Rebind a later clause's generic event-context amount to the - /// scalar result of the immediately preceding resolved instruction. + /// CR 608.2c: Rebind a later clause's generic event-context amount ("that + /// much", "that many") to the `antecedent` the surrounding grammar names. /// - /// Parser chain assembly uses this only when grammar proves that the - /// antecedent is the prior effect, rather than a triggering event or a - /// per-player iteration. The recursive walk preserves arithmetic wrappers - /// such as "twice that much". - pub fn rebind_event_context_amount_to_previous_effect(&mut self) { + /// `EventContextAmount` is the *unbound* demonstrative: it means "the amount + /// from the surrounding event context", which is correct only when a + /// triggering event or a per-player iteration supplies one. When chain + /// assembly can PROVE a different antecedent from the printed grammar — the + /// preceding instruction's scalar result, or a number a preceding clause had + /// a player choose — it rebinds the leaf here. The antecedent is a parameter + /// rather than one method per referent, so every provable binding shares one + /// recursive walk (which preserves arithmetic wrappers such as "twice that + /// much"); callers must not use it for merely plausible antecedents. + pub fn rebind_event_context_amount(&mut self, antecedent: &QuantityRef) { match self { QuantityExpr::Ref { qty: QuantityRef::EventContextAmount, } => { *self = QuantityExpr::Ref { - qty: QuantityRef::PreviousEffectAmount { - channel: DamageChannel::Total, - }, + qty: antecedent.clone(), }; } QuantityExpr::Offset { inner, .. } @@ -7548,15 +7706,15 @@ impl QuantityExpr { | QuantityExpr::UpTo { max: inner } | QuantityExpr::Power { exponent: inner, .. - } => inner.rebind_event_context_amount_to_previous_effect(), + } => inner.rebind_event_context_amount(antecedent), QuantityExpr::Sum { exprs } | QuantityExpr::Max { exprs } => { for expr in exprs { - expr.rebind_event_context_amount_to_previous_effect(); + expr.rebind_event_context_amount(antecedent); } } QuantityExpr::Difference { left, right } => { - left.rebind_event_context_amount_to_previous_effect(); - right.rebind_event_context_amount_to_previous_effect(); + left.rebind_event_context_amount(antecedent); + right.rebind_event_context_amount(antecedent); } QuantityExpr::Fixed { .. } | QuantityExpr::Ref { .. } => {} } @@ -12446,6 +12604,33 @@ pub enum Effect { #[serde(default = "default_target_filter_self_ref")] target: TargetFilter, }, + /// CR 101.4 + CR 608.2c: Publish the numbers `players` secretly chose earlier + /// in this resolution — "then all players reveal those numbers + /// simultaneously" (Wheel of Misfortune), "then you reveal the number you + /// chose" (The Toymaker's Trap), "Then those numbers are revealed" (Menacing + /// Ogre). + /// + /// Deliberately NOT a member of the `Reveal` / `RevealTop` / `RevealHand` + /// family: CR 701.20a defines revealing a CARD ("show that card to all + /// players"), and those effects are parameterized over zone, count and card + /// filter. A committed number is not a card and has none of those axes — it + /// is a per-player choice made during resolution (CR 608.2d), so it gets its + /// own publication channel rather than a card-reveal variant bent to fit. + /// + /// A player's chosen number is private until this effect names them: the + /// resolver calls + /// [`crate::types::player::Player::reveal_chosen_number`], which swaps that + /// player's [`ChosenAttribute::Number`] for + /// [`ChosenAttribute::RevealedNumber`]. `game::visibility` redacts the + /// former from every other viewer and leaves the latter public, so privacy + /// is a property of the attribute kind rather than of any separate flag. + /// Naming a player who chose no number is a legal no-op (CR 609.3), which is + /// what makes `players: All` correct for a card whose choosers were only a + /// subset of the table. + RevealChosenNumbers { + #[serde(default)] + players: PlayerFilter, + }, /// CR 701.20a: Reveal the top N card(s) of a player's library. RevealTop { /// The player whose library to reveal from. @@ -15665,6 +15850,7 @@ impl Effect { Effect::StartYourEngines { .. } // CR 311.7: the chaos anchor swap is a non-targeting per-player effect. | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } // CR 109.4: owner/type_filter are non-targeting resolution-time // filters; the copy source is chosen from the format pool, not // declared as a target. @@ -16544,6 +16730,7 @@ impl Effect { | Effect::StartYourEngines { .. } | Effect::Suspect { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::SwitchPT { .. } | Effect::TakeTheInitiative | Effect::TargetOnly { .. } @@ -17096,6 +17283,7 @@ impl Effect { | Effect::TargetOnly { .. } | Effect::Choose { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::ChooseDamageSource { .. } | Effect::Suspect { .. } | Effect::Unsuspect { .. } @@ -17375,6 +17563,7 @@ impl Effect { | Effect::Cascade | Effect::Choose { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::ChooseAndSacrificeRest { .. } | Effect::EachPlayerCopyChosen { .. } | Effect::ChooseDamageSource { .. } @@ -17637,6 +17826,7 @@ impl Effect { | Effect::Cascade | Effect::Choose { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::ChooseAndSacrificeRest { .. } | Effect::EachPlayerCopyChosen { .. } | Effect::ChooseDamageSource { .. } @@ -17840,6 +18030,7 @@ pub fn effect_variant_name(effect: &Effect) -> &str { Effect::Choose { .. } => "Choose", Effect::OpponentGuess { .. } => "OpponentGuess", Effect::SwapChosenLabels { .. } => "SwapChosenLabels", + Effect::RevealChosenNumbers { .. } => "RevealChosenNumbers", Effect::ChooseDamageSource { .. } => "ChooseDamageSource", Effect::Suspect { .. } => "Suspect", Effect::Unsuspect { .. } => "Unsuspect", @@ -18355,6 +18546,10 @@ impl From<&Effect> for EffectKind { // CR 311.7: The chaos swap re-chooses each player's anchor, so it // reports as a `Choose`-kind resolution for event/AI purposes. Effect::SwapChosenLabels { .. } => EffectKind::Choose, + // CR 101.4: publishing a chosen number is a choice-ledger write, + // classified with the choice that produced it rather than with the + // CR 701.20 card reveals. + Effect::RevealChosenNumbers { .. } => EffectKind::Choose, Effect::ChooseDamageSource { .. } => EffectKind::ChooseDamageSource, Effect::Suspect { .. } => EffectKind::Suspect, Effect::Unsuspect { .. } => EffectKind::Unsuspect, @@ -26175,7 +26370,7 @@ mod tests { legacy, ChoiceType::NumberRange { min: 1, - max: 5, + max: Some(5), distinctness: NumberDistinctness::Repeatable, } ); @@ -26185,10 +26380,32 @@ mod tests { "Repeatable must not emit the distinctness field" ); + // CR 107.1a/b: making `max` optional must not disturb the BOUNDED wire + // shape — the assertions above already prove `"max":5` both reads and + // writes unchanged, so existing card-data round-trips byte-identically. + // The UNBOUNDED form is the new shape: it omits the key entirely, and a + // payload with no `max` reads back as unbounded rather than defaulting to + // some ceiling. + let unbounded = ChoiceType::NumberRange { + min: 0, + max: None, + distinctness: NumberDistinctness::Repeatable, + }; + assert_eq!( + serde_json::to_string(&unbounded).unwrap(), + r#"{"NumberRange":{"min":0}}"#, + "an unbounded range must omit max rather than emit a stand-in" + ); + assert_eq!( + serde_json::from_str::(r#"{"NumberRange":{"min":0}}"#).unwrap(), + unbounded, + "a payload with no max is unbounded, not defaulted" + ); + // A DistinctFromSourceHistory value round-trips and emits the field. let distinct = ChoiceType::NumberRange { min: 1, - max: 5, + max: Some(5), distinctness: NumberDistinctness::DistinctFromSourceHistory, }; let json = serde_json::to_string(&distinct).unwrap(); diff --git a/crates/engine/src/types/ability_visit.rs b/crates/engine/src/types/ability_visit.rs index c324f20045..d19f4aaec9 100644 --- a/crates/engine/src/types/ability_visit.rs +++ b/crates/engine/src/types/ability_visit.rs @@ -897,6 +897,7 @@ where | Effect::Choose { .. } | Effect::OpponentGuess { .. } | Effect::SwapChosenLabels { .. } + | Effect::RevealChosenNumbers { .. } | Effect::ChooseDamageSource { .. } | Effect::Suspect { .. } | Effect::Unsuspect { .. } diff --git a/crates/engine/src/types/events.rs b/crates/engine/src/types/events.rs index 09230cc3a4..9c09fd36a8 100644 --- a/crates/engine/src/types/events.rs +++ b/crates/engine/src/types/events.rs @@ -1320,6 +1320,20 @@ pub enum GameEvent { card_ids: Vec, card_names: Vec, }, + /// CR 101.4 + CR 608.2c: Secretly-chosen numbers were published by a reveal + /// instruction ("then all players reveal those numbers simultaneously" — + /// Wheel of Misfortune). One event carries every number published by the + /// single instruction, because the card reveals them SIMULTANEOUSLY; a + /// per-player event would imply an ordering the rules do not have. + /// + /// Distinct from `CardsRevealed`, which is CR 701.20 (showing a card). This + /// is the game log's and the frontend's view of the secret→public + /// transition that `game::visibility` enforces on + /// `ChosenAttribute::RevealedNumber`. + ChosenNumbersRevealed { + /// Each revealing player and the number they had chosen, in APNAP order. + numbers: Vec<(PlayerId, u32)>, + }, CombatDamageDealtToPlayer { player_id: PlayerId, /// CR 120.1 + CR 510.2: Per-source combat damage amounts for this diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 8f9dc2dae3..81bd056f62 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -11067,6 +11067,17 @@ pub enum WaitingFor { /// object-scoped binding used by Khans Sieges and every other named choice. #[serde(default, skip_serializing_if = "Option::is_none")] persist_player: Option, + /// CR 107.1a/b: the free-entry contract for `choice_type`, when its + /// answer is supplied by the player rather than picked from `options`. + /// + /// Published so a client renders the right control and bounds its input + /// from engine-stated values, instead of inspecting `choice_type`'s + /// serialized shape and restating the domain itself — a second authority + /// that could reject a value the engine accepts. Always + /// `choice_type.free_entry()`; `named_choice_free_entry_matches_choice_type` + /// pins that across every prompt the engine raises. + #[serde(default, skip_serializing_if = "Option::is_none")] + free_entry: Option, }, /// CR 608.2d + CR 608.2e: a player other than the controller (an opponent / /// the defending player) guesses a committed value or proposition during diff --git a/crates/engine/src/types/player.rs b/crates/engine/src/types/player.rs index 48877106d2..065b85313e 100644 --- a/crates/engine/src/types/player.rs +++ b/crates/engine/src/types/player.rs @@ -258,6 +258,46 @@ fn default_contraption_crank_sprocket() -> u8 { } impl Player { + /// CR 101.4 + CR 608.2d: The number this player most recently chose for a + /// per-player `Effect::Choose { ChoiceType::NumberRange }` ("each player + /// secretly chooses a number 0 or greater"). `None` when this player has + /// not chosen a number — the aggregate readers of + /// [`crate::types::ability::QuantityRef::PlayerChosenNumber`] use that to + /// exclude non-choosers from a highest/lowest fold instead of counting them + /// as 0. + /// + /// Reads the secret and the revealed variants interchangeably: revealing a + /// number changes who may SEE it, never what it is, so every rules read + /// ("the highest number", "each player who chose the lowest number") must + /// return the same value on both sides of the reveal. + pub fn chosen_number(&self) -> Option { + use crate::types::ability::ChosenAttribute; + self.chosen_attributes.iter().find_map(|attr| match attr { + ChosenAttribute::Number(n) | ChosenAttribute::RevealedNumber(n) => Some(*n), + _ => None, + }) + } + + /// CR 101.4 + CR 608.2c: Publish this player's secretly-chosen number, the + /// state transition behind "then all players reveal those numbers + /// simultaneously". Returns the revealed value, or `None` when this player + /// chose no number (CR 609.3 — revealing nothing is a legal no-op, which is + /// what lets a card name every player when only some of them chose). + /// Idempotent: an already-revealed number stays revealed. + pub fn reveal_chosen_number(&mut self) -> Option { + use crate::types::ability::ChosenAttribute; + let value = self.chosen_number()?; + self.chosen_attributes.retain(|attribute| { + !matches!( + attribute, + ChosenAttribute::Number(_) | ChosenAttribute::RevealedNumber(_) + ) + }); + self.chosen_attributes + .push(ChosenAttribute::RevealedNumber(value)); + Some(value) + } + /// CR 122.1: Get the current count of a player counter. /// Poison counters route to the dedicated field (SBA at CR 704.5c). pub fn player_counter(&self, kind: &PlayerCounterKind) -> u32 { diff --git a/crates/engine/tests/integration/chosen_number_opponent_restriction.rs b/crates/engine/tests/integration/chosen_number_opponent_restriction.rs new file mode 100644 index 0000000000..7a15fd0ce6 --- /dev/null +++ b/crates/engine/tests/integration/chosen_number_opponent_restriction.rs @@ -0,0 +1,194 @@ +//! CR 101.4 + CR 608.2c: *"Choose an opponent with the highest number"* — +//! Itazura, Lingering Wick's selection clause. +//! +//! The restriction is part of the instruction, not decoration. Dropping it lets +//! the controller pick an opponent who did NOT choose the highest number and +//! then deal them the damage — a legal-looking choice the rules forbid. This +//! test pins that the offered option set is narrowed to the actual highest +//! chooser(s), in both the unique and the tied case, and that the damage follows +//! the selection through the "them" anaphor. +//! +//! Oracle-text note: the clause under test is exercised here on a distilled +//! spell carrying Itazura's exact selection-and-damage wording, because the real +//! card wraps it in unrelated exile/free-cast machinery whose own gaps would +//! dominate the assertions. Itazura's VERBATIM text is covered at the parse +//! layer by `secret_number_provenance_invariant_holds_across_the_class`, which +//! pins the card as a genuine `PlayerChosenNumber` reader — so the pairing +//! covers both "the real card binds the restriction" and "the restriction is +//! enforced at runtime". +//! +//! Fail-on-revert: remove the restriction from the `ChoiceType::Opponent` seam +//! and the non-highest opponent reappears in `options`, failing the first +//! assertion in each case. +//! +//! CR 101.4: APNAP order for simultaneous choices. +//! CR 120.3a: damage dealt to a player by a source without infect causes that +//! player to lose that much life. +//! CR 608.2c: follow the instructions in the order written. +//! CR 608.2d: a choice offered by a resolving spell is announced while applying +//! the effect; an illegal option can't be chosen. + +use engine::game::scenario::GameScenario; +use engine::types::ability::ChoiceType; +use engine::types::actions::GameAction; +use engine::types::game_state::{CastPaymentMode, GameState, WaitingFor}; +use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; + +const P0: PlayerId = PlayerId(0); +const P1: PlayerId = PlayerId(1); +const P2: PlayerId = PlayerId(2); + +/// Itazura's selection-and-damage wording, verbatim from the card, with its +/// unrelated exile/free-cast tail removed. +const ORACLE: &str = "Each opponent secretly chooses a number 0 or greater. Then those numbers are revealed. Choose an opponent with the highest number. Itazura deals that much damage to them."; + +fn life(state: &GameState, player: PlayerId) -> i32 { + state.players[player.0 as usize].life +} + +/// Drives the spell with the given per-opponent bids, returning the option list +/// offered for the opponent choice and the final state. +fn run(bids: &[(PlayerId, &str)]) -> (Vec, GameState) { + let mut scenario = GameScenario::new_n_player(3, 42); + scenario.at_phase(Phase::PreCombatMain); + for player in [P0, P1, P2] { + scenario.with_library_top(player, &["Lib 1", "Lib 2", "Lib 3"]); + scenario.with_life(player, 20); + } + + let mut spell_builder = + scenario.add_spell_to_hand_from_oracle(P0, "Itazura, Lingering Wick", false, ORACLE); + spell_builder.with_mana_cost(ManaCost::Cost { + generic: 1, + shards: vec![ManaCostShard::Red], + }); + let spell = spell_builder.id(); + scenario.with_mana_pool( + P0, + vec![ + ManaUnit::new(ManaType::Red, spell, false, vec![]), + ManaUnit::new(ManaType::Red, spell, false, vec![]), + ], + ); + + let mut runner = scenario.build(); + let card_id = runner.state().objects[&spell].card_id; + runner + .act(GameAction::CastSpell { + object_id: spell, + card_id, + targets: vec![], + payment_mode: CastPaymentMode::Auto, + }) + .expect("casting must start"); + + let mut opponent_options: Vec = Vec::new(); + for _ in 0..256 { + match runner.state().waiting_for.clone() { + WaitingFor::ManaPayment { .. } => { + runner.act(GameAction::PassPriority).expect("mana"); + } + WaitingFor::NamedChoice { + player, + options, + choice_type, + .. + } => { + let choice = match choice_type { + // The secret number: free-entry, answered per the bid table. + ChoiceType::NumberRange { .. } => bids + .iter() + .find(|(seat, _)| *seat == player) + .map(|(_, bid)| (*bid).to_string()) + .unwrap_or_else(|| panic!("unexpected number chooser {player:?}")), + // THE ASSERTION SURFACE: which opponents the engine offers. + ChoiceType::Opponent { .. } => { + opponent_options = options.clone(); + options + .first() + .cloned() + .expect("an opponent must be offered") + } + other => panic!("unexpected choice {other:?}"), + }; + runner + .act(GameAction::ChooseOption { choice }) + .unwrap_or_else(|e| panic!("answering {player:?} must succeed: {e:?}")); + } + WaitingFor::Priority { .. } => { + if runner.act(GameAction::PassPriority).is_err() { + break; + } + if runner.state().stack.is_empty() && !opponent_options.is_empty() { + break; + } + } + other => panic!("unexpected prompt: {other:?}"), + } + } + (opponent_options, runner.state().clone()) +} + +/// A UNIQUE highest chooser: only that opponent may be selected, and the damage +/// equals their number. The opponent who bid lower must not be offered at all — +/// CR 608.2d, an illegal option can't be chosen. +#[test] +fn only_the_highest_chooser_is_offered_and_takes_the_damage() { + let (options, state) = run(&[(P1, "5"), (P2, "2")]); + + assert_eq!( + options, + vec![P1.0.to_string()], + "only the opponent who chose the highest number may be selected; \ + offering P2 (who chose 2) is the illegal choice the restriction prevents" + ); + // CR 120.3a + CR 608.2c: "that much damage to them" follows the selection. + assert_eq!( + life(&state, P1), + 15, + "P1 chose 5 and takes exactly that much" + ); + assert_eq!( + life(&state, P2), + 20, + "P2 was not selectable and is untouched" + ); + assert_eq!(life(&state, P0), 20, "the controller is not an opponent"); +} + +/// A TIE for highest: both opponents are legal selections (CR 608.2d resolves +/// ties by leaving the choice to the controller), and whichever is chosen takes +/// the tied number. This is the case a "first match wins" implementation would +/// get wrong by narrowing to one seat. +#[test] +fn tied_highest_choosers_are_both_offered() { + let (options, state) = run(&[(P1, "4"), (P2, "4")]); + + let mut sorted = options.clone(); + sorted.sort(); + assert_eq!( + sorted, + vec![P1.0.to_string(), P2.0.to_string()], + "both opponents tied for the highest number must be selectable" + ); + + // Exactly one of them took 4; the other is untouched. Which one is the + // controller's choice (the driver picks the first offered), so assert the + // shape rather than a specific seat. + let damaged: Vec = [P1, P2] + .into_iter() + .filter(|p| life(&state, *p) == 16) + .collect(); + let untouched: Vec = [P1, P2] + .into_iter() + .filter(|p| life(&state, *p) == 20) + .collect(); + assert_eq!( + damaged.len(), + 1, + "exactly one tied opponent takes the damage" + ); + assert_eq!(untouched.len(), 1, "the other tied opponent is untouched"); +} diff --git a/crates/engine/tests/integration/frostcliff_siege_anchor_word_modes.rs b/crates/engine/tests/integration/frostcliff_siege_anchor_word_modes.rs index be18dc048b..30bf1813d2 100644 --- a/crates/engine/tests/integration/frostcliff_siege_anchor_word_modes.rs +++ b/crates/engine/tests/integration/frostcliff_siege_anchor_word_modes.rs @@ -82,6 +82,7 @@ fn drive_siege_choice( ) { let source = crate::support::exact_named_choice_source(runner.state(), siege); runner.state_mut().waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: P0, choice_type: ChoiceType::Labeled { options: vec!["Jeskai".to_string(), "Temur".to_string()], @@ -514,6 +515,7 @@ fn cast_siege_from_hand(runner: &mut GameRunner, siege: ObjectId, chosen_label: // is silently dropped on real ETB. match &runner.state().waiting_for { WaitingFor::NamedChoice { + free_entry: None, player, choice_type, options, diff --git a/crates/engine/tests/integration/greymond_avacyns_stalwart.rs b/crates/engine/tests/integration/greymond_avacyns_stalwart.rs index a6a7ce037f..22c73fd8fc 100644 --- a/crates/engine/tests/integration/greymond_avacyns_stalwart.rs +++ b/crates/engine/tests/integration/greymond_avacyns_stalwart.rs @@ -430,6 +430,7 @@ fn ai_candidates_three_pairs_for_greymond_choice() { // Drive state into the keyword NamedChoice for Greymond's choice. let source = crate::support::exact_named_choice_source(runner.state(), greymond); runner.state_mut().waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: P0, choice_type: ChoiceType::Keyword { options: vec![Keyword::FirstStrike, Keyword::Vigilance, Keyword::Lifelink], diff --git a/crates/engine/tests/integration/life_at_stake_both_choosers_6965.rs b/crates/engine/tests/integration/life_at_stake_both_choosers_6965.rs index ad4cc04071..1731023e53 100644 --- a/crates/engine/tests/integration/life_at_stake_both_choosers_6965.rs +++ b/crates/engine/tests/integration/life_at_stake_both_choosers_6965.rs @@ -119,10 +119,21 @@ fn life_at_stake_prompts_the_caster_then_the_targets_controller() { if matches!(choice_type, ChoiceType::NumberRange { .. }) { number_choosers.push(player); } - let choice = options - .first() - .cloned() - .expect("a number choice must offer options"); + // CR 107.1a/b: Life at Stake says "a number 0 or greater", which + // states no maximum — so the prompt enumerates nothing and the + // value is supplied by the player. Answer from the free-entry + // path when there is no option list; the bounded prompts this + // loop also sees (target selection, etc.) still pick an option. + let choice = match options.first() { + Some(option) => option.clone(), + None => { + assert!( + choice_type.options_supplied_by_player(), + "an optionless prompt must be a free-entry one, got {choice_type:?}" + ); + "3".to_string() + } + }; runner .act(GameAction::ChooseOption { choice }) .expect("answering the number choice must succeed"); diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index fda6125963..d8cc52c74d 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -90,6 +90,7 @@ mod chains_of_mephistopheles_discard_draw_or_mill; mod chandra_revolution_doesnt_untap_slot; mod charging_cinderhorn_issue_2868; mod chatterstorm_storm; +mod chosen_number_opponent_restriction; mod claim_jumper_repeat; mod cleanup_discard_trigger_pipeline; mod cleave_text_changing_cost; @@ -855,6 +856,7 @@ mod mutable_pupa_perpetual_keyword_mirror; mod mycoloth_upkeep_trigger; mod myrkul_crew_phase1_incarnation; mod mystic_forge_regression; +mod named_choice_free_entry_contract; mod narci_fable_singer_final_chapter_drain; mod narset_jeskai_waymaster_draw_spells_cast; mod natural_balance; @@ -1342,6 +1344,7 @@ mod vohar_discard_drain; mod wand_of_orcus_compound_subject_6965; mod weeping_angel_combat_prevention; mod wheel_and_deal; +mod wheel_of_misfortune_secret_numbers; mod where_x_coverage_runtime; mod where_x_quantity_channel_binds; mod where_x_totality_guard; diff --git a/crates/engine/tests/integration/modal_enters_becomes_choice.rs b/crates/engine/tests/integration/modal_enters_becomes_choice.rs index f0af417c8c..53df0fbb1d 100644 --- a/crates/engine/tests/integration/modal_enters_becomes_choice.rs +++ b/crates/engine/tests/integration/modal_enters_becomes_choice.rs @@ -63,6 +63,7 @@ fn place_and_choose( let source = crate::support::exact_named_choice_source(runner.state(), obj); runner.state_mut().waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: P0, choice_type: ChoiceType::Labeled { options: labels.iter().map(|s| s.to_string()).collect(), @@ -383,6 +384,7 @@ fn cast_and_engine_choose( // LOAD-BEARING: the engine (not the test) must have paused the entry on the // modal choice produced by the Moved/Battlefield Choose replacement. let WaitingFor::NamedChoice { + free_entry: None, options, source: Some(source), .. diff --git a/crates/engine/tests/integration/morophon_chosen_type_1653.rs b/crates/engine/tests/integration/morophon_chosen_type_1653.rs index 6a0631c178..bd2e0b4910 100644 --- a/crates/engine/tests/integration/morophon_chosen_type_1653.rs +++ b/crates/engine/tests/integration/morophon_chosen_type_1653.rs @@ -111,6 +111,7 @@ fn morophon_creature_type_choice_marks_layers_dirty() { let source = crate::support::exact_named_choice_source(runner.state(), morophon); runner.state_mut().waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: P0, choice_type: ChoiceType::creature_type(), options: vec!["Human".to_string(), "Elf".to_string()], @@ -156,6 +157,7 @@ fn card_name_choice_marks_layers_dirty_for_chosen_name_static() { let choice_source = crate::support::exact_named_choice_source(runner.state(), source); runner.state_mut().waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: P0, choice_type: ChoiceType::CardName, options: Vec::new(), diff --git a/crates/engine/tests/integration/named_choice_free_entry_contract.rs b/crates/engine/tests/integration/named_choice_free_entry_contract.rs new file mode 100644 index 0000000000..458136a21d --- /dev/null +++ b/crates/engine/tests/integration/named_choice_free_entry_contract.rs @@ -0,0 +1,201 @@ +//! CR 107.1a/b: the free-entry contract a `NamedChoice` prompt publishes. +//! +//! An unbounded number choice ("choose a number 0 or greater") cannot be +//! offered as an option list, so the player types a value. Something has to tell +//! the client what a legal value is. If the client works that out for itself — +//! by inspecting the serialized `ChoiceType` shape and restating the numeric +//! domain — it becomes a second authority, free to reject a value the engine +//! would have accepted, and free to drift when the engine's domain changes. +//! +//! So the engine publishes the contract on the prompt and enforces answers +//! against that same value. These tests pin both halves at the adapter surface a +//! client actually consumes: +//! +//! 1. the projected prompt carries the contract, and it equals +//! `choice_type.free_entry()` — the one definition; +//! 2. the contract survives JSON serialization with its bounds readable, so a +//! client never has to decode `ChoiceType` to find them; +//! 3. the published maximum is exactly the boundary the engine enforces — it +//! accepts that value and rejects the next one up. +//! +//! Point 3 is what makes this more than a shape test: a published bound that +//! didn't match the enforced bound would pass 1 and 2 and still be the defect. +//! +//! Fail-on-revert: recompute the contract anywhere other than +//! `ChoiceType::free_entry`, or let the prompt omit it, and 1 or 3 fails. + +use engine::game::scenario::GameScenario; +use engine::game::visibility::filter_state_for_viewer; +use engine::types::ability::{ChoiceType, FreeEntry}; +use engine::types::actions::GameAction; +use engine::types::game_state::{CastPaymentMode, GameState, WaitingFor}; +use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; + +const P0: PlayerId = PlayerId(0); + +/// Wheel of Misfortune's choice clause, which states no maximum. +const ORACLE: &str = "Each player secretly chooses a number 0 or greater."; + +/// Casts a spell carrying `ORACLE` and stops on the first prompt it raises. +fn stop_at_number_prompt() -> engine::game::scenario::GameRunner { + let mut scenario = GameScenario::new_n_player(2, 7); + scenario.at_phase(Phase::PreCombatMain); + for player in [P0, PlayerId(1)] { + scenario.with_library_top(player, &["Lib 1", "Lib 2"]); + scenario.with_life(player, 20); + } + + let mut builder = scenario.add_spell_to_hand_from_oracle(P0, "Wheel Probe", false, ORACLE); + builder.with_mana_cost(ManaCost::Cost { + generic: 0, + shards: vec![ManaCostShard::Red], + }); + let spell = builder.id(); + scenario.with_mana_pool(P0, vec![ManaUnit::new(ManaType::Red, spell, false, vec![])]); + + let mut runner = scenario.build(); + let card_id = runner.state().objects[&spell].card_id; + runner + .act(GameAction::CastSpell { + object_id: spell, + card_id, + targets: vec![], + payment_mode: CastPaymentMode::Auto, + }) + .expect("casting must start"); + + for _ in 0..64 { + if matches!(runner.state().waiting_for, WaitingFor::NamedChoice { .. }) { + return runner; + } + if runner.act(GameAction::PassPriority).is_err() { + break; + } + } + panic!("the spell never raised its number prompt"); +} + +fn prompt_parts(state: &GameState) -> (ChoiceType, Option) { + match &state.waiting_for { + WaitingFor::NamedChoice { + choice_type, + free_entry, + .. + } => (choice_type.clone(), *free_entry), + other => panic!("expected a named choice, got {other:?}"), + } +} + +/// (1) and (2): the prompt a client receives carries the contract, and it is the +/// single definition rather than a copy that could disagree with it. +#[test] +fn an_unbounded_number_prompt_publishes_its_entry_contract() { + let runner = stop_at_number_prompt(); + + // Read it from the PROJECTED state — what a client is actually sent — not + // from authoritative state, so a projection that dropped the field fails. + let projected = filter_state_for_viewer(runner.state(), P0); + let (choice_type, published) = prompt_parts(&projected); + + assert_eq!( + published, + choice_type.free_entry(), + "the published contract must BE the engine's definition, not a second \ + copy of it" + ); + let Some(FreeEntry::Number { min, max }) = published else { + panic!("an unbounded number choice must publish a number contract, got {published:?}"); + }; + assert_eq!(min, 0, "the card states \"0 or greater\""); + assert_eq!( + max, + i32::MAX as u32, + "the maximum is the engine's own quantity domain, which is what a client \ + must be told rather than hard-code" + ); + + // The client reads JSON. Both bounds must be present there without decoding + // the choice type — that decoding is exactly what this contract replaces. + let json = serde_json::to_value(&projected.waiting_for).expect("prompt must serialize"); + let entry = json + .pointer("/data/free_entry") + .expect("the serialized prompt must carry free_entry"); + assert_eq!( + entry["kind"], "Number", + "the contract states its kind: {entry}" + ); + assert_eq!(entry["min"], 0, "{entry}"); + assert_eq!(entry["max"], i32::MAX, "{entry}"); +} + +/// (3): the published maximum is the enforced maximum. A client that trusts the +/// contract can neither be surprised by a rejection nor allow an acceptance the +/// engine refuses. +#[test] +fn the_published_bounds_are_the_bounds_the_engine_enforces() { + let runner = stop_at_number_prompt(); + let (choice_type, published) = prompt_parts(runner.state()); + let Some(FreeEntry::Number { min, max }) = published else { + panic!("expected a number contract, got {published:?}"); + }; + + for (answer, expected, why) in [ + (min.to_string(), true, "the published minimum is legal"), + (max.to_string(), true, "the published maximum is legal"), + ( + u64::from(max + 1).to_string(), + false, + "one past the published maximum is not", + ), + ( + "-1".to_string(), + false, + "a negative is below the published minimum", + ), + ] { + assert_eq!( + choice_type.accepts_free_entry_answer(&answer), + Some(expected), + "{why} (answer {answer})" + ); + } + + // And the interactive handler agrees with the contract it published: a value + // far past the old invented ceiling, well inside the published range, is + // taken. This is the assertion a re-introduced UI/engine split would fail. + let mut runner = runner; + runner + .act(GameAction::ChooseOption { + choice: "1000000".to_string(), + }) + .expect("a value within the published range must be accepted"); +} + +/// A choice whose answers ARE enumerable publishes no contract — the option list +/// is the domain. Without this the first test could pass on a prompt that +/// published a contract unconditionally. +#[test] +fn an_enumerated_choice_publishes_no_entry_contract() { + let bounded = ChoiceType::NumberRange { + min: 0, + max: Some(20), + distinctness: engine::types::ability::NumberDistinctness::Repeatable, + }; + assert_eq!( + bounded.free_entry(), + None, + "a stated maximum means the options enumerate the domain" + ); + assert_eq!( + bounded.accepts_free_entry_answer("5"), + None, + "and membership, not a range, validates the answer" + ); + assert_eq!( + ChoiceType::creature_type().free_entry(), + None, + "non-numeric enumerated choices likewise publish nothing" + ); +} diff --git a/crates/engine/tests/integration/serras_emissary_chosen_card_type_protection.rs b/crates/engine/tests/integration/serras_emissary_chosen_card_type_protection.rs index 7cb0e40c0c..be07c825c9 100644 --- a/crates/engine/tests/integration/serras_emissary_chosen_card_type_protection.rs +++ b/crates/engine/tests/integration/serras_emissary_chosen_card_type_protection.rs @@ -80,6 +80,7 @@ fn setup_emissary_choosing_creature(db: &CardDatabase) -> (GameState, ObjectId, // emissary via the production handler. let source = crate::support::exact_named_choice_source(runner.state(), emissary); runner.state_mut().waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: P0, choice_type: ChoiceType::card_type(), options: vec![ diff --git a/crates/engine/tests/integration/the_who_opponent_guess_resolution.rs b/crates/engine/tests/integration/the_who_opponent_guess_resolution.rs index f4560960ad..3cb00c14e8 100644 --- a/crates/engine/tests/integration/the_who_opponent_guess_resolution.rs +++ b/crates/engine/tests/integration/the_who_opponent_guess_resolution.rs @@ -65,13 +65,13 @@ fn drive_to_wait(runner: &mut GameRunner, want: &str) { /// Build The Toymaker's Trap as a 0/0 enchantment P0 controls, set up P0's /// upkeep, and advance until its trigger is on the stack. Returns the runner and /// the enchantment's id. -fn toymaker_at_upkeep(seeded_numbers: &[u8]) -> (GameRunner, ObjectId) { +fn toymaker_at_upkeep(seeded_numbers: &[u32]) -> (GameRunner, ObjectId) { toymaker_at_upkeep_with_player_count(2, seeded_numbers) } fn toymaker_at_upkeep_with_player_count( player_count: u8, - seeded_numbers: &[u8], + seeded_numbers: &[u32], ) -> (GameRunner, ObjectId) { let mut scenario = if player_count == 2 { GameScenario::new() @@ -343,7 +343,7 @@ fn toymakers_trap_exhausted_numbers_makes_no_guess() { .objects .get_mut(&trap) .unwrap() - .chosen_attributes = (1u8..=5).map(ChosenAttribute::Number).collect(); + .chosen_attributes = (1u32..=5).map(ChosenAttribute::Number).collect(); { let state = runner.state_mut(); state.turn_number = 2; diff --git a/crates/engine/tests/integration/wheel_of_misfortune_secret_numbers.rs b/crates/engine/tests/integration/wheel_of_misfortune_secret_numbers.rs new file mode 100644 index 0000000000..426fb75a55 --- /dev/null +++ b/crates/engine/tests/integration/wheel_of_misfortune_secret_numbers.rs @@ -0,0 +1,365 @@ +//! Wheel of Misfortune — *"Each player secretly chooses a number 0 or greater, +//! then all players reveal those numbers simultaneously and determine the +//! highest and lowest numbers revealed this way. Wheel of Misfortune deals +//! damage equal to the highest number to each player who chose that number. +//! Each player who didn't choose the lowest number discards their hand, then +//! draws seven cards."* +//! +//! The card is the flagship of the secret-simultaneous-number class (Menacing +//! Ogre, Life at Stake), and every clause of it keys on a CROSS-PLAYER extremum +//! of per-player choices. This test drives the real parse → cast → resolution +//! pipeline and pins the three behaviors that make the card what it is: +//! +//! 1. every player is prompted for a number (CR 101.4 APNAP order); +//! 2. the damage lands on the players who chose the HIGHEST number — all of +//! them when there is a tie — and on nobody else, for exactly that much; +//! 3. the wheel (discard hand, draw seven) hits every player who did NOT +//! choose the LOWEST number, and skips the one who did. +//! +//! The seating is chosen to discriminate: P0 and P1 both choose 4 (a tie for +//! highest), P2 chooses 1 (the unique lowest). A filter that collapsed to "all +//! players" would wheel P2 too; one that took only the first tied player would +//! spare P1 the damage; one that read a per-source chosen number instead of a +//! per-player one would deal 0. +//! +//! CR 101.4: when multiple players make choices at the same time, the active +//! player chooses first, then the remaining players in turn order. +//! CR 101.4b: a player normally knows the earlier choices — which is why the +//! card says "secretly", and why `game::visibility` keeps each player's +//! `ChosenAttribute::Number` private to that player. +//! CR 120.3a: damage dealt to a player by a source without infect causes that +//! player to lose that much life. +//! CR 121.1: a player draws a card by putting the top card of their library +//! into their hand. +//! CR 608.2c: the controller follows the spell's instructions in written order. +//! CR 608.2d: a choice offered by a resolving spell is announced while applying +//! the effect. +//! CR 701.9a: to discard a card, move it from its owner's hand to that player's +//! graveyard. + +use engine::game::scenario::GameScenario; +use engine::types::ability::ChosenAttribute; +use engine::types::actions::GameAction; +use engine::types::game_state::{CastPaymentMode, WaitingFor}; +use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; + +const P0: PlayerId = PlayerId(0); +const P1: PlayerId = PlayerId(1); +const P2: PlayerId = PlayerId(2); + +/// Verbatim Oracle text (Scryfall). A paraphrase can take a different parser +/// branch and go green while the real card stays broken. +const WHEEL_OF_MISFORTUNE: &str = "Each player secretly chooses a number 0 or greater, then all players reveal those numbers simultaneously and determine the highest and lowest numbers revealed this way. Wheel of Misfortune deals damage equal to the highest number to each player who chose that number. Each player who didn't choose the lowest number discards their hand, then draws seven cards."; + +/// The number each seat secretly chooses, in APNAP order. P0/P1 tie for the +/// highest; P2 is the unique lowest. +const CHOICES: [(PlayerId, &str); 3] = [(P0, "4"), (P1, "4"), (P2, "1")]; + +fn hand_size(state: &engine::types::game_state::GameState, player: PlayerId) -> usize { + state.players[player.0 as usize].hand.len() +} + +fn life(state: &engine::types::game_state::GameState, player: PlayerId) -> i32 { + state.players[player.0 as usize].life +} + +#[test] +fn wheel_of_misfortune_burns_the_highest_choosers_and_wheels_everyone_but_the_lowest() { + let mut scenario = GameScenario::new_n_player(3, 42); + scenario.at_phase(Phase::PreCombatMain); + + // Seven-card libraries so every wheeled player can actually draw seven + // (CR 121.1), and a distinguishable starting hand per seat. + for player in [P0, P1, P2] { + scenario.with_library_top( + player, + &[ + "Lib 1", "Lib 2", "Lib 3", "Lib 4", "Lib 5", "Lib 6", "Lib 7", "Lib 8", + ], + ); + scenario.with_cards_in_hand(player, &["Hand A", "Hand B"]); + } + + let mut spell_builder = scenario.add_spell_to_hand_from_oracle( + P0, + "Wheel of Misfortune", + false, + WHEEL_OF_MISFORTUNE, + ); + spell_builder.with_mana_cost(ManaCost::Cost { + generic: 2, + shards: vec![ManaCostShard::Red], + }); + let spell = spell_builder.id(); + scenario.with_mana_pool( + P0, + vec![ + ManaUnit::new(ManaType::Red, spell, false, vec![]), + ManaUnit::new(ManaType::Red, spell, false, vec![]), + ManaUnit::new(ManaType::Red, spell, false, vec![]), + ], + ); + + let mut runner = scenario.build(); + // Staging sanity check: two cards each, plus the spell itself in P0's hand + // (it leaves for the stack when the cast commits, CR 601.2a). Every seat + // starts with a NON-SEVEN hand, so the post-resolution 7 / 7 / 2 below can + // only come from the wheel actually firing on P0 and P1 and not on P2. + let hands_before: Vec = [P0, P1, P2] + .iter() + .map(|p| hand_size(runner.state(), *p)) + .collect(); + assert_eq!(hands_before, vec![3, 2, 2], "staged hands"); + + let card_id = runner.state().objects[&spell].card_id; + runner + .act(GameAction::CastSpell { + object_id: spell, + card_id, + targets: vec![], + payment_mode: CastPaymentMode::Auto, + }) + .expect("casting Wheel of Misfortune must start"); + + // Drive the resolution, answering each number prompt with that seat's + // scripted choice and recording who was asked, in order. + let mut number_choosers: Vec = Vec::new(); + for _ in 0..256 { + match runner.state().waiting_for.clone() { + WaitingFor::ManaPayment { .. } => { + runner + .act(GameAction::PassPriority) + .expect("mana payment must auto-finalize"); + } + WaitingFor::NamedChoice { + player, + options, + choice_type, + .. + } => { + let (_, choice) = CHOICES + .iter() + .find(|(seat, _)| *seat == player) + .unwrap_or_else(|| panic!("unexpected chooser {player:?}")); + // CR 107.1a/b: "a number 0 or greater" states no maximum, so the + // prompt enumerates NOTHING and the value is supplied by the + // player. An option list here would mean the engine had invented + // a ceiling — the bug that made 21 illegal. + assert!( + options.is_empty(), + "an unbounded number choice must not enumerate options; got {options:?}" + ); + assert!( + choice_type.options_supplied_by_player(), + "the prompt must route to the free-entry path" + ); + assert_eq!( + choice_type.accepts_free_entry_answer(choice), + Some(true), + "{choice} must be a legal answer for {player:?}" + ); + number_choosers.push(player); + // CR 101.4b: BEFORE the reveal, a chooser must not be able to + // read the answers already given. Checked at the moment the + // second and third seats are prompted — the exact window the + // card's "secretly" wording exists to close. + for (earlier, _) in CHOICES.iter().take(number_choosers.len() - 1) { + let view = + engine::game::visibility::filter_state_for_viewer(runner.state(), player); + assert!( + !view.players[earlier.0 as usize] + .chosen_attributes + .iter() + .any(|a| matches!(a, ChosenAttribute::Number(_))), + "{player:?} must not see {earlier:?}'s number before the reveal" + ); + } + runner + .act(GameAction::ChooseOption { + choice: (*choice).to_string(), + }) + .expect("answering the number choice must succeed"); + } + WaitingFor::Priority { .. } => { + if runner.act(GameAction::PassPriority).is_err() { + break; + } + if runner.state().stack.is_empty() && number_choosers.len() == CHOICES.len() { + break; + } + } + other => panic!("unexpected prompt during resolution: {other:?}"), + } + } + + // CR 101.4 + CR 608.2c: AFTER the reveal instruction resolved, every number + // is public to every player. This is the other half of the privacy contract + // — without it the engine would keep the numbers secret past the instruction + // that publishes them, which is what a bare `Effect::NoOp` reveal did. + for viewer in [P0, P1, P2] { + let view = engine::game::visibility::filter_state_for_viewer(runner.state(), viewer); + for (seat, chosen) in CHOICES { + let expected: u32 = chosen.parse().expect("scripted choice is numeric"); + assert!( + view.players[seat.0 as usize] + .chosen_attributes + .contains(&ChosenAttribute::RevealedNumber(expected)), + "{viewer:?} must see {seat:?}'s revealed number {expected} after the reveal" + ); + } + } + + // CR 101.4: the active player chooses first, then the rest in turn order. + assert_eq!( + number_choosers, + vec![P0, P1, P2], + "every player must secretly choose a number, in APNAP order" + ); + + let state = runner.state(); + + // CR 120.3a: the highest number revealed is 4, and BOTH players who chose it + // take exactly that much. P2 chose 1, which is not the highest, so P2 takes + // none — a filter that widened to "each player" would show -4 here too. + assert_eq!(life(state, P0), 16, "P0 tied for the highest number (4)"); + assert_eq!(life(state, P1), 16, "P1 tied for the highest number (4)"); + assert_eq!(life(state, P2), 20, "P2 did not choose the highest number"); + + // CR 701.9a + CR 121.1: everyone who did NOT choose the lowest number (1) + // discards their hand and draws seven. P2 chose the lowest and is skipped + // entirely — hand untouched, no draw. + assert_eq!( + hand_size(state, P0), + 7, + "P0 didn't choose the lowest number, so it wheels to a fresh seven" + ); + assert_eq!( + hand_size(state, P1), + 7, + "P1 didn't choose the lowest number, so it wheels to a fresh seven" + ); + assert_eq!( + hand_size(state, P2), + 2, + "P2 chose the lowest number and keeps its hand — no discard, no draw" + ); +} + +/// CR 107.1a/b: "a number 0 or greater" states NO maximum, so a number past any +/// ceiling the engine might have invented must be both choosable and effective. +/// +/// This is the case the three-seat test above structurally cannot detect: it only +/// ever chooses 1 and 4, both inside the range the engine used to invent +/// (`min: 0, max: 20`), so it stayed green while 21 was rejected outright. Here +/// P1 bids exactly 21 and P0 bids 40 — past both the old ceiling and a starting +/// life total — and every assertion below is reachable only if those values were +/// accepted at the answer seam, stored at full width, folded as the cross-player +/// maximum, and dealt as damage. +#[test] +fn a_number_past_the_old_ceiling_is_choosable_and_deals_that_much_damage() { + let mut scenario = GameScenario::new_n_player(3, 42); + scenario.at_phase(Phase::PreCombatMain); + for player in [P0, P1, P2] { + scenario.with_library_top( + player, + &[ + "Lib 1", "Lib 2", "Lib 3", "Lib 4", "Lib 5", "Lib 6", "Lib 7", "Lib 8", + ], + ); + scenario.with_cards_in_hand(player, &["Hand A", "Hand B"]); + // High enough that a 40-point hit is survivable, so the assertion reads a + // life total rather than an elimination. + scenario.with_life(player, 60); + } + + let mut spell_builder = scenario.add_spell_to_hand_from_oracle( + P0, + "Wheel of Misfortune", + false, + WHEEL_OF_MISFORTUNE, + ); + spell_builder.with_mana_cost(ManaCost::Cost { + generic: 2, + shards: vec![ManaCostShard::Red], + }); + let spell = spell_builder.id(); + scenario.with_mana_pool( + P0, + vec![ + ManaUnit::new(ManaType::Red, spell, false, vec![]), + ManaUnit::new(ManaType::Red, spell, false, vec![]), + ManaUnit::new(ManaType::Red, spell, false, vec![]), + ], + ); + + let mut runner = scenario.build(); + let card_id = runner.state().objects[&spell].card_id; + runner + .act(GameAction::CastSpell { + object_id: spell, + card_id, + targets: vec![], + payment_mode: CastPaymentMode::Auto, + }) + .expect("casting Wheel of Misfortune must start"); + + let bids: [(PlayerId, &str); 3] = [(P0, "40"), (P1, "21"), (P2, "0")]; + let mut answered = 0usize; + for _ in 0..256 { + match runner.state().waiting_for.clone() { + WaitingFor::ManaPayment { .. } => { + runner + .act(GameAction::PassPriority) + .expect("mana payment must auto-finalize"); + } + WaitingFor::NamedChoice { player, .. } => { + let (_, bid) = bids + .iter() + .find(|(seat, _)| *seat == player) + .unwrap_or_else(|| panic!("unexpected chooser {player:?}")); + runner + .act(GameAction::ChooseOption { + choice: (*bid).to_string(), + }) + .unwrap_or_else(|e| { + panic!("{bid} must be a legal answer for {player:?}: {e:?}") + }); + answered += 1; + } + WaitingFor::Priority { .. } => { + if runner.act(GameAction::PassPriority).is_err() { + break; + } + if runner.state().stack.is_empty() && answered == bids.len() { + break; + } + } + other => panic!("unexpected prompt during resolution: {other:?}"), + } + } + assert_eq!(answered, 3, "all three bids must have been accepted"); + + let state = runner.state(); + // CR 120.3a: 40 is the highest bid, so P0 — and only P0 — takes exactly that + // much. Under the invented ceiling this assertion could not even be reached: + // the answer seam rejected the bid. + assert_eq!(life(state, P0), 20, "P0 bid 40 and takes exactly that much"); + assert_eq!(life(state, P1), 60, "P1 did not bid the highest"); + assert_eq!(life(state, P2), 60, "P2 did not bid the highest"); + + // CR 701.9a + CR 121.1: P2 bid the lowest (0) and is spared; the other two + // wheel — including the 21 bid the old range also rejected. + assert_eq!(hand_size(state, P0), 7, "P0 wheels"); + assert_eq!( + hand_size(state, P1), + 7, + "P1 bid 21 — past the old ceiling — and wheels" + ); + assert_eq!( + hand_size(state, P2), + 2, + "P2 bid the lowest and keeps its hand" + ); +} diff --git a/crates/mtgish-import/src/convert/action.rs b/crates/mtgish-import/src/convert/action.rs index 45568d5949..8b587fddb5 100644 --- a/crates/mtgish-import/src/convert/action.rs +++ b/crates/mtgish-import/src/convert/action.rs @@ -4431,10 +4431,10 @@ pub fn convert(a: &Action) -> ConvResult { // are out of range or inverted (defensive — the engine would generate // a degenerate option list). Action::ChooseANumberBetween(min, max) => { - let (Ok(min_u8), Ok(max_u8)) = (u8::try_from(*min), u8::try_from(*max)) else { + let (Ok(min_u8), Ok(max_u8)) = (u32::try_from(*min), u32::try_from(*max)) else { return Err(ConversionGap::EnginePrerequisiteMissing { engine_type: "ChoiceType::NumberRange", - needed_variant: format!("number-range bounds out of u8 ({min}, {max})"), + needed_variant: format!("number-range bounds out of u32 ({min}, {max})"), }); }; if min_u8 > max_u8 { @@ -4446,7 +4446,10 @@ pub fn convert(a: &Action) -> ConvResult { Effect::Choose { choice_type: ChoiceType::NumberRange { min: min_u8, - max: max_u8, + // CR 107.1a: "between X and Y" states an upper bound, so this + // converts to the BOUNDED form. The unbounded engine shape is + // reserved for text that states no maximum. + max: Some(max_u8), distinctness: engine::types::ability::NumberDistinctness::Repeatable, }, persist: true, diff --git a/crates/mtgish-import/src/convert/replacement.rs b/crates/mtgish-import/src/convert/replacement.rs index 88eca1baf6..037468cda6 100644 --- a/crates/mtgish-import/src/convert/replacement.rs +++ b/crates/mtgish-import/src/convert/replacement.rs @@ -1895,10 +1895,10 @@ fn build_replacement_exec( // values are out of range or inverted (defensive — the engine // would generate a degenerate option list). A::ChooseANumberBetween(min, max) => { - let (Ok(min_u8), Ok(max_u8)) = (u8::try_from(*min), u8::try_from(*max)) else { + let (Ok(min_u8), Ok(max_u8)) = (u32::try_from(*min), u32::try_from(*max)) else { return Err(ConversionGap::EnginePrerequisiteMissing { engine_type: "ChoiceType::NumberRange", - needed_variant: format!("number-range bounds out of u8 ({min}, {max})"), + needed_variant: format!("number-range bounds out of u32 ({min}, {max})"), }); }; if min_u8 > max_u8 { @@ -1910,7 +1910,9 @@ fn build_replacement_exec( Effect::Choose { choice_type: ChoiceType::NumberRange { min: min_u8, - max: max_u8, + // CR 107.1a: "between X and Y" states an upper bound, so this + // converts to the BOUNDED form. + max: Some(max_u8), distinctness: engine::types::ability::NumberDistinctness::Repeatable, }, persist: true, diff --git a/crates/phase-ai/src/bin/ai_commander.rs b/crates/phase-ai/src/bin/ai_commander.rs index 87aebca92b..df989f9821 100644 --- a/crates/phase-ai/src/bin/ai_commander.rs +++ b/crates/phase-ai/src/bin/ai_commander.rs @@ -1191,6 +1191,7 @@ mod tests { ); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(1), choice_type: ChoiceType::CardName, options: Vec::new(), diff --git a/crates/phase-ai/src/policies/effect_classify.rs b/crates/phase-ai/src/policies/effect_classify.rs index dee30eec3d..8099fc9643 100644 --- a/crates/phase-ai/src/policies/effect_classify.rs +++ b/crates/phase-ai/src/policies/effect_classify.rs @@ -358,6 +358,10 @@ pub(crate) fn effect_polarity(effect: &Effect) -> EffectPolarity { | Effect::Renown { .. } | Effect::ReturnAsAura { .. } | Effect::Reveal { .. } + // CR 101.4: publishing already-chosen numbers moves no card and changes + // no board state, so it is neither good nor bad on its own — the damage + // and wheel clauses that READ those numbers carry the polarity. + | Effect::RevealChosenNumbers { .. } | Effect::RevealFromHand { .. } | Effect::RevealHand { .. } | Effect::RevealTop { .. } diff --git a/crates/phase-ai/src/policies/redundancy_avoidance.rs b/crates/phase-ai/src/policies/redundancy_avoidance.rs index 309fb9add2..adbc9b6492 100644 --- a/crates/phase-ai/src/policies/redundancy_avoidance.rs +++ b/crates/phase-ai/src/policies/redundancy_avoidance.rs @@ -590,6 +590,10 @@ fn redundancy_delta( | Effect::Cascade | Effect::Ripple { .. } | Effect::Reveal { .. } + // CR 101.4: no targets and nothing to deduplicate — publishing a + // committed number is idempotent, so a second application is harmless + // rather than redundant in the sense this policy detects. + | Effect::RevealChosenNumbers { .. } // CR 702.xxx: Prepare (Strixhaven) — no redundancy detection. | Effect::BecomePrepared { .. } | Effect::BecomeUnprepared { .. } diff --git a/crates/phase-ai/src/search.rs b/crates/phase-ai/src/search.rs index 7de95de3ab..fcc3ae168e 100644 --- a/crates/phase-ai/src/search.rs +++ b/crates/phase-ai/src/search.rs @@ -517,6 +517,7 @@ fn random_card_predicate_guess( use rand::seq::IndexedRandom; let WaitingFor::NamedChoice { + free_entry: _, player, choice_type, options, @@ -9322,6 +9323,7 @@ mod tests { Zone::Battlefield, ); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(1), choice_type: ChoiceType::CardPredicateGuess { options: ChoiceType::land_or_nonland_card_predicate_options(), @@ -9421,6 +9423,7 @@ mod tests { Zone::Battlefield, ); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(1), choice_type: ChoiceType::CardPredicate { options: ChoiceType::land_or_nonland_card_predicate_options(), @@ -9458,6 +9461,7 @@ mod tests { ); state.all_card_names = vec!["Forest".to_string(), "Island".to_string()].into(); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: ChoiceType::CardName, options: Vec::new(), @@ -9477,6 +9481,7 @@ mod tests { let mut state = make_state(); state.all_card_names = vec!["Forest".to_string()].into(); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: P0, choice_type: ChoiceType::CardName, options: Vec::new(), @@ -9509,6 +9514,7 @@ mod tests { let mut state = make_state(); state.all_card_names = Vec::new().into(); state.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(0), choice_type: ChoiceType::CardName, options: Vec::new(), @@ -12226,6 +12232,7 @@ mod tests { Zone::Battlefield, ); guess.waiting_for = WaitingFor::NamedChoice { + free_entry: None, player: PlayerId(1), choice_type: ChoiceType::CardPredicateGuess { options: ChoiceType::land_or_nonland_card_predicate_options(),