Asking the display rather than only the phase, because a hidden unit's point-blank shot is aimed during + * the enemy's movement phase (TW p.260). Testing the phase alone drew that arc from the hull facing and ignored + * the turret entirely, so rotating the turret changed the ranges shown not at all.
+ * + * @return {@code true} if a weapon is being aimed right now + */ + private boolean isPlayerAiming() { + return game.getPhase().isFiring() + || game.getPhase().isTargeting() + || game.getPhase().isOffboard() + || (clientGUI.getCurrentPanel() instanceof PointblankShotDisplay); + } + private void updateFacing(WeaponMounted weapon, int assumedFacing) { if (firingEntity == null) { return; @@ -375,7 +393,7 @@ private void updateFacing(WeaponMounted weapon, int assumedFacing) { // In the aiming phases (firing and targeting/TAG/offboard) the effective facing includes torso twist and any // turret or directional-mount rotation, so the field of fire matches the real firing arc (issues #1040, #6518). // Other phases (e.g. the movement field-of-fire preview) use the base facing. - if (game.getPhase().isFiring() || game.getPhase().isTargeting() || game.getPhase().isOffboard()) { + if (isPlayerAiming()) { facing = TurretFacing.weaponFacing(firingEntity, firingEntity.getEquipmentNum(weapon)); } else { facing = firingEntity.getFacing(); diff --git a/megamek/src/megamek/client/ui/panels/phaseDisplay/AttackPhaseDisplay.java b/megamek/src/megamek/client/ui/panels/phaseDisplay/AttackPhaseDisplay.java index 3b0a7421d72..f04d2b207b0 100644 --- a/megamek/src/megamek/client/ui/panels/phaseDisplay/AttackPhaseDisplay.java +++ b/megamek/src/megamek/client/ui/panels/phaseDisplay/AttackPhaseDisplay.java @@ -331,7 +331,7 @@ public void rotateSelectedMount() { } else if (!tank.hasNoTurret()) { // The main turret follows the unit's secondary facing, so rotating it is a turret twist: the dialog // only picks the facing and the twist is declared through the same path as the Twist button. - new TurretFacingDialog(clientgui.getFrame(), tank, clientgui, this::declareSecondaryFacing) + new TurretFacingDialog(clientgui.getFrame(), tank, clientgui, this::rotateMainTurretTo) .setVisible(true); } return; @@ -350,13 +350,51 @@ public void rotateSelectedMount() { } } + /** + * Declares the main turret's new facing and redraws the unit. Called back by the facing dialog when the player + * accepts it. + * + *The dialog is not modal, so {@code setVisible} returns as soon as it is on screen. Refreshing there ran + * before the player had chosen anything, which is why the board kept the old facing.
+ * + * @param facing the absolute facing (0-5) the player picked + */ + private void rotateMainTurretTo(int facing) { + declareSecondaryFacing(facing); + refreshAfterRotation(); + } + + /** + * Redraws the unit and its firing arc after a turret or mount rotation. + * + *Only one of the rotation paths refreshed the board on its own. A vehicle main turret is declared as a twist, + * which refreshes; a Mek turret and a Directional Torso Mount only send the new facing to the server, which + * applies it and echoes nothing, so the unit kept its old facing on screen until something else redrew it.
+ */ + private void refreshAfterRotation() { + Entity entity = currentEntity(); + if (entity == null) { + return; + } + clientgui.onAllBoardViews(boardView -> boardView.redrawEntity(entity)); + // The arc is drawn for whatever weapon the unit display currently shows, so it only picks up the new + // facing when the weapon panel is rebuilt. Reselecting the same weapon keeps the player's choice, which a + // full refresh would drop back to the first weapon. Same sequence the flip-mount button uses. + WeaponMounted selectedWeapon = clientgui.getUnitDisplay().wPan.getSelectedWeapon(); + clientgui.getUnitDisplay().wPan.displayMek(entity); + if (selectedWeapon != null) { + clientgui.getUnitDisplay().wPan.selectWeapon(selectedWeapon); + } + clientgui.updateFiringArc(entity); + } + /** * Opens the facing dialog for a dual-turret vehicle's rear (main) turret - the "Rotate Rr. Turret" button. The rear * turret follows the unit's secondary facing, so the rotation is declared as a turret twist. */ public void rotateRearTurret() { if ((currentEntity() instanceof Tank tank) && !tank.hasNoDualTurret()) { - new TurretFacingDialog(clientgui.getFrame(), tank, clientgui, this::declareSecondaryFacing) + new TurretFacingDialog(clientgui.getFrame(), tank, clientgui, this::rotateMainTurretTo) .setVisible(true); } } diff --git a/megamek/src/megamek/client/ui/panels/phaseDisplay/FiringDisplay.java b/megamek/src/megamek/client/ui/panels/phaseDisplay/FiringDisplay.java index 142d1056331..cc872fbea08 100644 --- a/megamek/src/megamek/client/ui/panels/phaseDisplay/FiringDisplay.java +++ b/megamek/src/megamek/client/ui/panels/phaseDisplay/FiringDisplay.java @@ -2467,9 +2467,29 @@ private void doToggleRHS() { updateRHS(); } + /** + * Enables or disables one firing button and its menu item. + * + *The button may be absent. {@link PointblankShotDisplay} declares its own command set and its own button + * map, so this map is never filled in for it; it overrides the setters for the buttons it has and inherits the + * rest, which have no button to enable. Reaching straight into the map threw instead: firing a point-blank shot + * crashed on the Fire button, because disabling the buttons ran through a command that display does not carry.
+ * + * @param command the button to change + * @param enabled whether it should be usable + */ + private void enableFiringButton(FiringCommand command, boolean enabled) { + if (buttons != null) { + MegaMekButton button = buttons.get(command); + if (button != null) { + button.setEnabled(enabled); + } + } + clientgui.getMenuBar().setEnabled(command.getCmd(), enabled); + } + protected void setFireEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_FIRE).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_FIRE.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_FIRE, enabled); } /** @@ -2485,50 +2505,46 @@ protected void setFireEnabled(boolean enabled) { * @return {@code true} if firing the selected weapon is currently allowed */ public boolean isFireAllowed() { - return buttons.get(FiringCommand.FIRE_FIRE).isEnabled(); + if (buttons == null) { + return false; + } + MegaMekButton fireButton = buttons.get(FiringCommand.FIRE_FIRE); + return (fireButton != null) && fireButton.isEnabled(); } protected void setTwistEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_TWIST).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_TWIST.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_TWIST, enabled); } protected void setSkipEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_SKIP).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_SKIP.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_SKIP, enabled); } protected void setFindClubEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_FIND_CLUB).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_FIND_CLUB.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_FIND_CLUB, enabled); } protected void setNextTargetEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_NEXT_TARG).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_NEXT_TARG.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_NEXT_TARG, enabled); } protected void setFlipArmsEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_FLIP_ARMS).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_FLIP_ARMS.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_FLIP_ARMS, enabled); } @Override protected void setFlipMountEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_FLIP_MOUNT).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_FLIP_MOUNT.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_FLIP_MOUNT, enabled); } @Override protected void setRotateTurretEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_ROTATE_TURRET).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_ROTATE_TURRET.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_ROTATE_TURRET, enabled); } @Override protected void setRotateRearTurretEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_ROTATE_TURRET_2).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_ROTATE_TURRET_2.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_ROTATE_TURRET_2, enabled); } @Override @@ -2538,23 +2554,19 @@ protected void setRotateTurretLabel(boolean dualTurretTank) { } protected void setSpotEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_SPOT).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_SPOT.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_SPOT, enabled); } protected void setSearchlightEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_SEARCHLIGHT).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_SEARCHLIGHT.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_SEARCHLIGHT, enabled); } protected void setFireModeEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_MODE).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_MODE.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_MODE, enabled); } protected void setFireChargeLevelEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_CHARGE).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_CHARGE.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_CHARGE, enabled); } /** @@ -2591,48 +2603,39 @@ private void updateFireModeTooltip(Mounted weapon) { } protected void setFireCalledEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_CALLED).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_CALLED.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_CALLED, enabled); } protected void setFireClearTurretEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_CLEAR_TURRET).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_CLEAR_TURRET.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_CLEAR_TURRET, enabled); } protected void setFireClearWeaponJamEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_CLEAR_WEAPON).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_CLEAR_WEAPON.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_CLEAR_WEAPON, enabled); } protected void setFireExtinguishEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_EXTINGUISH).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_EXTINGUISH.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_EXTINGUISH, enabled); } protected void setStrafeEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_STRAFE).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_STRAFE.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_STRAFE, enabled); } protected void setNextEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_NEXT).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_NEXT.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_NEXT, enabled); } protected void setActivateSPAEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_ACTIVATE_SPA).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_ACTIVATE_SPA.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_ACTIVATE_SPA, enabled); } protected void setRHSEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_RHS).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_RHS.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_RHS, enabled); } protected void setSuicideImplantsEnabled(boolean enabled) { - buttons.get(FiringCommand.FIRE_SUICIDE_IMPLANTS).setEnabled(enabled); - clientgui.getMenuBar().setEnabled(FiringCommand.FIRE_SUICIDE_IMPLANTS.getCmd(), enabled); + enableFiringButton(FiringCommand.FIRE_SUICIDE_IMPLANTS, enabled); } @Override diff --git a/megamek/src/megamek/client/ui/panels/phaseDisplay/PointblankShotDisplay.java b/megamek/src/megamek/client/ui/panels/phaseDisplay/PointblankShotDisplay.java index 0f0e0109bec..23514b47dd9 100644 --- a/megamek/src/megamek/client/ui/panels/phaseDisplay/PointblankShotDisplay.java +++ b/megamek/src/megamek/client/ui/panels/phaseDisplay/PointblankShotDisplay.java @@ -62,7 +62,6 @@ import megamek.common.compute.Compute; import megamek.common.compute.ComputeArc; import megamek.common.enums.AimingMode; -import megamek.common.enums.ChargeLevel; import megamek.common.enums.GamePhase; import megamek.common.equipment.AmmoType; import megamek.common.equipment.Mounted; @@ -73,6 +72,7 @@ import megamek.common.options.OptionsConstants; import megamek.common.rolls.TargetRoll; import megamek.common.units.Entity; +import megamek.common.units.Tank; import megamek.common.units.Targetable; import megamek.common.weapons.Weapon; import megamek.common.weapons.capitalWeapons.CapitalMissileWeapon; @@ -97,6 +97,8 @@ public class PointblankShotDisplay extends FiringDisplay { */ public enum FiringCommand implements PhaseCommand { FIRE_TWIST("fireTwist"), + FIRE_ROTATE_TURRET("fireRotateTurret"), + FIRE_ROTATE_TURRET_2("fireRotateTurret2"), FIRE_FIRE("fireFire"), FIRE_SKIP("fireSkip"), FIRE_MODE("fireMode"), @@ -310,6 +312,11 @@ protected ArrayListA hidden unit revealed by enemy movement may immediately make the shot, and the rule allows the target to + * "continue its move after the attack" when it has MP left. That is only possible part way through a move, so a + * ground unit reveals as it passes rather than only when it stops. Requiring the mover to stop meant walking + * past a hidden unit did nothing at all.
+ * + *An airborne mover is different: it reveals what it flies over, so the range depends on whether it carries + * an Active Probe.
+ * + * @param mover the unit that is moving + * @param distance hexes between the mover's current step and the hidden unit + * + * @return {@code true} if the hidden unit is revealed and may take its shot + */ + public static boolean revealsHiddenUnitForPointblankShot(Entity mover, int distance) { + if (distance > 1) { + return false; + } + if (!mover.isAirborne()) { + return true; + } + return distance == ((mover.getBAPRange() > 0) ? 1 : 0); + } + /** * Lightweight helper for some step evaluation. No side effects. * diff --git a/megamek/src/megamek/server/totalWarfare/MovePathHandler.java b/megamek/src/megamek/server/totalWarfare/MovePathHandler.java index 0095dec453f..7b3abaf5985 100644 --- a/megamek/src/megamek/server/totalWarfare/MovePathHandler.java +++ b/megamek/src/megamek/server/totalWarfare/MovePathHandler.java @@ -2195,15 +2195,17 @@ private void processSteps() { // Potential point-blank shot when not causing stacking violation, but only in some situations: - // 1. mover is ground unit _and_ ends its movement adjacent to / in the hidden unit's hex; + // 1. mover is a ground unit and moves adjacent to / into the hidden unit's hex; // 2. mover is Aerospace and hidden unit is within detection range of its flight path // (with or without Active Probe). // and the revealed hidden unit has not already made a pointblank shot this turn. - } else if ( - (dist <= 1) && !hiddenEntity.madePointblankShot() && - ((!this.entity.isAirborne() && md.isEndStep(step)) || - (this.entity.isAirborne() && (dist == ((this.entity.getBAPRange() > 0) ? 1 : 0)))) - ) { + // + // The ground case deliberately does not wait for the end of the move. TW: a hidden unit + // revealed by enemy movement may immediately make the shot, and the target "may continue + // its move after the attack" if it has MP left - which can only happen part way through a + // move. Requiring the mover to stop meant walking past a hidden unit did nothing at all. + } else if (!hiddenEntity.madePointblankShot() + && Compute.revealsHiddenUnitForPointblankShot(this.entity, dist)) { // Hidden unit should always be revealed as the PBS trigger _is_ getting revealed. hiddenEntity.setHidden(false); diff --git a/megamek/src/megamek/server/totalWarfare/TWGameManager.java b/megamek/src/megamek/server/totalWarfare/TWGameManager.java index bd758fe49cf..4223dc9f0a3 100644 --- a/megamek/src/megamek/server/totalWarfare/TWGameManager.java +++ b/megamek/src/megamek/server/totalWarfare/TWGameManager.java @@ -27510,6 +27510,10 @@ private void receiveEntityMountedFacingChange(Packet c, int connIndex) throws In } else { m.setFacing(facing); } + // Tell the clients. Without this the facing changed only on the server and on the client that sent it, + // so nobody else saw the turret move and the sender's own board kept the old facing until some other + // update happened to redraw the unit. + entityUpdate(entityId); } /** diff --git a/megamek/unittests/megamek/common/ComputeTest.java b/megamek/unittests/megamek/common/ComputeTest.java index cdda63c34c0..53974603641 100644 --- a/megamek/unittests/megamek/common/ComputeTest.java +++ b/megamek/unittests/megamek/common/ComputeTest.java @@ -1091,4 +1091,42 @@ void mekInNonBuildingHexIsNotInBuilding() { assertFalse(Compute.isInBuilding(getGame(), mek)); } } + + @Test + void groundMoverRevealsAHiddenUnitAsItPasses() { + // TW p.260: the shot follows from being revealed by enemy movement, and the target may continue its move + // afterwards, which is only possible part way through a move. Walking past used to reveal nothing. + Entity groundMover = mock(Entity.class); + when(groundMover.isAirborne()).thenReturn(false); + + assertTrue(Compute.revealsHiddenUnitForPointblankShot(groundMover, 0), + "moving into the hidden unit's own hex reveals it"); + assertTrue(Compute.revealsHiddenUnitForPointblankShot(groundMover, 1), + "moving adjacent reveals it, whether or not the mover stops there"); + assertFalse(Compute.revealsHiddenUnitForPointblankShot(groundMover, 2), + "two hexes away is out of reach"); + } + + @Test + void airborneMoverRevealsOnlyWhatItFliesOverWithoutAProbe() { + Entity flyer = mock(Entity.class); + when(flyer.isAirborne()).thenReturn(true); + when(flyer.getBAPRange()).thenReturn(0); + + assertTrue(Compute.revealsHiddenUnitForPointblankShot(flyer, 0), "it reveals what it overflies"); + assertFalse(Compute.revealsHiddenUnitForPointblankShot(flyer, 1), + "without an Active Probe an adjacent hex is not revealed"); + } + + @Test + void airborneMoverWithAProbeRevealsAnAdjacentHex() { + Entity flyerWithProbe = mock(Entity.class); + when(flyerWithProbe.isAirborne()).thenReturn(true); + when(flyerWithProbe.getBAPRange()).thenReturn(4); + + assertTrue(Compute.revealsHiddenUnitForPointblankShot(flyerWithProbe, 1), + "an Active Probe extends the reveal to an adjacent hex"); + assertFalse(Compute.revealsHiddenUnitForPointblankShot(flyerWithProbe, 0), + "with a probe the reveal is the adjacent hex, not the overflown one"); + } }