From a816e3378e5a94be03a49cf4f0392b0ad7ffdbc1 Mon Sep 17 00:00:00 2001 From: dam <27978131-real_damighty@users.noreply.gitlab.com> Date: Tue, 28 Apr 2026 23:51:30 +0300 Subject: [PATCH] Docs, tests --- .../kotlin/content/CombatMovementTests.kt | 176 ++++++++++++++++++ docs/combat-movement-rewrite.md | 100 ++++++++++ 2 files changed, 276 insertions(+) create mode 100644 Server/src/test/kotlin/content/CombatMovementTests.kt create mode 100644 docs/combat-movement-rewrite.md diff --git a/Server/src/test/kotlin/content/CombatMovementTests.kt b/Server/src/test/kotlin/content/CombatMovementTests.kt new file mode 100644 index 000000000..ff33179c9 --- /dev/null +++ b/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 + } +} diff --git a/docs/combat-movement-rewrite.md b/docs/combat-movement-rewrite.md new file mode 100644 index 000000000..4d1427d2d --- /dev/null +++ b/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.