Skip to content

Let an aero on the ground end its turn, and say where its altitude went - #8959

Open
HammerGS wants to merge 7 commits into
mainfrom
Fix-8931-aero-altitude-loss-no-crash
Open

Let an aero on the ground end its turn, and say where its altitude went#8959
HammerGS wants to merge 7 commits into
mainfrom
Fix-8931-aero-altitude-loss-no-crash

Conversation

@HammerGS

@HammerGS HammerGS commented Sep 12, 2026

Copy link
Copy Markdown
Member

What this changes

An aero sitting on the ground can end its turn again. The unused-velocity check asks isAirborne(), which is true for any aerodyne whatever its altitude, so a fighter at altitude 0 looked like it was still flying and was refused permission to end its turn. It had no way to comply: on a ground map one velocity point costs sixteen hexes of movement, and at altitude 0 every move step is illegal because the aero movement section is gated behind altitude above zero. Move refused and not-moving refused leaves the unit stuck for good. Now exempts an aero at altitude 0 on a ground map.

The altitude report says what the altitude went on. Report 9095 read "loses N altitude(s) from weapon attack", which sounds like the unit was shot down. It fires only when a unit pays for its own air-to-ground attack run: 2 for a dive bomb, 1 for a standard attack, 0 for strafing or an altitude bomb. Nothing an enemy fires reduces a target's altitude. It now reads "loses N altitude(s) making an attack run", with Spanish and Russian updated to match.

Related to #8931

Testing

Tested in game. A fighter at altitude 0 with velocity remaining now ends its turn on Done, where before it was told it had not used all its velocity and the turn would not end.

compileJava, checkstyleMain, spotlessApply and javadoc pass.

What is not proven yet

  • No unit test. The behaviour is a Swing nag gate reading entity state; it was verified by watching the gate's own inputs and verdict in the log, before and after.
  • The reworded report was read in the properties files, not seen on screen.
  • The Spanish and Russian rewordings are mine and have not been checked by a native speaker.

Scope

This came out of #8931 and does not close it. That report has a fighter losing altitude it never spent, which is a separate fault still open. This makes the resulting state survivable rather than terminal.

An earlier version of this PR also added a crash check, on the theory that a fighter could fly itself to altitude 0 on an attack run. It cannot. ComputeToHitIsImpossible already refuses an attack that would take the attacker to zero:

// You can't make attacks that would lower you to zero altitude
if (altitudeLoss >= (attacker.getAltitude() + altLossThisRound)) {
    return Messages.getString("WeaponAttackAction.TooMuchAltLoss");
}

At altitude 1 a standard attack costs 1 and is refused; at altitude 2 a dive bomb costs 2 and is refused. The crash check was guarding a state legal play does not produce, so it has been dropped. Worth knowing for anyone looking at this path in future.

Known and not fixed here

  • Eject is illegal at altitude 0. Confirmed in game: an eject step on a grounded fighter comes back MOVE_ILLEGAL, so the crew cannot leave. Being able to end the turn is not the same as being able to escape.
  • Entity.isAirborne() reads (!isDestroyed() && altitude > 0) || mode == AERODYNE || mode == SPHEROID, so any aerodyne counts as airborne at altitude 0 and even when destroyed. That is the root of this whole class of problem, and 309 call sites depend on it.

  • This PR is focused on one issue or RFE
  • Every file in the diff has a deliberate change
  • Tests added or updated - not applicable; a nag-gate condition and a report string, verified in game
  • Javadoc literals use {@code true} / {@code null} rather than bare or quoted text
  • Dev team only: AI tools used -> AI Assisted Development label applied

Fixes #8931

Altitude stripped by weapon fire was applied without checking whether the
unit had reached the ground, so a fighter could sit at altitude zero
having never crashed. It was then stuck: the velocity nag refused every
move because it still counted as airborne, and eject, shut down and
flying off the map were all blocked with it.

Three of the four places that take altitude off an aero already check for
a crash afterwards: the out-of-control heat path, the control-roll path
and movement. The weapon-attack path did not. It now does, using the same
checkCrash and processCrash pair, which returns false in space and on
board types where altitude zero is legal.

Reported state corroborated by the reporter's game log, which shows the
Aria losing 2, 2, 2 then 1 altitude to weapon fire with no crash reported.
Their save could not be loaded to confirm the final altitude directly; it
predates the removal of Player.numMfConv.

Not changed: Entity.isAirborne reads
  (!isDestroyed() && altitude > 0) || mode == AERODYNE || mode == SPHEROID
so any aerodyne counts as airborne at altitude zero, and when destroyed.
That is why the velocity nag fired at all. 309 call sites depend on it, so
it needs its own change and its own testing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 12, 2026 14:31
@HammerGS HammerGS added the AI Assisted Development This project/code contains AI use under the supervision of a human developer. With Human testing. label Sep 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is minimal, uses established crash-handling utilities consistently with other call sites, and directly addresses the reported stuck-at-altitude-0 behavior without introducing new API surface.

Pull request overview

Fixes a Total Warfare server-side edge case where an aerospace fighter that loses altitude to weapon-fire-induced altitude loss can reach altitude 0 but not be processed as having crashed, leaving it “airborne” and unable to resolve movement/velocity-gated actions (Issue #8931).

Changes:

  • After applying per-round aero altitude loss from weapon fire, immediately check whether the unit has reached a crash condition.
  • If a crash condition is met, invoke the existing crash-processing flow to resolve the crash and generate reports.
File summaries
File Description
megamek/src/megamek/server/totalWarfare/TWGameManager.java Adds a post-altitude-loss checkCrash() + processCrash() step to ensure weapon-fire altitude loss that reaches the ground triggers a crash resolution.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

HammerGS and others added 2 commits September 12, 2026 14:04
The earlier commit called this altitude knocked off by weapon fire. It
is not. WeaponAttackAction.getAltitudeLoss is the altitude the attacker
spends making an air-to-ground attack: 2 for a dive bomb, 1 for a
standard attack, 0 for strafing or an altitude bomb.

The fix is unchanged and still correct. An aero that spends its last
altitude attacking a ground target ends up on the ground, which is a
crash, and nothing checked for one. Only the description was wrong, and
it made the issue impossible to reproduce from the steps given.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y1Lb2sbXg7jYcZoGDgyhWB
@HammerGS

Copy link
Copy Markdown
Member Author

Correction to the description, pushed as af41ea1. The code is unchanged.

I had this as altitude knocked off by weapon fire. It is not. WeaponAttackAction.getAltitudeLoss is the altitude the attacker spends making an air-to-ground attack: 2 for a dive bomb, 1 for a standard attack, 0 for strafing or an altitude bomb. Nothing an enemy fires strips a target's altitude, so the test steps I wrote could never have reproduced it.

The fix still holds. An aero that spends its last altitude attacking a ground target ends up on the ground, and nothing checked for a crash. Reproducing it means dive bombing from altitude 2, or a standard air-to-ground attack from altitude 1.

Report 9095 read "loses N altitude(s) from weapon attack", which sounds
like the unit was shot down. It fires only when a unit spends its own
altitude making an air-to-ground attack. The reporter's log reads as a
fighter being driven into the ground by enemy fire; it was dive bombing.

English, Spanish and Russian updated together. The Russian stays as
\uXXXX escapes to match the rest of that file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y1Lb2sbXg7jYcZoGDgyhWB
@HammerGS HammerGS changed the title Fix #8931: Aero knocked to the ground never crashed Fix #8931: Aero that spends its last altitude attacking never crashed Sep 12, 2026
@HammerGS
HammerGS marked this pull request as ready for review September 12, 2026 23:33
@HammerGS
HammerGS requested a review from a team as a code owner September 12, 2026 23:33
@HammerGS HammerGS added the AI ready for Review Indicates that is has been in game tested and is ready for review as it can be label Sep 12, 2026
@HammerGS HammerGS changed the title Fix #8931: Aero that spends its last altitude attacking never crashed Crash an aero left at altitude 0 by an attack run, and say what the altitude went on Sep 12, 2026
@HammerGS
HammerGS marked this pull request as draft September 12, 2026 23:53
@HammerGS HammerGS removed the AI ready for Review Indicates that is has been in game tested and is ready for review as it can be label Sep 12, 2026
HammerGS and others added 3 commits September 12, 2026 18:11
The unused-velocity gate asks isAirborne(), which is true for any
aerodyne whatever its altitude, so a fighter sitting on the ground still
looked like it was flying and was refused permission to end its turn.

It has no way to comply. On a ground map one velocity point costs
sixteen hexes of movement, and at altitude 0 every move step is illegal
because the aero movement section is gated behind altitude above zero.
Move refused and not-moving refused leaves the unit stranded for good.

Now exempts an aero at altitude 0 on a ground map.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y1Lb2sbXg7jYcZoGDgyhWB
An attack that would take the attacker to altitude 0 is refused before
it can be declared:

    // You can't make attacks that would lower you to zero altitude
    if (altitudeLoss >= (attacker.getAltitude() + altLossThisRound)) {
        return Messages.getString("WeaponAttackAction.TooMuchAltLoss");
    }

At altitude 1 a standard attack costs 1 and is refused; at altitude 2 a
dive bomb costs 2 and is refused. A fighter cannot legally fly itself
into the ground on an attack run, so the crash check guarded a state
legal play does not produce.

The reporter's fighter reached altitude 0 without ever attacking, which
bypasses that guard entirely and is a separate fault still open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y1Lb2sbXg7jYcZoGDgyhWB
@HammerGS HammerGS changed the title Crash an aero left at altitude 0 by an attack run, and say what the altitude went on Let an aero on the ground end its turn, and say where its altitude went Sep 13, 2026
@HammerGS
HammerGS marked this pull request as ready for review September 13, 2026 01:32
@HammerGS HammerGS added the AI ready for Review Indicates that is has been in game tested and is ready for review as it can be label Sep 13, 2026
@HammerGS

Copy link
Copy Markdown
Member Author

Tested in game and working. Out of draft.

A fighter at altitude 0 with velocity remaining now ends its turn on Done. The gate's own inputs are unchanged, only its verdict:

alt=0 airborne=true unusedVelocity=true ... -> blocks=false

Scope narrowed while testing. An earlier version added a crash check for a fighter flying itself to altitude 0 on an attack run; that cannot happen, because ComputeToHitIsImpossible already refuses an attack that would take the attacker to zero. It was dropped in 739da54 and the reasoning is in the notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Assisted Development This project/code contains AI use under the supervision of a human developer. With Human testing. AI ready for Review Indicates that is has been in game tested and is ready for review as it can be

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants