Fix #8863: Point-blank shot offered no turret rotation, and would not fire - #8961
Fix #8863: Point-blank shot offered no turret rotation, and would not fire#8961HammerGS wants to merge 7 commits into
Conversation
Fixes #8863 Total Warfare p.260 lets a hidden unit torso twist or rotate its turret before a point-blank shot. Twist was wired up; rotation was not offered at all, so a turreted vehicle had to fire on whatever facing it hid with. PointblankShotDisplay declares its own FiringCommand enum and it had no rotate commands, which left rotateSelectedMount, rotateRearTurret and updateRotateTurret unreachable during a point-blank shot even though the shared AttackPhaseDisplay already implements all three. Adds FIRE_ROTATE_TURRET and FIRE_ROTATE_TURRET_2, the two setters and the label setter, and the action handling, all mirroring FiringDisplay. The rear-turret button is offered only for a dual-turret vehicle, as in the firing phase, and the buttons are refreshed when the unit is selected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The change affects gameplay-critical UI wiring for a rare rules path and is explicitly untested in-game with no automated coverage, so it needs human verification before approval.
Pull request overview
This PR fixes a UI/phase-display gap in the point-blank shot flow for hidden units: the point-blank display now exposes the same turret-rotation actions that already exist in the shared attack-phase logic, aligning the UI with Total Warfare p.260 and resolving #8863.
Changes:
- Adds point-blank phase commands for rotating the selected turret/mount and (when applicable) the rear turret.
- Wires the new commands into button visibility/enabling/labeling and the action handler, mirroring firing-phase behavior.
- Ensures rotate-turret enablement state is refreshed when selecting an entity during the point-blank shot display.
File summaries
| File | Description |
|---|---|
| megamek/src/megamek/client/ui/panels/phaseDisplay/PointblankShotDisplay.java | Adds rotate-turret commands and hooks them into point-blank shot UI so turret rotation is offered (including dual-turret rear rotation gating). |
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.
Rotating a turret changed the facing but left the old one on screen. Of the five rotation paths only the vehicle main turret refreshed, and only because it is declared as a twist. A Mek turret and a Directional Torso Mount send the facing to the server and nothing comes back, so the board was never told. Two gaps, one each side. The client now redraws the unit and its firing arc after the facing dialog closes, on every path. The server now echoes a mount facing change with entityUpdate, so other players see the turret move at all. Both are older than the point-blank work; the firing and targeting phases went through the same paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1Lb2sbXg7jYcZoGDgyhWB
Found while testing the turret rotation this PR adds. Rotating the turret worked, but nothing the player relies on reflected it, and the shot could not reliably be reached at all. Trigger. A hidden unit only got its shot when the mover stopped next to it, so walking past did nothing. TW p.260 keys the shot on being revealed by enemy movement, and says the target may continue its move afterwards, which is only possible part way through a move. The rule now lives in Compute.revealsHiddenUnitForPointblankShot with tests. Crash. Pressing Fire threw a NullPointerException. PointblankShotDisplay has its own command set and button map, so the parent map is never filled; it overrides ten of the twenty-two button setters and inherits the rest, and any inherited one reached into a null map. All the setters now go through one null-safe helper. Ranges. The field of fire only followed the turret in the firing, targeting and offboard phases. A point-blank shot is aimed during the enemy's movement phase, so the arc was drawn from the hull facing and rotating the turret changed nothing. It now asks whether the player is aiming rather than which phase it is. Redraw. The facing dialog is not modal, so refreshing after setVisible ran before the player had chosen. It is now a callback on accept, and it rebuilds the weapon panel, which is what the arc is drawn from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1Lb2sbXg7jYcZoGDgyhWB
|
Tested in game and working. Out of draft. An Ajax Assault Tank Prime hidden against a moving Mek. The turret rotates, the shot triggers when the enemy walks past rather than only when it stops, Fire completes without throwing, and the range overlay follows the turret. Scope grew from the original issue and the notes say why: fixing the rotation button revealed that the shot could not be reached or seen. The trigger waited for the mover to stop, Fire threw on a null button map, and the field of fire asked which phase the game was in, which is never the firing phase for a point-blank shot. |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1Lb2sbXg7jYcZoGDgyhWB
There was a problem hiding this comment.
🔵 Needs a closer look
It spans a repo-wide firing-button setter refactor, a movement/point-blank trigger rule change, and a client broadcast, with several paths (dual-turret, Mek turret, aerospace, multiplayer) the author states are unverified.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Balanced
Inserting rotateMainTurretTo and revealsHiddenUnitForPointblankShot above existing methods left each of those methods' javadoc stranded on the new one. refreshAfterRotation and canDetectHidden had none. The stranded block also still claimed the facing dialog is modal, which was the wrong reading this branch already corrected. Dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1Lb2sbXg7jYcZoGDgyhWB
What this changes
A hidden unit can now rotate its turret before its point-blank shot, and the shot itself works.
Total Warfare p.260 lets a revealed hidden unit torso twist or rotate its turret before firing. Twist was wired up; rotation was not offered at all, so a turreted vehicle had to fire on whatever facing it had been hiding with.
Testing that turned up three further faults in the same shot. They are in here because without them the rotation is either unreachable or invisible, so the original fix could not be verified on its own.
The button.
PointblankShotDisplaydeclares its own command set, and it had no rotate commands, which leftrotateSelectedMount,rotateRearTurretandupdateRotateTurretunreachable even though the shared parent implements all three. Adds both commands, mirroring the firing phase. The rear-turret button appears only for a dual-turret vehicle, as it does there.The trigger. The shot only fired when the moving unit stopped next to the hidden one, so walking past it did nothing. TW p.260 keys the shot on being revealed by enemy movement, and adds that the target "may continue its move after the attack" when it has MP left, which is only possible part way through a move. The rule now lives in
Compute.revealsHiddenUnitForPointblankShot, next to the related detection rule, and is covered by tests.The crash. Pressing Fire threw a NullPointerException from
setFireChargeLevelEnabled.PointblankShotDisplayhas its own button map, so the parent's is never filled; it overrides ten of the twenty-two button setters and inherits the rest, and any inherited one reached into a null map. Disabling the buttons on Fire hit exactly that. Every setter now goes through one null-safe helper, rather than adding a single override and leaving the other twelve waiting.The ranges. The field of fire only followed the turret during the firing, targeting and offboard phases. A point-blank shot is aimed during the enemy's movement phase, so the arc was drawn from the hull facing and rotating the turret changed the displayed ranges by nothing. It now asks whether the player is aiming rather than which phase it is.
The redraw. The facing dialog is not modal, so refreshing straight after
setVisibleran before the player had chosen anything. It is now a callback on accept, and it rebuilds the weapon panel, which is what the arc is drawn from.Fixes #8863
Testing
Tested in game, on an Ajax Assault Tank Prime hidden against a moving Mek. All four behaviours confirmed: the turret rotates, the shot triggers when the enemy walks past rather than only when it stops, Fire completes without throwing, and the range overlay follows the turret.
compileJava,checkstyleMain,checkstyleTestandjavadocpass.ComputeTestis 32 tests, 0 failures.Three new tests pin the trigger rule: a ground mover reveals at range 0 and 1 and not at 2, an airborne mover without an Active Probe reveals only what it overflies, and one with a probe reveals the adjacent hex instead.
What is not proven yet
Out of scope
Vehicles never rotate their turret graphic on the board: only Meks and ProtoMeks pick their sprite by secondary facing. A vehicle's turret facing shows as the green arrow and in the dialog, which is the existing behaviour everywhere and is left alone.
Three direct button-map lookups outside the setters, in the called-shot hotkey and the RHS update, have the same null-map fault and are untouched here.