Fixed melee attack timeout for unreachable NPCs, clipping window memoization, ranged candidate cap to 16

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

@@ -1,5 +1,6 @@
 package core.game.node.entity.combat
 
+import core.ServerConstants
 import core.game.container.impl.EquipmentContainer
 import core.game.node.Node
 import core.game.node.entity.Entity
@@ -23,7 +24,7 @@ import kotlin.math.sqrt
  * walking queues are ticked.
  */
 object CombatMovementIntents {
-    private const val MAX_RANGED_APPROACH_CANDIDATES = 128
+    private const val MAX_RANGED_APPROACH_CANDIDATES = 16
     private const val MAX_PLAYER_COMBAT_PATH_DETOUR = 6.0
     private const val MAX_DIRECT_COMBAT_PATH_DISTANCE = 32
 
@@ -338,7 +339,7 @@ object CombatMovementIntents {
             reserveOccupiedTiles(reservedTiles, attacker, attackPath.projectedLocation)
             return
         }
-        if (!blockedByReservation && shouldStopUnreachableCombat(attacker, target, standingOnCandidate)) {
+        if (!blockedByReservation && shouldStopUnreachableCombat(attacker, target, standingOnCandidate, trace)) {
             stopUnreachableCombat(attacker)
         }
     }
@@ -1075,9 +1076,36 @@ object CombatMovementIntents {
     }
 
     private fun shouldStopUnreachableCombat(
-        attacker: Entity, target: Entity, exhaustedLocalApproach: Boolean = false
+        attacker: Entity, target: Entity, exhaustedLocalApproach: Boolean = false, trace: IntentTrace? = null
     ): Boolean {
-        return attacker is Player && (exhaustedLocalApproach || !CombatMovementPlanner.hasMovementStepThisTick(target))
+        if (attacker !is Player) {
+            return false
+        }
+        if (exhaustedLocalApproach || !CombatMovementPlanner.hasMovementStepThisTick(target)) {
+            return true
+        }
+        if (attacker.properties.combatPulse.style != CombatStyle.MELEE) {
+            return false
+        }
+        return !hasMeleeRouteToTarget(attacker, target, trace)
+    }
+
+    /**
+     * A moving target should only keep a stalled melee chase alive while the static map
+     * still allows walking adjacent to it; an alternative (partial) route means the rest
+     * of the approach is permanently blocked (e.g. an NPC swimming in water), so waiting
+     * for the target to stand still before rejecting would chase it forever.
+     */
+    private fun hasMeleeRouteToTarget(attacker: Player, target: Entity, trace: IntentTrace?): Boolean {
+        val targetTile = target.getClosestOccupiedTile(attacker.location)
+        if (attacker.location.getDistance(targetTile) > ServerConstants.MAX_PATHFIND_DISTANCE) {
+            return true
+        }
+        if (trace != null) {
+            trace.rsmodRouteCalls++
+        }
+        val path = Pathfinder.find(attacker, target, true, Pathfinder.SMART)
+        return path.isSuccessful && !path.isMoveNear
     }
 
     @JvmStatic

Server/src/main/core/game/world/map/RegionManager.kt

@@ -6,6 +6,7 @@ import core.game.node.entity.Entity
 import core.game.node.entity.npc.NPC
 import core.game.node.entity.player.Player
 import core.game.node.scenery.Scenery
+import core.game.world.GameWorld
 import core.game.world.map.zone.ZoneBorders
 import core.tools.Log
 import core.tools.RandomFunction
@@ -152,6 +153,11 @@ object RegionManager {
 
     @JvmStatic
     fun loadClippingWindow(center: Location, size: Int) {
+        val ensured = ENSURED_WINDOW_REGIONS.get()
+        if (ensured.tick != GameWorld.ticks) {
+            ensured.regions.clear()
+            ensured.tick = GameWorld.ticks
+        }
         val minX = center.x - (size / 2)
         val minY = center.y - (size / 2)
         val maxX = minX + size - 1
@@ -159,12 +165,27 @@ object RegionManager {
         for (regionX in (minX shr 6)..(maxX shr 6)) {
             for (regionY in (minY shr 6)..(maxY shr 6)) {
                 val regionId = (regionX shl 8) or regionY
+                if (!ensured.regions.add(regionId)) {
+                    continue
+                }
                 Region.load(forId(regionId))
                 initialiseRsmodProjectileRegion(regionId)
             }
         }
     }
 
+    /**
+     * Pathfinding probes load the same clipping window many times per tick; regions
+     * cannot transition to unloaded between probes within one tick, so each thread only
+     * needs to ensure a region once per tick instead of taking the region lock per probe.
+     */
+    private class EnsuredWindowRegions {
+        var tick = -1
+        val regions = HashSet<Int>()
+    }
+
+    private val ENSURED_WINDOW_REGIONS = ThreadLocal.withInitial { EnsuredWindowRegions() }
+
     private fun resetRsmodFlags(regionId: Int) {
         val baseX = (regionId shr 8) shl 6
         val baseY = (regionId and 0xFF) shl 6

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

@@ -888,6 +888,64 @@ class CombatMovementTests {
         }
     }
 
+    @Test
+    fun meleePlayerShouldRejectConstantlySwimmingDuckOnceApproachIsExhausted() {
+        TestUtils.getMockPlayer("combat_melee_moving_duck_attacker").use { player ->
+            val duckLocation = Location.create(3238, 3244, 0)
+            val swimTiles = listOf(duckLocation, duckLocation.transform(-1, 0, 0))
+            val start = walkableTileNear(duckLocation, minDistance = 8, 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.")
+                for (tile in swimTiles) {
+                    assertFalse(
+                        RegionManager.isTeleportPermitted(tile), "Lumbridge river duck swim tile should be on water."
+                    )
+                }
+
+                player.attack(duck)
+
+                var rejectedAfter = -1
+                for (tick in 0 until 30) {
+                    queueWalk(duck, swimTiles[(tick + 1) % swimTiles.size])
+                    assertTrue(
+                        CombatMovementPlanner.hasMovementStepThisTick(duck),
+                        "The duck must be moving every tick for this scenario."
+                    )
+                    TestUtils.advanceTicks(1, false)
+                    if (receivedMessage(player, "I can't reach that!")) {
+                        rejectedAfter = tick
+                        break
+                    }
+                }
+
+                assertTrue(
+                    rejectedAfter >= 0,
+                    "Melee combat against a constantly swimming water NPC should be rejected once the " + "walkable approach is exhausted instead of chasing forever. " + "player=${player.location}, duck=${duck.location}"
+                )
+                assertTrue(
+                    rejectedAfter <= 12,
+                    "Rejection should happen shortly after the player reaches the river bank, " + "not after a long futile chase (rejected after $rejectedAfter ticks)."
+                )
+                assertFalse(
+                    player.properties.combatPulse.isAttacking,
+                    "Combat should stop when the moving water NPC is rejected."
+                )
+                assertTrue(
+                    player.walkingQueue.queue.size <= 1,
+                    "Stopping unreachable combat should not keep a movement path queued."
+                )
+            } finally {
+                duck.clear()
+                CombatMovementIntents.clear()
+            }
+        }
+    }
+
     @Test
     fun unreachableCombatTargetShouldStopAndTellPlayer() {
         TestUtils.getMockPlayer("combat_unreachable_target").use { player ->