sim: only GameType.Commander seats a commander — Brawl, Oathbreaker, TinyLeaders and the Vanguard variants silently run misconfigured

[johannesamee]

Summary

SimulateMatch special-cases only GameType.Commander when building each RegisteredPlayer. Every other variant format documented for sim -f therefore falls to the plain constructor, so no commander/avatar is seated and starting life stays at 20. The run completes and prints plausible results — it is just simulating a different format from the one requested.

Verified against Forge 2.0.14, and the code path is unchanged on master today.

The code

forge-gui-desktop/src/main/java/forge/view/SimulateMatch.java#L146-L152:

RegisteredPlayer rp;

if (type.equals(GameType.Commander)) {
    rp = RegisteredPlayer.forCommander(d);
} else {
    rp = new RegisteredPlayer(d);
}

RegisteredPlayer.forVariants(...) already models all of these correctly — Brawl seats commanders and adds +5 life at two players, TinyLeaders adds +5, Oathbreaker seats commanders, and Vanguard/MomirBasic/MoJhoSto set the avatar. SimulateMatch just never calls it.

Impact

docs/AI.md documents seven values for -f. Six of them route through the else branch:

-f value	Expected	What sim actually does
Commander	commanders, 40 life	correct — the one handled case
Brawl	commanders, 25 life (2p)	no commander, 20 life
Oathbreaker	oathbreaker + signature spell	neither seated
TinyLeaders	commander, 25 life	no commander, 20 life
Vanguard	avatar assigned	no avatar
MomirBasic	avatar assigned	no avatar
MoJhoSto	avatar assigned	no avatar

The failure is silent rather than a refusal, which is what makes it worth reporting: someone reads AI.md, runs sim -f brawl, and gets numbers describing a commander-less 20-life format.

Reproduction

java -Xmx4096m -jar forge-gui-desktop-2.0.14-jar-with-dependencies.jar sim \
     -d deckA.dck deckB.dck -f brawl -n 3 -q -s 42

(Note: -Djava.awt.headless=true makes the process exit 1 with no output — unrelated, but it bites when scripting this.)

Measured across 8 games at 8 distinct seeds, with two Standard Brawl decks whose commanders are Thranduil, the Elvenking and Bifur, Melodic Rider:

• both commanders appear 0 times in the game logs
• every Life: line starts from 20, not 25
• the same two decks under -f commander cast their commanders normally (16 log hits), so the machinery works — the Brawl branch just doesn't reach it

Please treat the Brawl row as measured and the other five as read from source: I hit this building a Standard Brawl harness and only instrumented that format. The shared else branch is the same in each case, but I would not want the table read as five more empirical results.

Suggested direction

(Twice corrected — see the follow-up comments. I have narrowed this to what I can actually defend from
outside the project; the bug report above is unchanged and unaffected.)

For the three commander-style formats, routing the else branch through forVariants looks right, since it
already models each of them. The player count in this path is just the number of decks:

final int playerCount = params.containsKey("d") ? params.get("d").size() : 2;
...
RegisteredPlayer rp;
if (type.equals(GameType.Commander)) {
    rp = RegisteredPlayer.forCommander(d);
} else {
    rp = RegisteredPlayer.forVariants(playerCount, EnumSet.of(type), d, null, false, null, null);
}

That covers Brawl, Oathbreaker and TinyLeaders, in non-tournament mode only. Three things it does not
cover, all of which I got wrong in earlier drafts and would rather state than paper over:

1. Tournament mode. simulate() dispatches -t at
   L104-107
   and returns, so this branch is never reached there; simulateTournament registers TournamentPlayers
   through a separate path.
2. MomirBasic and MoJhoSto need their deck generated first. They are the only two GameTypes
   carrying a deckAutoGenerator, and it replaces the supplied deck with 60 basics plus the avatar(s).
   forVariants only reads DeckSection.Avatar off the deck it is handed, so passing an ordinary -d deck
   would either miss that section or simulate the wrong deck. These want
   type.hasDeckAutoGenerator() / type.autoGenerateDeck(rp), which is a different change from the above.
3. Planechase and Archenemy take planes/schemes that sim has no flags for.

Vanguard I have not checked — it has no auto-generator, so it may well be covered by the snippet, but I am
not going to claim a fourth format I have not looked at.

The measurement in this report came from a ~50-line external driver compiled against the shipped fat jar,
not from the snippet above:

RegisteredPlayer rp = RegisteredPlayer.forVariants(
        2, EnumSet.of(GameType.Brawl), d, null, false, null, null);

Ai(1)-deckA commander=[Thranduil, the Elvenking] startingLife=25
Ai(2)-deckB commander=[Bifur, Melodic Rider]     startingLife=25

I am happy to open a PR for the non-tournament commander-style half, but given I have now corrected this
section twice from the outside, someone who can build and run the tournament and Momir paths should
probably own the rest.

Environment

• Forge 2.0.14 (measurement) / master (source check)
• macOS 27.0 arm64, OpenJDK 26.0.2

[johannesamee]

Correcting my own suggested patch — two defects in it, both my fault. The bug report above is unaffected; this is only about the fix I proposed. I have edited the original comment and am leaving this note so the change is visible rather than silent.

1. It didn't compile. params is a Map<String, List<String>>, so Integer.parseInt(params.get("p")) is wrong — every other scalar read in the file uses .get(0) (L75, L81, L88, L94, L160, L247, L249, L270). The genuinely list-valued options, -a and -d, are read as lists at L118/L130/L255.

2. Worse, -p cannot reach that branch at all. simulate() dispatches tournament runs at L104-107 and returns, so the RegisteredPlayer construction at L146 is only ever reached in non-tournament mode — and -p is read only inside simulateTournament at L249. I had plumbed a parameter into the one path where it is never set, which would have silently pinned the count to 2 in every run.

The player count in the non-tournament path is just the number of decks:

final int playerCount = params.containsKey("d") ? params.get("d").size() : 2;
...
RegisteredPlayer rp;
if (type.equals(GameType.Commander)) {
    rp = RegisteredPlayer.forCommander(d);
} else {
    rp = RegisteredPlayer.forVariants(playerCount, EnumSet.of(type), d, null, false, null, null);
}

Scope, corrected: this fixes non-tournament sim only. Tournament mode builds TournamentPlayers through a separate path I have not traced, so -t runs stay misconfigured for the six affected formats even with the above applied. I did not know that when I wrote "a one-line fix covers all six" — it covers all six formats, but only outside -t.

Apologies for the noise. The measured Brawl evidence in the report stands unchanged; it was taken with an external driver, not with this patch.

[johannesamee]

Second correction to my suggested fix, and I'm narrowing it rather than patching it a third time. The bug report itself is unchanged — six of seven -f values misconfigured, Brawl measured, the rest read from source.

MomirBasic and MoJhoSto are not fixed by routing through forVariants. They are the only two GameTypes with a deckAutoGenerator, and it replaces the supplied deck with 60 basics plus the avatar(s). forVariants only reads DeckSection.Avatar off whatever deck it is handed, so an ordinary -d deck would miss that section or simply simulate the wrong deck. Those two want type.hasDeckAutoGenerator() / type.autoGenerateDeck(rp) — a different change.

So the defensible scope of my snippet is Brawl, Oathbreaker and TinyLeaders, non-tournament only — three of the six, not six. Vanguard has no auto-generator and may be covered, but I haven't checked it and won't claim it.

I've edited the issue body to say exactly that. Having now corrected this section twice from outside the project, I'd suggest treating the report as the reliable part and the patch as a starting point only — someone who can build and run the tournament and Momir paths should own the rest.