Docs, tests

Server/src/test/kotlin/content/CombatMovementTests.kt

@@ -0,0 +1,176 @@
+package content
+
+import TestUtils
+import core.ServerConstants
+import core.game.node.entity.Entity
+import core.game.node.entity.combat.equipment.WeaponInterface
+import core.game.node.entity.npc.NPC
+import core.game.world.map.Location
+import core.game.world.map.RegionManager
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertNotEquals
+import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.Disabled
+import org.junit.jupiter.api.Test
+
+@Disabled("Pending combat movement engine rewrite; see docs/combat-movement-rewrite.md")
+class CombatMovementTests {
+    init {
+        TestUtils.preTestSetup()
+    }
+
+    @Test
+    fun meleeAttackerShouldMirrorRunningVictimAndKeepAttackPressure() {
+        TestUtils.getMockPlayer("combat_mirror_attacker").use { attacker ->
+            TestUtils.getMockPlayer("combat_mirror_victim").use { victim ->
+                val origin = arenaOrigin()
+                place(attacker, origin)
+                place(victim, origin.transform(1, 0, 0))
+                configureMelee(attacker)
+                configureMelee(victim)
+
+                attacker.attack(victim)
+                queueRun(victim, origin.transform(8, 0, 0))
+                TestUtils.advanceTicks(8, false)
+
+                assertTrue(attacker.properties.combatPulse.isAttacking)
+                assertTrue(
+                    meleeReach(attacker, victim),
+                    "Attacker should stay in melee reach while the victim is running."
+                )
+            }
+        }
+    }
+
+    @Test
+    fun mutualMeleeAttackersShouldApproachInsteadOfWaitingForTheOtherActor() {
+        TestUtils.getMockPlayer("combat_meet_a").use { first ->
+            TestUtils.getMockPlayer("combat_meet_b").use { second ->
+                val origin = arenaOrigin()
+                val firstStart = origin
+                val secondStart = origin.transform(6, 0, 0)
+                place(first, firstStart)
+                place(second, secondStart)
+                configureMelee(first)
+                configureMelee(second)
+
+                first.attack(second)
+                second.attack(first)
+                TestUtils.advanceTicks(4, false)
+
+                assertTrue(first.properties.combatPulse.isAttacking)
+                assertTrue(second.properties.combatPulse.isAttacking)
+                assertNotEquals(firstStart, first.location, "First attacker should step toward the target.")
+                assertNotEquals(secondStart, second.location, "Second attacker should step toward the target.")
+                assertTrue(
+                    first.location.getDistance(second.location) < firstStart.getDistance(secondStart),
+                    "Mutual melee combat should close distance instead of waiting indefinitely."
+                )
+            }
+        }
+    }
+
+    @Test
+    fun playerShouldChaseMovingMeleeNpcWithoutGenericInteractionMovementPulse() {
+        TestUtils.getMockPlayer("combat_npc_chaser").use { player ->
+            val origin = arenaOrigin()
+            place(player, origin)
+            configureMelee(player)
+
+            val npc = NPC.create(100, origin.transform(4, 0, 0))
+            npc.init()
+            try {
+                configureMelee(npc)
+                queueRun(npc, origin.transform(10, 0, 0))
+
+                player.attack(npc)
+                TestUtils.advanceTicks(8, false)
+
+                assertTrue(player.properties.combatPulse.isAttacking)
+                assertTrue(
+                    player.location.getDistance(npc.location) <= 2.0,
+                    "Player should continue closing on a moving melee NPC target."
+                )
+            } finally {
+                npc.clear()
+            }
+        }
+    }
+
+    @Test
+    fun movementLockedMeleeAttackerShouldNotMoveButCanAttackIfAlreadyInRange() {
+        TestUtils.getMockPlayer("combat_locked_attacker").use { attacker ->
+            TestUtils.getMockPlayer("combat_locked_victim").use { victim ->
+                val origin = arenaOrigin()
+                place(attacker, origin)
+                place(victim, origin.transform(1, 0, 0))
+                configureMelee(attacker)
+                configureMelee(victim)
+                attacker.locks.lockMovement(10)
+
+                attacker.attack(victim)
+                TestUtils.advanceTicks(2, false)
+
+                assertEquals(origin, attacker.location, "Movement lock should prevent combat chase movement.")
+                assertTrue(
+                    attacker.properties.combatPulse.isAttacking,
+                    "Movement lock should not prevent an in-range melee swing."
+                )
+            }
+        }
+    }
+
+    @Test
+    fun meleeReachShouldUseOccupiedTilesForLargeTargets() {
+        TestUtils.getMockPlayer("combat_large_target_attacker").use { player ->
+            val origin = arenaOrigin()
+            place(player, origin)
+            configureMelee(player)
+
+            val npc = NPC.create(100, origin.transform(4, 0, 0))
+            npc.setSize(2)
+            npc.init()
+            try {
+                configureMelee(npc)
+
+                player.attack(npc)
+                TestUtils.advanceTicks(6, false)
+
+                assertTrue(player.properties.combatPulse.isAttacking)
+                assertTrue(
+                    player.location.getDistance(npc.getClosestOccupiedTile(player.location)) <= 1.0,
+                    "Melee reach should be measured against the large target's occupied border."
+                )
+            } finally {
+                npc.clear()
+            }
+        }
+    }
+
+    private fun arenaOrigin(): Location {
+        return ServerConstants.HOME_LOCATION!!.transform(32, 32, 0)
+    }
+
+    private fun place(entity: Entity, location: Location) {
+        entity.location = location
+        RegionManager.move(entity)
+        entity.walkingQueue.reset()
+    }
+
+    private fun queueRun(entity: Entity, destination: Location) {
+        entity.walkingQueue.reset(true)
+        entity.walkingQueue.addPath(destination.x, destination.y)
+    }
+
+    private fun configureMelee(entity: Entity) {
+        entity.properties.attackStyle = WeaponInterface.AttackStyle(
+            WeaponInterface.STYLE_AGGRESSIVE,
+            WeaponInterface.BONUS_CRUSH
+        )
+        entity.properties.combatPulse.updateStyle()
+    }
+
+    private fun meleeReach(attacker: Entity, victim: Entity): Boolean {
+        return attacker.location.getDistance(victim.getClosestOccupiedTile(attacker.location)) <= 1.0
+    }
+}

docs/combat-movement-rewrite.md

@@ -0,0 +1,100 @@
+# Combat Movement Rewrite Notes
+
+This branch is intended to separate combat movement from generic interaction
+movement before changing combat behavior. The immediate goal is to make the
+current engine boundaries explicit, then replace the hidden coupling in small,
+testable steps.
+
+## Current Tick Order
+
+The major update worker currently runs a tick in this order:
+
+1. `PacketProcessor.processQueue()`
+2. `GameWorld.Pulser.updateAll()`
+3. `GameWorld.tickListeners`
+4. `UpdateSequence.start()`
+5. `UpdateSequence.run()`
+6. `UpdateSequence.end()`
+7. `GameWorld.pulse()`
+8. `Managers.tick()`
+
+`UpdateSequence.start()` ticks NPCs first, then players. Each entity tick runs
+`scripts.preMovement()`, dispatches `TickEvent`, pulses skills, applies
+`walkingQueue.update()`, runs `scripts.postMovement(...)`, processes timers, and
+prepares update masks.
+
+That means pulse code mutates movement state before the entity movement phase,
+but the actual tile step is applied later by `WalkingQueue`.
+
+## Current Combat Movement Coupling
+
+`Properties` constructs one `CombatPulse` for every entity. `CombatPulse`
+constructs a private `MovementPulse(entity, null)` during initialization and
+later uses it as a pathing helper by replacing its destination in
+`CombatPulse.setVictim(...)`.
+
+During `CombatPulse.interactable()`, melee/range/magic swing handlers decide
+whether the attacker can swing, should move and still interact, or cannot
+interact. If movement is needed, `CombatPulse` calls the private movement
+helper's `updatePath()`. That helper resets and repopulates the attacker's
+`WalkingQueue`; `WalkingQueue.update()` applies the step later in the entity
+tick.
+
+NPC and player attack clicks do not go through normal approach scripts in the
+common case. NPC attack interaction is registered as an instant listener, so it
+starts `CombatPulse` directly.
+
+## Known Failure Modes
+
+- A melee attacker following a running victim responds too late and can lose
+  melee pressure.
+- Mutual melee attackers can wait for the other actor to approach instead of
+  both moving to meet.
+- `MovementPulse.checkAllowMovement()` contains an anti-loop rule that blocks a
+  mover from approaching a target that is already combat-pathing back to it.
+- `MeleeSwingHandler.canSwing()` only permits moving interaction against a
+  moving victim when the victim is not targeting the attacker.
+- Combat pathing currently reuses generic interaction movement logic, including
+  object/entity interaction shortcuts that are not necessarily valid combat
+  movement rules.
+
+## Intended Boundary
+
+Movement should remain an engine feature, not a `ScriptProcessor` feature.
+Scripts may request actions and process content-specific results, but the
+movement phase should own tile stepping, path queues, clipping, run/walk state,
+and movement locks.
+
+Combat should also remain an engine feature for targeting, swing timing, reach,
+path pressure, and movement intent. Scripts should be reserved for attack
+results and content effects such as damage application, sounds, graphics,
+special effects, drops, and dialogue/interface side effects.
+
+The rewrite should replace the private combat `MovementPulse` helper with a
+combat movement planner that produces explicit movement intents. Those intents
+should be resolved in a deterministic engine phase before `WalkingQueue` applies
+steps.
+
+## First Behavioral Targets
+
+Pending tests live in `Server/src/test/kotlin/content/CombatMovementTests.kt`.
+They are disabled until the combat movement engine exists so the main test suite
+does not fail during the rewrite.
+
+Initial targets:
+
+- A PvP melee attacker mirrors a running victim and remains in melee reach.
+- Mutual melee attackers approach instead of waiting indefinitely.
+- Player versus moving NPC melee chase remains functional.
+- Movement-locked attackers do not move, but may swing if already in range.
+- Large target melee reach is measured against occupied border tiles.
+
+## Suggested Implementation Sequence
+
+1. Extract a `CombatReach` utility from the swing handlers.
+2. Add a `CombatMovementPlanner` that can choose target border tiles and predict
+   one or two target movement steps from `WalkingQueue`.
+3. Add a combat movement intent phase that resolves step conflicts
+   deterministically.
+4. Remove the private `MovementPulse` from `CombatPulse`.
+5. Enable the pending tests one scenario at a time.