Fixed combat movement toward hard unreachable NPCs

Players now attempt to path into combat range before rejecting attacks against stationary NPCs on unwalkable tiles, such as Lumbridge river ducks. Ranged and magic attacks can move to valid attack-range land tiles, while melee still fails with "I can't reach that!" only after pathing as close as possible.

Added regression coverage for Lumbridge river duck ranged and melee attacks, and adjusted unreachable-target combat movement tests to expect pathing before failure.

Server/src/main/core/game/node/entity/combat/CombatMovementIntents.kt

@@ -1,11 +1,17 @@
 package core.game.node.entity.combat
 
+import core.game.container.impl.EquipmentContainer
+import core.game.node.Node
 import core.game.node.entity.Entity
+import core.game.node.entity.combat.equipment.RangeWeapon
+import core.game.node.entity.combat.equipment.Weapon
+import core.game.node.entity.combat.equipment.WeaponInterface
 import core.game.node.entity.npc.NPC
 import core.game.node.entity.player.Player
 import core.game.world.map.Direction
 import core.game.world.map.Location
 import core.game.world.map.Point
+import core.game.world.map.RegionManager
 import core.game.world.map.path.Path
 import core.game.world.map.path.Pathfinder
 import core.game.world.repository.Repository
@@ -16,12 +22,17 @@ import java.util.LinkedHashMap
  * walking queues are ticked.
  */
 object CombatMovementIntents {
+    private const val MAX_RANGED_APPROACH_CANDIDATES = 32
+
     private data class Intent(val attacker: Entity, val target: Entity)
     private data class MovementDestination(
-        val location: Location,
+        val node: Node,
         val pathfinder: Pathfinder,
         val allowPartialPath: Boolean
-    )
+    ) {
+        val location: Location
+            get() = node.location
+    }
     private data class CandidatePath(val steps: List<Point>, val projectedLocation: Location)
 
     private val intents = LinkedHashMap<Entity, Intent>()
@@ -136,7 +147,11 @@ object CombatMovementIntents {
             return false
         }
         if (CombatMovementPlanner.exceedsCombatChaseDistance(attacker, target)) {
-            attacker.properties.combatPulse.stop()
+            if (shouldStopUnreachableCombat(attacker, target)) {
+                stopUnreachableCombat(attacker)
+            } else {
+                attacker.properties.combatPulse.stop()
+            }
             return false
         }
         return attacker !is NPC || !attacker.isNeverWalks
@@ -151,9 +166,91 @@ object CombatMovementIntents {
         }
 
         val targetLocation = targetLocationFor(attacker, target)
-        return CombatMovementPlanner.candidateAttackTiles(attacker, target, targetLocation).map {
+        val destinations = CombatMovementPlanner.candidateAttackTiles(attacker, target, targetLocation).map {
             MovementDestination(it, pathfinder, allowPartialPath = false)
         }
+        if (attacker is Player) {
+            return playerAttackRangeDestinations(attacker, target, targetLocation, pathfinder) +
+                destinations +
+                MovementDestination(target, pathfinder, allowPartialPath = true)
+        }
+        return destinations
+    }
+
+    private fun playerAttackRangeDestinations(
+        attacker: Player,
+        target: Entity,
+        targetLocation: Location,
+        pathfinder: Pathfinder
+    ): List<MovementDestination> {
+        val range = playerAttackRange(attacker)
+        if (range <= CombatReach.meleeDistance(attacker)) {
+            return emptyList()
+        }
+        return attackRangeTiles(attacker, target, targetLocation, range).map {
+            MovementDestination(it, pathfinder, allowPartialPath = false)
+        }
+    }
+
+    private fun playerAttackRange(attacker: Player): Int {
+        return when (attacker.properties.combatPulse.style) {
+            CombatStyle.MAGIC -> 10
+            CombatStyle.RANGE -> playerRangedAttackRange(attacker)
+            else -> CombatReach.meleeDistance(attacker)
+        }
+    }
+
+    private fun playerRangedAttackRange(attacker: Player): Int {
+        var distance = 7
+        val weaponInterface = attacker.getExtension<Any>(WeaponInterface::class.java) as? WeaponInterface
+        if (weaponInterface?.weaponInterface?.interfaceId == 91) {
+            distance -= 2
+        }
+        if (attacker.properties.attackStyle.style == WeaponInterface.STYLE_LONG_RANGE) {
+            distance += 2
+        }
+        val rangeWeapon = RangeWeapon.get(attacker.equipment.getNew(EquipmentContainer.SLOT_WEAPON).id)
+        if (rangeWeapon != null &&
+            (rangeWeapon.weaponType == Weapon.WeaponType.DOUBLE_SHOT ||
+                rangeWeapon.weaponType == Weapon.WeaponType.DEGRADING)
+        ) {
+            distance = 10
+        }
+        return distance
+    }
+
+    private fun attackRangeTiles(attacker: Player, target: Entity, targetLocation: Location, range: Int): List<Location> {
+        val tiles = ArrayList<Location>()
+        val minX = targetLocation.x - range
+        val maxX = targetLocation.x + target.size() - 1 + range
+        val minY = targetLocation.y - range
+        val maxY = targetLocation.y + target.size() - 1 + range
+        for (x in minX..maxX) {
+            for (y in minY..maxY) {
+                val tile = Location.create(x, y, targetLocation.z)
+                if (!RegionManager.isTeleportPermitted(tile)) {
+                    continue
+                }
+                val closestTargetTile = closestOccupiedTile(target, targetLocation, tile)
+                if (tile.getDistance(closestTargetTile) <= range) {
+                    tiles.add(tile)
+                }
+            }
+        }
+        return tiles.sortedWith(
+            compareBy<Location> { it.getDistance(attacker.location) }
+                .thenBy { it.getDistance(closestOccupiedTile(target, targetLocation, it)) }
+                .thenBy { it.x }
+                .thenBy { it.y }
+        ).take(MAX_RANGED_APPROACH_CANDIDATES)
+    }
+
+    private fun closestOccupiedTile(entity: Entity, location: Location, from: Location): Location {
+        return Location.create(
+            from.x.coerceIn(location.x, location.x + entity.size() - 1),
+            from.y.coerceIn(location.y, location.y + entity.size() - 1),
+            location.z
+        )
     }
 
     private fun dumbNpcAttackDestinations(attacker: NPC, target: Entity): List<Location> {
@@ -210,7 +307,7 @@ object CombatMovementIntents {
             return null
         }
 
-        val path = Pathfinder.find(attacker, destination.location, destination.allowPartialPath, destination.pathfinder)
+        val path = Pathfinder.find(attacker, destination.node, destination.allowPartialPath, destination.pathfinder)
         if (!path.reaches(destination.location) && (!destination.allowPartialPath || path.points.isEmpty())) {
             return null
         }
@@ -269,7 +366,8 @@ object CombatMovementIntents {
         return attacker is Player && CombatMovementPlanner.movementStepsThisTick(target) == 0
     }
 
-    private fun stopUnreachableCombat(attacker: Entity) {
+    @JvmStatic
+    fun stopUnreachableCombat(attacker: Entity) {
         attacker.properties.combatPulse.stop()
         attacker.walkingQueue.reset()
         if (attacker is Player) {

Server/src/main/core/game/node/entity/combat/CombatPulse.kt

@@ -113,9 +113,15 @@ class CombatPulse(
             return true
         }
         if (!interactable()) {
-            return if (entity.walkingQueue.isMoving) {
+            return if (entity.walkingQueue.isMoving || entity.walkingQueue.hasPath()) {
                 false
-            } else combatTimeOut++ > entity.properties.combatTimeOut
+            } else {
+                val timedOut = combatTimeOut++ > entity.properties.combatTimeOut
+                if (timedOut && entity is Player && CombatMovementPlanner.movementStepsThisTick(victim!!) == 0) {
+                    CombatMovementIntents.stopUnreachableCombat(entity)
+                }
+                timedOut
+            }
         }
         combatTimeOut = 0
         entity.face(victim)
@@ -203,7 +209,11 @@ class CombatPulse(
             return false
         }
         if (CombatMovementPlanner.exceedsCombatChaseDistance(attacker, target)) {
-            stop()
+            if (attacker is Player && CombatMovementPlanner.movementStepsThisTick(target) == 0) {
+                CombatMovementIntents.stopUnreachableCombat(attacker)
+            } else {
+                stop()
+            }
             return false
         }
         val type = canInteract()

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

@@ -10,6 +10,7 @@ import core.game.node.entity.combat.CombatMovementPlanner
 import core.game.node.entity.combat.equipment.WeaponInterface
 import core.game.node.entity.npc.NPC
 import core.game.node.entity.player.Player
+import core.game.node.entity.skill.Skills
 import core.game.node.item.Item
 import core.game.world.GameWorld
 import core.game.world.map.Location
@@ -23,6 +24,7 @@ import org.junit.jupiter.api.Assertions.assertFalse
 import org.junit.jupiter.api.Assertions.assertNotEquals
 import org.junit.jupiter.api.Assertions.assertTrue
 import org.junit.jupiter.api.Test
+import kotlin.math.abs
 
 class CombatMovementTests {
     init {
@@ -346,6 +348,101 @@ class CombatMovementTests {
         }
     }
 
+    @Test
+    fun rangedPlayerShouldPathTowardLumbridgeRiverDuckBeforeAttacking() {
+        TestUtils.getMockPlayer("combat_ranged_duck_attacker").use { player ->
+            val duckLocation = Location.create(3238, 3244, 0)
+            val start = walkableTileNear(duckLocation, minDistance = 11, maxRadius = 16)
+            place(player, start)
+            configureRanged(player)
+            enableRun(player)
+
+            val duck = stationaryNpc(46, duckLocation)
+            try {
+                assertTrue(RegionManager.isTeleportPermitted(start), "Test start tile must be walkable.")
+                assertFalse(RegionManager.isTeleportPermitted(duckLocation), "Lumbridge river duck spawn should be on water.")
+
+                val startDistance = start.getDistance(duck.location)
+                player.attack(duck)
+                TestUtils.advanceTicks(1, false)
+
+                assertTrue(
+                    player.properties.combatPulse.isAttacking,
+                    "Ranged combat should keep chasing a water NPC instead of rejecting before movement."
+                )
+                assertFalse(receivedMessage(player, "I can't reach that!"))
+
+                TestUtils.advanceTicks(3, false)
+
+                assertTrue(
+                    player.location.getDistance(duck.location) < startDistance,
+                    "Player should path toward ranged attack range."
+                )
+                assertFalse(receivedMessage(player, "I can't reach that!"))
+                TestUtils.advanceTicks(8, false)
+
+                assertTrue(
+                    player.properties.combatPulse.isAttacking,
+                    "Ranged combat should remain active after moving into range."
+                )
+                assertTrue(
+                    player.location.getDistance(duck.location) <= 10.0,
+                    "Player should stop once close enough to attack the duck from land."
+                )
+                assertFalse(receivedMessage(player, "I can't reach that!"))
+            } finally {
+                duck.clear()
+                CombatMovementIntents.clear()
+            }
+        }
+    }
+
+    @Test
+    fun meleePlayerShouldPathTowardLumbridgeRiverDuckBeforeRejecting() {
+        TestUtils.getMockPlayer("combat_melee_duck_attacker").use { player ->
+            val duckLocation = Location.create(3238, 3244, 0)
+            val start = walkableTileNear(duckLocation, minDistance = 11, maxRadius = 16)
+            place(player, start)
+            configureMelee(player)
+            enableRun(player)
+
+            val duck = stationaryNpc(46, duckLocation)
+            try {
+                assertTrue(RegionManager.isTeleportPermitted(start), "Test start tile must be walkable.")
+                assertFalse(RegionManager.isTeleportPermitted(duckLocation), "Lumbridge river duck spawn should be on water.")
+
+                val startDistance = start.getDistance(duck.location)
+                player.attack(duck)
+                TestUtils.advanceTicks(1, false)
+
+                assertTrue(
+                    player.properties.combatPulse.isAttacking,
+                    "Melee combat should attempt to path before reporting that a water NPC cannot be reached."
+                )
+                assertFalse(receivedMessage(player, "I can't reach that!"))
+
+                TestUtils.advanceTicks(3, false)
+
+                assertTrue(
+                    player.location.getDistance(duck.location) < startDistance,
+                    "Player should path toward the duck before failing melee reach."
+                )
+                TestUtils.advanceTicks(20, false)
+
+                assertFalse(player.properties.combatPulse.isAttacking)
+                assertTrue(
+                    receivedMessage(player, "I can't reach that!"),
+                    "Melee combat should report unreachable after pathing. " +
+                        "location=${player.location}, distance=${player.location.getDistance(duck.location)}, " +
+                        "isAttacking=${player.properties.combatPulse.isAttacking}"
+                )
+            } finally {
+                duck.clear()
+                CombatMovementIntents.clear()
+            }
+        }
+    }
+
     @Test
     fun unreachableCombatTargetShouldStopAndTellPlayer() {
         TestUtils.getMockPlayer("combat_unreachable_target").use { player ->
@@ -359,14 +456,28 @@ class CombatMovementTests {
             try {
                 blockMovementTiles(blockedTiles)
 
+                val startDistance = origin.getDistance(npc.location)
                 player.attack(npc)
                 TestUtils.advanceTicks(1, false)
 
+                assertTrue(
+                    player.properties.combatPulse.isAttacking,
+                    "Unreachable local combat targets should first path as close as possible."
+                )
+                assertFalse(receivedMessage(player, "I can't reach that!"))
+
+                TestUtils.advanceTicks(3, false)
+
+                assertTrue(
+                    player.location.getDistance(npc.location) < startDistance,
+                    "Player should move toward the nearest reachable tile first."
+                )
+                TestUtils.advanceTicks(6, false)
+
                 assertFalse(
                     player.properties.combatPulse.isAttacking,
-                    "Unreachable combat targets should stop the combat pulse instead of retrying pathfinding."
+                    "Unreachable combat targets should stop once the closest reachable tile is reached."
                 )
-                assertEquals(origin, player.location)
                 assertTrue(
                     player.walkingQueue.queue.size <= 1,
                     "Stopping unreachable combat should not keep a movement path queued."
@@ -667,6 +778,33 @@ class CombatMovementTests {
         player.walkingQueue.setRunning(false)
     }
 
+    private fun stationaryNpc(id: Int, location: Location): NPC {
+        val npc = NPC.create(id, location)
+        npc.isWalks = false
+        npc.isNeverWalks = true
+        npc.init()
+        npc.skills.setStaticLevel(Skills.HITPOINTS, 10_000)
+        npc.skills.lifepoints = 10_000
+        return npc
+    }
+
+    private fun walkableTileNear(target: Location, minDistance: Int, maxRadius: Int): Location {
+        for (radius in minDistance..maxRadius) {
+            for (dx in -radius..radius) {
+                for (dy in -radius..radius) {
+                    if (abs(dx) != radius && abs(dy) != radius) {
+                        continue
+                    }
+                    val candidate = target.transform(dx, dy, 0)
+                    if (candidate.getDistance(target) >= minDistance && RegionManager.isTeleportPermitted(candidate)) {
+                        return candidate
+                    }
+                }
+            }
+        }
+        throw AssertionError("No walkable tile found near $target between $minDistance and $maxRadius tiles.")
+    }
+
     private fun enablePvp(first: Entity, second: Entity) {
         core.game.world.GameWorld.settings!!.wild_pvp_enabled = true
         first.asPlayer().skullManager.isWilderness = true