Skip to content

Recruit: add Effect - #11355

Merged
Hanmac merged 3 commits into
masterfrom
recruit
Jul 24, 2026
Merged

Recruit: add Effect#11355
Hanmac merged 3 commits into
masterfrom
recruit

Conversation

@Hanmac

@Hanmac Hanmac commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Closes #11280

Api Effect
No new cards yet
no extra Ai yet

Comment thread forge-game/src/main/java/forge/game/ability/effects/RecruitEffect.java Outdated
Comment thread forge-game/src/main/java/forge/game/ability/effects/RecruitEffect.java Outdated
@Agetian

Agetian commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Connive AI is likely reusable for this mechanic?

Hanmac added 2 commits July 24, 2026 09:12
Fix makeTokenTableInternal
fix unused import
@Hanmac
Hanmac marked this pull request as ready for review July 24, 2026 07:19
@Hanmac

Hanmac commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Connive AI is likely reusable for this mechanic?

Probably

@Hanmac
Hanmac merged commit 28431f2 into master Jul 24, 2026
8 checks passed
@Hanmac
Hanmac deleted the recruit branch July 24, 2026 10:04
Tyrathalis added a commit to Tyrathalis/forge that referenced this pull request Jul 27, 2026
…-Forge#11355 regression)

SpellAbilityEffect.discard() writes the post-move collection back into the
map it is handed (discardedMap.put at :906) so its DiscardedAll loop can
read it. Upstream 28431f2 ("Recruit: add Effect", Card-Forge#11355, 2026-07-24)
changed ConniveEffect to pass an immutable Map.of(...) and gave the new
RecruitEffect the same shape, so every connive resolution throws
UnsupportedOperationException. Our D4 rebase inherited it.

Measured cost before the fix: 28 UnsupportedOperationException crashes in a
4,000-game arms read (0.7%), against 4 in all pre-rebase history, and
concentrated in the five pool decks carrying connive cards (Illuminator
Virtuoso, Ledger Shredder, Lethal Scheme) — a per-matchup bias, not uniform
noise. That read is discarded.

ConniveDiscardMapTest pins it, validated failing first (UOE from
ConniveEffect:99 via SpellAbilityEffect:906; after: draw one discard one,
hand 1 -> 1, graveyard 1). Suite 292 green.

FORK-LOCAL AND DELIBERATELY NOT UPSTREAMED: Hanmac has taken responsibility
for the regression and is fixing it. No caller of discard() reads the map
back, so the maintainer fix (likely: make discard() keep its own local map,
which restores Map.of() at the call sites) is behaviorally identical to this
one — there is no divergence for the two engines to disagree along. Expect a
conflict on these lines at the next rebase and DROP OURS in favour of
upstream, the way the Card-Forge#11203 copier fixes came home.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tyrathalis added a commit to Tyrathalis/forge that referenced this pull request Aug 12, 2026
…-Forge#11355 regression)

SpellAbilityEffect.discard() writes the post-move collection back into the
map it is handed (discardedMap.put at :906) so its DiscardedAll loop can
read it. Upstream 28431f2 ("Recruit: add Effect", Card-Forge#11355, 2026-07-24)
changed ConniveEffect to pass an immutable Map.of(...) and gave the new
RecruitEffect the same shape, so every connive resolution throws
UnsupportedOperationException. Our D4 rebase inherited it.

Measured cost before the fix: 28 UnsupportedOperationException crashes in a
4,000-game arms read (0.7%), against 4 in all pre-rebase history, and
concentrated in the five pool decks carrying connive cards (Illuminator
Virtuoso, Ledger Shredder, Lethal Scheme) — a per-matchup bias, not uniform
noise. That read is discarded.

ConniveDiscardMapTest pins it, validated failing first (UOE from
ConniveEffect:99 via SpellAbilityEffect:906; after: draw one discard one,
hand 1 -> 1, graveyard 1). Suite 292 green.

FORK-LOCAL AND DELIBERATELY NOT UPSTREAMED: Hanmac has taken responsibility
for the regression and is fixing it. No caller of discard() reads the map
back, so the maintainer fix (likely: make discard() keep its own local map,
which restores Map.of() at the call sites) is behaviorally identical to this
one — there is no divergence for the two engines to disagree along. Expect a
conflict on these lines at the next rebase and DROP OURS in favour of
upstream, the way the Card-Forge#11203 copier fixes came home.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tyrathalis added a commit to Tyrathalis/forge that referenced this pull request Aug 21, 2026
…-Forge#11355 regression)

SpellAbilityEffect.discard() writes the post-move collection back into the
map it is handed (discardedMap.put at :906) so its DiscardedAll loop can
read it. Upstream 28431f2 ("Recruit: add Effect", Card-Forge#11355, 2026-07-24)
changed ConniveEffect to pass an immutable Map.of(...) and gave the new
RecruitEffect the same shape, so every connive resolution throws
UnsupportedOperationException. Our D4 rebase inherited it.

Measured cost before the fix: 28 UnsupportedOperationException crashes in a
4,000-game arms read (0.7%), against 4 in all pre-rebase history, and
concentrated in the five pool decks carrying connive cards (Illuminator
Virtuoso, Ledger Shredder, Lethal Scheme) — a per-matchup bias, not uniform
noise. That read is discarded.

ConniveDiscardMapTest pins it, validated failing first (UOE from
ConniveEffect:99 via SpellAbilityEffect:906; after: draw one discard one,
hand 1 -> 1, graveyard 1). Suite 292 green.

FORK-LOCAL AND DELIBERATELY NOT UPSTREAMED: Hanmac has taken responsibility
for the regression and is fixing it. No caller of discard() reads the map
back, so the maintainer fix (likely: make discard() keep its own local map,
which restores Map.of() at the call sites) is behaviorally identical to this
one — there is no divergence for the two engines to disagree along. Expect a
conflict on these lines at the next rebase and DROP OURS in favour of
upstream, the way the Card-Forge#11203 copier fixes came home.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hobbit: recruit

3 participants