sim: only GameType.Commander seats a commander — Brawl, Oathbreaker, TinyLeaders and the Vanguard variants silently run misconfigured
- Dominant language
- Java
- Stars
- 2.7k
- Forks
- 1.1k
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 158
Description
## 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`](https://github.com/Card-Forge/forge/blob/master/forge-gui-desktop/src/main/java/forge/view/SimulateMatch.java#L146-L152):
```java
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`](https://github.com/Card-Forge/forge/blob/master/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:
```java
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](https://github.com/Card-Forge/forge/blob/master/forge-gui-desktop/src/main/java/forge/view/SimulateMatch.java#L104-L107)
and returns, so this branch is never reached there; `simulateTournament` registers `TournamentPlayer`s
through a separate path.
2. **`MomirBasic` and `MoJhoSto` need their deck generated first.** They are the only two `GameType`s
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:
```java
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
Contributor guide
Research direction
Start in forge-gui-desktop/src/main/java/forge/view/SimulateMatch.java, especially simulate() and the RegisteredPlayer construction around lines 146-152. Compare the existing Commander path with RegisteredPlayer.forVariants and inspect the separate tournament and auto-generated-deck paths for MomirBasic and MoJhoSto. Done means the supported simulation formats seat the expected commander or avatar and use the documented starting configuration, with the Brawl reproduction no longer showing 20 life or missing commanders.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, game-dev
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100