Implement RFE #7032 / Core Rules - Walk on deployment - #8812
Conversation
…e using walk on initiative, and exclude them from movement init too if immobile.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8812 +/- ##
============================================
+ Coverage 36.41% 36.58% +0.16%
- Complexity 32692 33021 +329
============================================
Files 3586 3589 +3
Lines 354030 354601 +571
Branches 62030 62135 +105
============================================
+ Hits 128934 129740 +806
+ Misses 211598 211109 -489
- Partials 13498 13752 +254 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Add setter function for TW Add initialization to the TWRulesManager for walkOnDeployment.
|
This implements an old RFE. #7032 |
TenkawaBC
changed the title
There was a problem hiding this comment.
Pull request overview
Begins implementing “walk-on deployment” for Core Rules by introducing a new DEPLOY move step and adjusting eligibility/turn-handling so undeployed units can enter the map during the Movement phase instead of the Deployment phase (when the rule is enabled).
Changes:
- Add
MoveStepType.DEPLOYplus aDeployStepphase-pass to support a deployment step in movement path compilation. - Update client movement UI/commands to allow selecting a legal deployment hex and to clear deployment state when cancelling movement.
- Add/propagate a
walkOnDeployment()rule toggle viaRulesGame(Core defaults on; TW defaults off with a placeholder hook).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| megamek/src/megamek/server/totalWarfare/TWGameManager.java | Attempts to skip certain entities from turn counting based on walk-on deployment/immobility. |
| megamek/src/megamek/server/totalWarfare/MovePathHandler.java | Applies server-side “deploy” effects when a move path contains DEPLOY. |
| megamek/src/megamek/common/units/Entity.java | Adjusts phase eligibility to allow undeployed entities to participate in Movement under walk-on deployment. |
| megamek/src/megamek/common/rules/totalwarfare/TWRulesGame.java | Adds TW-side backing boolean and overrides for walk-on deployment (default off). |
| megamek/src/megamek/common/rules/RulesGame.java | Introduces walkOnDeployment() and a TW hook setWalkOnDeployment(boolean). |
| megamek/src/megamek/common/rules/core/CoreRulesGame.java | Enables walk-on deployment in Core rules (default on). |
| megamek/src/megamek/common/moves/PhasePassSelector.java | Registers a DeployStep phase-pass for MoveStepType.DEPLOY. |
| megamek/src/megamek/common/moves/MoveStep.java | Adds per-step isDeploying state for the new deploy step. |
| megamek/src/megamek/common/moves/DeployStep.java | New phase-pass that marks a step as deploying and short-circuits compilation. |
| megamek/src/megamek/common/game/Game.java | Adds a placeholder hook to enable walk-on deployment for TW via game options (not yet wired). |
| megamek/src/megamek/common/enums/MoveStepType.java | Adds DEPLOY as a new move step type. |
| megamek/src/megamek/common/board/Board.java | Narrows edge deployment-zone width to 1 when walk-on deployment is enabled. |
| megamek/src/megamek/client/ui/panels/phaseDisplay/MovementDisplay.java | Adds UI handling to place an undeployed unit on a legal edge hex during Movement. |
| megamek/src/megamek/client/commands/MoveCommand.java | Clears deployment state when cancelling planned movement containing DEPLOY. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Walk on deployment no longer has issue with invalid hexes Walk on deployment is now consistent with deployment
New issue found. switching to Jump generates an NPE around StepSprite.
|
StepSprite line 93 is causing issues when switching movement type to Jump. |
|
Code to set a turn in the hex of deployment to be no cost is not working properly. |
Add exclusion to prevent bots from using walk-on deployment (until they support it)
|
-Need overall review for how to position initial deployment to not cost turns if immediately after deployment and not exiting the hex yet. |
|
Here is the DropShip fix as a patch, ready to apply. It is the change from my last comment, nothing else. I checked it applies cleanly to Save as diff --git a/megamek/src/megamek/common/units/Dropship.java b/megamek/src/megamek/common/units/Dropship.java
index de837e9ce3..0fa46e4b7e 100644
--- a/megamek/src/megamek/common/units/Dropship.java
+++ b/megamek/src/megamek/common/units/Dropship.java
@@ -565,6 +565,13 @@ public class Dropship extends SmallCraft {
}
super.setPosition(position, false);
+ if (position == null) {
+ // Clearing a walk-on deployment sets the position to null. Without dropping the secondary hexes too,
+ // getOccupiedCoords() keeps returning the seven hexes this DropShip used to fill, the position lookup
+ // never releases them, and anything that asks what is in one of those hexes gets back a DropShip
+ // whose getPosition() is null.
+ secondaryPositions.clear();
+ }
if ((getAltitude() == 0) && (null != game) && !isSpaceborne() && (position != null)) {
secondaryPositions.put(0, position);
secondaryPositions.put(1, position.translated(getFacing()));With it applied, clearing a walk-on deployment takes Two things worth doing alongside it, which the patch deliberately does not touch since they are your code to change:
Still digging on why an airborne aero's DEPLOY step comes out illegal. Will follow up separately. |
# Conflicts: # megamek/src/megamek/client/ui/panels/phaseDisplay/DeploymentDisplay.java # megamek/src/megamek/common/units/Entity.java # megamek/src/megamek/server/totalWarfare/TWGameManager.java
# Conflicts: # megamek/src/megamek/server/totalWarfare/TWGameManager.java
|
After making the changes, still getting an NPE when pressing escape twice with a dropship at altitude. |
|
09:34:52,164 ERROR [megamek.MegaMek] {AWT-EventQueue-0} |
…ovement. Dropships have been moved to always use the deployment phase for now.
|
Testing has shown the following:
|
…tart of deployment.
|
Picking this up at First, an apology. Two comments ago I said the paint path could not throw. I traced it with a landed DropShip, and a landed DropShip takes the branch of Where it lands, and why only airborne units
g.drawImage(image, p.x, p.y - (int) (bv.DROP_SHADOW_DISTANCE * bv.getScale()), this);
The flat Why the sprite is still there
That is findings (3), (4) and (5) exactly: comment out Numbers, from a throwaway test with a mocked
Same harness on The fixTwo hunks. The first is the actual fix; the second is the guard so a stray sprite can never take the client down again. Applies cleanly to diff --git a/megamek/src/megamek/client/ui/panels/phaseDisplay/MovementDisplay.java b/megamek/src/megamek/client/ui/panels/phaseDisplay/MovementDisplay.java
--- a/megamek/src/megamek/client/ui/panels/phaseDisplay/MovementDisplay.java
+++ b/megamek/src/megamek/client/ui/panels/phaseDisplay/MovementDisplay.java
@@ -2016,6 +2016,10 @@ public class MovementDisplay extends ActionPhaseDisplay {
markDeploymentHexes(currentlySelectedEntity);
currentlySelectedEntity.setDeployed(false);
currentlySelectedEntity.setPosition(null);
+ // The unit no longer has a position, so its sprites must go. redrawEntity() is the method that
+ // strips the sprites of an entity with a null position; without this call the BoardView keeps an
+ // IsometricSprite for the unit and the next repaint dereferences the null position.
+ clientgui.boardViews().forEach(bv -> ((BoardView) bv).redrawEntity(currentlySelectedEntity));
refreshButtons();
}
return;
diff --git a/megamek/src/megamek/client/ui/clientGUI/boardview/sprite/isometric/IsometricSprite.java b/megamek/src/megamek/client/ui/clientGUI/boardview/sprite/isometric/IsometricSprite.java
--- a/megamek/src/megamek/client/ui/clientGUI/boardview/sprite/isometric/IsometricSprite.java
+++ b/megamek/src/megamek/client/ui/clientGUI/boardview/sprite/isometric/IsometricSprite.java
@@ -117,6 +117,11 @@ public class IsometricSprite extends HexSprite {
} else {
p = bv.getHexLocation(entity.getSecondaryPositions().get(secondaryPos));
}
+ if (p == null) {
+ // The entity has no position (for example a walk-on deployment that was taken back with Escape).
+ // There is nothing to draw; BoardView.redrawEntity(entity) is what removes this sprite.
+ return;
+ }
Graphics2D g2 = (Graphics2D) g;
if (onlyDetectedBySensors()) {The guard alone would stop the crash, but it would leave the un-deployed unit's flat sprite painted at the hex it just left until something else redraws it, so I would take both. Worth keeping as a regression test: the Two things I hit on the way, same commit range
Small
Both throwaway tests were deleted after taking the numbers. |
|
Follow-up on the airborne walk-on path, at One correction to my 2026-09-11 comment first: the What compiles todayAerospace fighter, altitude 3, on a 16x17 ground map, facing south at 0303. Each row is one step: the movement type it gets, its MP, hexes counted as flown ( With the lobby default velocity of 0, which is what a fighter has unless someone typed a Start Velocity in the lobby:
So a fighter that walks on cannot plot anything: the deploy step is red before the first click, Accelerate is red too, and there is no way out. Two rules do it together. With velocity 3 the deploy step is legal, but it still counts as a flown hex (
Landed fighter, altitude 0:
TAKEOFF is only legal on the first step ( Two things worth knowing that are not bugs. The client never sets a velocity on walk-on; The method: DEPLOY is placement, not movementThree hunks, all in
diff --git a/megamek/src/megamek/common/moves/DeployStep.java b/megamek/src/megamek/common/moves/DeployStep.java
--- a/megamek/src/megamek/common/moves/DeployStep.java
+++ b/megamek/src/megamek/common/moves/DeployStep.java
@@ -59,7 +59,10 @@ class DeployStep implements PhasePass {
final Entity entity,
MoveStep prev,
final CachedEntityState cachedEntityState) {
- if (entity.isDropShip() && entity.isAeroLandedOnGroundMap()) {
+ if (entity.isAero()) {
+ // Walk-on placement of an aerospace unit, airborne or landed, is not a hex flown or taxied: no MP,
+ // and none of the velocity, straight-flight or distance bookkeeping that compileMove() would add.
+ // The step keeps the position, facing, altitude and velocity it was seeded with from the entity.
moveStep.setMp(0);
return PhasePassResult.BREAK;
}
diff --git a/megamek/src/megamek/common/moves/MoveStep.java b/megamek/src/megamek/common/moves/MoveStep.java
--- a/megamek/src/megamek/common/moves/MoveStep.java
+++ b/megamek/src/megamek/common/moves/MoveStep.java
@@ -912,6 +912,11 @@ public class MoveStep implements Serializable {
// A climb mode change is only meta info and does not count as an action
setFirstStep();
+ } else if (prev.isFirstStep() && (prev.getType() == MoveStepType.DEPLOY)) {
+ // Walk-on placement puts the unit on the map; it is not an action, so the step after it is the first
+ // real step (take-off, first hex flown, first-step amnesties all key on this)
+ setFirstStep();
+
} else if (prev.isFirstStep()
&& prev.isTurning
&& entity instanceof ConvInfantry infantry
@@ -2181,6 +2186,11 @@ public class MoveStep implements Serializable {
if ((type == MoveStepType.CLIMB_MODE_ON) || (type == MoveStepType.CLIMB_MODE_OFF)) {
movementType = prev.movementType;
}
+ if (type == MoveStepType.DEPLOY) {
+ // Walk-on placement is legal in itself. An aerospace unit's deploy step costs nothing and stays
+ // MOVE_NONE; a ground unit's deploy hex costs terrain MP and is graded walk/run like any other hex below.
+ movementType = EntityMovementType.MOVE_NONE;
+ }
// check for ejection (always legal?)
if (type == MoveStepType.EJECT) {
movementType = EntityMovementType.MOVE_NONE;Same paths with the patchEvery walk-on path now matches its already-deployed control step for step, with DEPLOY in front as a free, zero-distance step:
On the drawing side nothing needs to change. Not proven here
The throwaway test was deleted after taking the numbers. |
|
Revised to the rule Roundtop set out: a DropShip walks on for 0 MP whether it is landed or flying; a landed fighter walks on like a ground unit and pays the terrain cost of its entry hex; anything airborne gets a flight path with accelerate and decelerate working; and the server has to accept what the client sends. This supersedes the diff in my previous comment - only the Why there is never a valid path today, end to endI built the path the client builds for a walk-on fighter, velocity 0 from the lobby, altitude 3, facing south: DEPLOY, Accelerate, then 16 forward hexes (one velocity point on a ground map). Then I handed that path to
The chain on the head: The method, three hunks
Applies cleanly to diff --git a/megamek/src/megamek/common/moves/DeployStep.java b/megamek/src/megamek/common/moves/DeployStep.java
--- a/megamek/src/megamek/common/moves/DeployStep.java
+++ b/megamek/src/megamek/common/moves/DeployStep.java
@@ -59,7 +59,11 @@ class DeployStep implements PhasePass {
final Entity entity,
MoveStep prev,
final CachedEntityState cachedEntityState) {
- if (entity.isDropShip() && entity.isAeroLandedOnGroundMap()) {
+ if (entity.isAirborne() || entity.isDropShip()) {
+ // Walk-on placement of an airborne unit, or of a DropShip landed or not, is not a hex flown or taxied:
+ // no MP, and none of the velocity, straight-flight or distance bookkeeping that compileMove() would add.
+ // The step keeps the position, facing, altitude and velocity it was seeded with from the entity.
+ // A landed fighter walks on like a ground unit and pays the terrain cost of its entry hex.
moveStep.setMp(0);
return PhasePassResult.BREAK;
}
diff --git a/megamek/src/megamek/common/moves/MoveStep.java b/megamek/src/megamek/common/moves/MoveStep.java
--- a/megamek/src/megamek/common/moves/MoveStep.java
+++ b/megamek/src/megamek/common/moves/MoveStep.java
@@ -912,6 +912,11 @@ public class MoveStep implements Serializable {
// A climb mode change is only meta info and does not count as an action
setFirstStep();
+ } else if (prev.isFirstStep() && (prev.getType() == MoveStepType.DEPLOY)) {
+ // Walk-on placement puts the unit on the map; it is not an action, so the step after it is the first
+ // real step (take-off, first hex flown, first-step amnesties all key on this)
+ setFirstStep();
+
} else if (prev.isFirstStep()
&& prev.isTurning
&& entity instanceof ConvInfantry infantry
@@ -2181,6 +2186,11 @@ public class MoveStep implements Serializable {
if ((type == MoveStepType.CLIMB_MODE_ON) || (type == MoveStepType.CLIMB_MODE_OFF)) {
movementType = prev.movementType;
}
+ if (type == MoveStepType.DEPLOY) {
+ // Walk-on placement is legal in itself. An aerospace unit's deploy step costs nothing and stays
+ // MOVE_NONE; a ground unit's deploy hex costs terrain MP and is graded walk/run like any other hex below.
+ movementType = EntityMovementType.MOVE_NONE;
+ }
// check for ejection (always legal?)
if (type == MoveStepType.EJECT) {
movementType = EntityMovementType.MOVE_NONE;Every case, step by step, with the patchEach walk-on path was compiled next to the same path for an already-deployed unit (the control). Airborne fighter at altitude 3:
Landed fighter and DropShips:
The DropShip rows are compile-only; DropShips are still excluded from walk-on by Flight path, accelerate and decelerate, displayNothing extra is needed to get a flight path rather than a ground path. Which path a click builds is decided by the entity, not the step: Accelerate and decelerate: the buttons already open up after a deploy-only path through Display: Server side
Not proven here
Both throwaway tests were deleted after taking the numbers. |
|
One issue remaining that I have found. When switching to jumping movement, the cost of the first hex terrain is still applied. eg: clicking into a light woods hex to deploy, click jump, then click the next hex it shows 3, not 2. it correctly ignores the terrain and elevation of the next hex when jumping, so this is only when the deploy step is used and the unit is jumping. |
This is the beginning of the implementation of walk-on deployment. Gated behind core rules.
What this changes
If an entity is not yet deployed, and their deployment is not Any or Center, and the deployment round is not <0 (pre-deployment), then skip the unit for the deployment phase and instead have them deploy as part of movement.
The first click in the movement phase will be on the edge to deploy the unit, the second click should make the movement path. Pressing Escape should remove the entire path and clear the deployment, so it can be deployed elsewhere.
When clicking the Move button, it sends the movepath to the server and tells the server to also deploy the entity.
As part of core rules, it is also skipping any units that are immobile for movement/deployment and initiative.
This is a pre-alpha and has several issues currently (See below)
Known issues:
-Deployment zones in lobby still show the width of 3, even though I am overriding this to 1 in the code. This should instead set to 1 when walkOnDeployment() is true by default.