# Upstream notes Defects and sharp edges found in MegaMek 0.51.0 while building this harness, with what we do about each. Line numbers are from the 0.51.0 source tree. Two reasons this file exists. A workaround with no recorded cause gets removed by the next person who thinks it looks superstitious. And several of these are worth reporting upstream, which needs the analysis written down somewhere other than a commit message. Nothing here is a criticism of MegaMek. Almost all of it only bites a headless, bot-only, many-matches-in-parallel workload, which is not what it was built for. --- ## 1. `AutosaveService` spins forever on `String.format` **Severity: fatal. Not headless-specific.** This is a real, unconditional bug. `server/AutosaveService.java:103-119`, `getAutosaveFilename()`: ```java String localDateTime = LocalDateTime.now().format(...); // computed once while (repeatedName) { fileName = String.format(FILENAME_FORMAT, gameManager.getGame().getCurrentRound(), // loop-invariant localDateTime); // loop-invariant repeatedName = false; for (final File file : autosaveFiles) { if (file.getName().compareToIgnoreCase(fileName) == 0) { repeatedName = true; // nothing ever changes break; } } } ``` Both format arguments are loop-invariant, so the generated name is identical on every pass. If a file of that name already exists, the loop never terminates. It runs on the **packet pump thread**, holding `serverLock` and `GAME_LOCK`, from `TWPhasePreparationManager` (case `INITIATIVE`) — *after* `AbstractGameManager.changePhase` has already set the phase. So the game freezes at `INITIATIVE` with every other thread healthy, the sockets fine, and no exception anywhere. Why it fires: `MMConstants.SAVEGAME_DIR` is `"savegames"` **relative to the process working directory**, so every match launched from the same directory shares one folder. The default stamp format has one-second resolution, and the default `max_rotating_round_saves = 3` leaves the two newest files in place. Two matches reaching their first initiative in the same wall-clock second collide. **What we do:** set `OptionsConstants.BASE_MAX_NUMBER_ROUND_SAVES` to `0` in `SdsHost`, which short-circuits `performRollingAutosave()` before the loop. It also removes a full XStream serialisation of the `Game` to gzipped XML from the packet pump once per round, which a bot-only benchmark has no use for. **Upstream fix** is two lines — hoist the `format` out of the loop and vary something per attempt, or bound the loop: ```java for (int attempt = 0; ; attempt++) { fileName = String.format(FILENAME_FORMAT, round, localDateTime + (attempt == 0 ? "" : "-" + attempt)); ... } ``` --- ## 2. Lost wakeup in `Server.PacketPump.run` **Severity: hangs a quiet game. Observed 2026-08-19.** `server/Server.java:151-163`. The `packetQueue.isEmpty()` check is outside the monitor; the producer at `:299-302` does `synchronized (packetQueue) { add; notifyAll; }`. An add landing between the check and the `wait()` at `:161` loses the notify, and the pump sleeps with a non-empty queue until the *next* packet. A GUI game always has more traffic. A headless bot-only game sitting in a report phase waiting on exactly two Done packets does not, so the hang is permanent. Confirmed on `2v2-clan-invasion-s101020`, which stalled at round 3. The dump: ``` Packet Pump WAITING at java.lang.Object.wait(Object.java:339) at megamek.server.Server$PacketPump.run(Server.java:161) Client Connection, Player North RUNNABLE (blocked reading the socket) Client Connection, Player South RUNNABLE ``` Both clients waiting on a server whose pump is asleep, which is the predicted signature. No frame of ours appears except the dump writer. Intermittent: the same scenario and seed ran to completion five times either side of it. Timing decides, so a rate cannot be read off one observation. **What we do:** record it and move on. `bridge/` compiles against the stock jar, and the only fix inside that constraint is to wake the pump from outside, which means reflecting into a private field. `SdsHost` has always taken `--stall-seconds`, but nothing passed it, so a hang cost the 300 s default. `MatchSpec.stall_seconds` and the CLI's `--stall-seconds` now reach it; the default is still 300, and a run that expects to meet this can lower it. Still recorded rather than retried: a harness that silently reruns stalls hides the bug that causes them. Not to be confused with the autosave hang (#1), which also parks the pump but does it by spinning inside `handle` rather than by sleeping in `wait`. The dump tells them apart: spinning shows `String.format`, this shows `Object.wait`. --- ## 3. A dead packet pump is silent `Server.PacketPump.run` (`server/Server.java:152-157`) has **no try/catch**, and `TWGameManager.handlePacket:731` calls the whole phase engine outside its own `try`, catching only `InvalidPacketDataException`. Any `RuntimeException` in the phase engine propagates out of `run()` and kills the pump; the server then queues packets forever and processes none. MegaMek installs a default uncaught-exception handler in `MegaMek.main` (`MegaMek.java:114`), which a program embedding `Server` never runs — so the failure is completely invisible and looks exactly like a phase that will not advance. **What we do:** `SdsHost` installs its own handler first thing in `main`, and dumps every thread's state on a stall. --- ## 4. Tactical Genius deadlocks a bot-only game `TWGameManager.checkReady:1456-1465`: on an initiative reroll request it calls `resetActivePlayersDone()`, re-rolls, and sends `SENDING_REPORTS_TACTICAL_GENIUS` — with no `PHASE_CHANGE`. `BotClient` only ever sends Done from `changePhase` (`BotClient.java:578-585`), and `Client.java:1109-1122` handles that packet by firing a `GameReportEvent`, which re-enables a human's Done button and does nothing in a bot. Both bots have just been set back to not-done, and nothing will ever set them done again. So a pilot with the `tactical_genius` SPA permanently deadlocks `INITIATIVE_REPORT` in a game with no humans. **What we do:** nothing. Our generated scenarios use plain regular pilots. If a scenario ever grants the SPA, this is what it will do. --- ## 5. `CLIENT_NAME` is handled off the packet pump `server/Server.java:266-274` dispatches `CLIENT_NAME` straight to `handle()` on the connection reader thread, with no lock. `receivePlayerName` → `sendCurrentInfo` → `TWGameManager.sendCurrentInfo:694-699` calls **`endCurrentPhase()`** when the phase does not use turns. A client connecting or reconnecting outside the lounge therefore advances the phase from a second thread, concurrently with the pump. **What we do:** attach all bots in the lounge and never reconnect one mid-match. --- ## 6. `WeaponType.getDamage()` returns a sentinel for cluster weapons **Not a bug — a trap.** `WeaponType.DAMAGE_BY_CLUSTER_TABLE = -2` and `DAMAGE_VARIABLE = -3`. Every missile weapon returns `-2`, so any consumer that treats `getDamage()` as a number gets *negative* damage for every LRM and SRM in the game. We shipped exactly that bug for a while: the bot was told its missiles did minus two damage and duly valued them below nothing. **What we do:** `Observation.java` uses `getBattleForceDamage(range, fcs) * 10`, which is MegaMek's own averaging and which the subclasses correct where they must (`SRMWeapon` overrides it with a factor of two). There is deliberately no plain `damage` field on the wire. --- ## 7. `Precognition` does not know two packet commands `client/bot/princess/Precognition.java` mirrors `Client`'s packet switch but is missing `UPDATE_CUT_HEXES` and `SYNC_TEMPORARY_ECM_FIELDS`, both of which the server sends once per round. Its `default:` arm logs at ERROR and breaks — no retry, no re-queue, no feedback loop. Purely log noise, and only on the Princess side, but it is several lines per round in every match log. **What we do:** nothing. Raising the log level for that class would hide real Precognition errors too. --- ## 8. `MMLogger` sharp edges - `parametrizedStringAnyway` (`logging/MMLogger.java:355-357`) falls back to `String.format` for any message that does not contain `{}`, so a message with a stray literal `%` throws `UnknownFormatConversionException` from inside a log call. - `warn(Throwable, ...)`, `error(Throwable, ...)` and `debug(Throwable, ...)` call `Sentry.captureException` **outside** the level check. **What we do:** `-Dsentry.dsn=` on every match JVM. --- ## 9. `SanityInputFilter` is expensive per deserialised class `common/net/marshalling/SanityInputFilter.java` runs ~40 regex matches for every class in an incoming object stream, plus a fallback doing `className.contains(pattern.toString())` — a substring search over the regex *source text*, which is not what that was meant to do. On a loopback socket with two bots this is measurable but not fatal. **What we do:** nothing. Noted in case packet handling ever shows up in a profile. --- ## 10. `VictoryHelper` freezes the victory conditions at construction **Severity: silent wrong results. Not headless-specific.** Any caller that sets a victory game option after the `Game` exists is affected. `server/victory/VictoryHelper.java`. `javap -p` on the 0.51.0 jar: ``` private final boolean checkForVictory; private final java.util.List victoryConditions; public VictoryHelper(megamek.common.game.Game); private void buildVCList(megamek.common.game.Game); ``` Both are `final` and both are filled from the game options in the constructor, by `buildVCList`. `BVDestroyedVictoryCondition` is built there from `VICTORY_BV_DESTROYED_PERCENT`. Nothing re-reads the option afterwards. `Game.createVictoryConditions()` makes a fresh helper, and `scenario.createGame()` has already called it by the time a host gets the game back. So the ordinary-looking sequence - load the scenario, then configure the options - sets values that nothing will ever read. Nothing fails. There is no exception, no warning, and the matches still play; they simply run to elimination or the round limit, because the condition that would have stopped them was never in the list. In 8v8 matches across 26 player records, no loser finished between 20% and 40% BV, where a 70%-destroyed stop lands, and eleven finished at exactly zero. **What we do:** call `game.createVictoryConditions()` in `SdsHost` after setting the options, which rebuilds the helper against them. **Upstream fix** is a documentation change at least: `createVictoryConditions` is public and does the right thing, but nothing says it must be called again after a victory option changes. Rebuilding the helper lazily on first `checkForVictory` after an option write would remove the ordering requirement altogether. --- ## 11. `FormationType.qualifies` divides by zero on the Order Lance **Severity: fatal to the caller, and one formation only.** `client/ratgenerator/FormationType.java`, the grouping half of `qualifies`: ```java int groupSize = Math.min(gc.getGroupSize(), matching.size()); int numGroups = Math.min(gc.getNumGroups(), matching.size() / groupSize); ``` `createOrderLance` builds its grouping constraint through the three-argument `GroupingConstraint` constructor, which leaves `groupSize` at its default of `0`. Every other formation with a grouping constraint uses the six-argument one and passes `2`. So for the Order Lance the second line divides by zero, and `qualifies` throws `ArithmeticException` rather than answering. It is invisible in the RAT generator because nothing calls `qualifies` on the Order Lance with a list to check: `generateFormation` builds a formation from the tables instead, and the Analyze Formation dialog is driven by a formation the user has already picked. Any caller that walks all 41 formations and asks each one hits it on the first Order Lance. **What we do:** `crates/sds-core/src/formation.rs` reads the definitions rather than calling `qualifies`, so this cannot throw here; the case is recorded because it is the reason the Order Lance's grouping constraint reads `size=0 groups=1` in `crates/sds-core/tests/corpus/formations.txt`, which looks like a dump defect and is not. **Upstream fix** is one word: `createOrderLance` should use the constructor that sets a group size, or `qualifies` should guard `groupSize == 0` by treating the constraint as unsatisfiable rather than dividing by it. --- ## 12. The Anti-Air Lance's description names a quirk it never tests **Severity: cosmetic, and expensive to a reader.** `client/ratgenerator/FormationType.java`, `createAntiAirLance()`. The constraint's description is `"Standard AC, LBX, Artillery weapon, Anti-Air targeting quirk"` and its report-metric key is `"AC/LBX/Artillery/AA Quirk"`. The predicate behind both is: ```java ms -> getMissionRoles(ms).contains(MissionRole.ANTI_AIRCRAFT) || ms.getEquipmentNames().stream().map(EquipmentType::get) .anyMatch(eq -> eq instanceof ACWeapon || eq instanceof LBXACWeapon || eq instanceof ArtilleryWeapon); ``` There is no quirk test. `FormationType` never calls `MekSummary.getQuirkNames()` or `getWeaponQuirkNames()` anywhere in the class. The fourth alternative is the RAT generator's own `MissionRole.ANTI_AIRCRAFT`, which `ModelRecord` parses out of `data/forcegenerator`, and which a unit can carry with no anti-air quirk at all. The description is the only documentation these constraints have — the `qualificationReport` prints it to a player, and a port reads it — so a reader who takes it at its word looks for a quirk that is not consulted and misses a data-file role that is. **What we do:** read the three weapon classes, which is the half that is a fact about a machine on the board, and record the mission-role half as a stated narrowing in `docs/FORMATIONS.md`. The two are an *or*, so the narrowing can only refuse a force MegaMek would have accepted. **Upstream fix** is the description string: "Standard AC, LB-X, artillery weapon, or the anti-aircraft mission role". ## 13. A combat vehicle's front hit table never rolls the right side **Reported upstream: https://github.com/MegaMek/megamek/issues/8850** (filed 2026-08-29 against 0.51.0). Check it before assuming this entry is still live. **Severity: play-affecting, and not headless-specific.** It changes where damage lands in any game MegaMek resolves. `Tank.rollHitLocation(int table, int side)` with `side = SIDE_FRONT` returns `LOC_LEFT` on a 5 **and** on a 9. Every other arc separates the pair: | arc | 5 | 9 | |---|---|---| | front | LS | LS | | left | FR | RR | | right | FR | RR | | rear | LS | RS | So a vehicle shot from in front never takes a right-side hit at all, and takes left-side hits at twice the rate the printed table gives. Rolls 5 and 9 are 4 ways of 36 each, so this is 8 rolls in 36 landing in the wrong place - not a corner case. **Confirmed by live fire.** The table read above is a dice-pinned probe, which is a fine way to read a table and a poor way to be believed. So it was fired instead: `bridge/sds/SdsVehicleLiveFire.java` puts a real Gürteltier MBT on a real board, deploys four Atlases around it, and resolves 200,000 hit locations per arc with real dice, taking the arc from `Entity.sideTable` (re-checked every iteration, drift 0) and the labels from MegaMek's own `getLocationAbbreviations()`. No damage is applied, so no location can be destroyed and bias what follows; the run asserts none was. | arc | FR | RS | LS | RR | TU | |---|---|---|---|---|---| | **front** | 0.6121 | **0.0000** | 0.2213 | 0.0000 | 0.1666 | | rear | 0.0000 | 0.1115 | 0.1111 | 0.6116 | 0.1657 | | left | 0.1123 | 0.0000 | 0.6103 | 0.1111 | 0.1664 | | right | 0.1115 | 0.6111 | 0.0000 | 0.1112 | 0.1663 | **Zero right-side hits in 200,000 from the front**, and the other three arcs are the control that makes it mean something: `RS` is plainly reportable, since the rear arc returns 22,309 of them and the right arc 122,214. Three arcs split their two adjacent locations evenly - rear 0.1111/0.1115, left 0.1123/0.1111, right 0.1115/0.1112 - and the front does not, giving one side 0.2213 and the other nothing. One arc of four failing to split, on the same entity, the same dice and the same counter, is a single-cell defect and not an instrument. The static probe agrees, and it eliminated its own alternatives first: - both rows consume exactly **two** `randomInt(6)` calls, so neither is reaching for a third die the pinned source would be answering wrongly; - forcing a third die to each of its six values moves neither row; - `Tank.SIDE_LOC_MAPPING` is `{FRONT->FR, LEFT->LS, RIGHT->RS, REAR->RR}`, so the arc-to-location mapping is not the cause; - `VTOL.rollHitLocation`, which is a different method, *does* separate them - front 5 gives RS and front 9 gives LS. **What we do:** model it. `crates/sds-core/src/hitloc.rs` holds MegaMek's table rather than the printed one, because MegaMek is what resolves the shots, and `a_vehicles_front_table_hits_one_side_twice_and_that_is_megameks` fails the day it changes. **Upstream fix** is one cell. Worth reporting: it is silent, it is not bot-specific, and a player would have to roll a few hundred front hits on a tank to notice their right side was never getting touched.