From 9331e40eb4dd2e322bbde9714cd850c9e5f6f996 Mon Sep 17 00:00:00 2001 From: dam <27978131-real_damighty@users.noreply.gitlab.com> Date: Tue, 7 Jul 2026 15:15:58 +0300 Subject: [PATCH] Stall thieving regression fix --- .../skill/thieving/StallThiefPulse.java | 86 +++++- .../kotlin/content/StallGuardReactionTests.kt | 266 ++++++++++++++++++ 2 files changed, 346 insertions(+), 6 deletions(-) create mode 100644 Server/src/test/kotlin/content/StallGuardReactionTests.kt diff --git a/Server/src/main/content/global/skill/thieving/StallThiefPulse.java b/Server/src/main/content/global/skill/thieving/StallThiefPulse.java index b08185335..9dcf9e162 100644 --- a/Server/src/main/content/global/skill/thieving/StallThiefPulse.java +++ b/Server/src/main/content/global/skill/thieving/StallThiefPulse.java @@ -1,6 +1,8 @@ package content.global.skill.thieving; import core.game.event.ResourceProducedEvent; +import core.game.node.entity.combat.CombatPulse; +import core.game.node.entity.combat.CombatReach; import core.game.node.entity.combat.ImpactHandler; import core.game.node.entity.skill.SkillPulse; import core.game.node.entity.skill.Skills; @@ -11,6 +13,8 @@ import core.game.node.scenery.Scenery; import core.game.node.scenery.SceneryBuilder; import core.game.world.GameWorld; import core.game.world.map.RegionManager; +import core.game.world.map.path.Path; +import core.game.world.map.path.Pathfinder; import core.game.world.update.flag.context.Animation; import core.tools.RandomFunction; import core.tools.StringUtils; @@ -156,15 +160,85 @@ public final class StallThiefPulse extends SkillPulse { player.sendMessage("A higher power smites you"); return false; } - for (NPC npc : RegionManager.getLocalNpcs(player.getLocation(), 8)) { - if (!npc.getProperties().getCombatPulse().isAttacking() && (npc.getId() == 32 || npc.getId() == 2236)) { - npc.sendChat("Hey! Get your hands off there!"); - npc.getProperties().getCombatPulse().attack(player); - return false; - } + NPC guard = findGuardForFailedSteal(player); + if (guard != null) { + guard.sendChat("Hey! Get your hands off there!"); + guard.getProperties().getCombatPulse().attack(player); + return false; } } return true; } + /** + * Finds the guard that busts a failed steal. Guards whose attack can actually + * connect are preferred: NPC combat chase uses dumb pathing, so a guard on the far + * side of a stall (or inside the guardhouse next to the Ardougne market) would + * shout and then silently never reach the player. If no guard can reach, any guard + * within detection range still catches the thief - the steal must not succeed just + * because the witness is boxed in. + * @param player the thieving player. + * @return The catching guard, or {@code null} if no guard is in range. + */ + public static NPC findGuardForFailedSteal(Player player) { + NPC catcher = findCatchingGuard(player); + return catcher != null ? catcher : findWitnessGuard(player); + } + + /** + * Finds a guard within detection range whose attack can actually connect. + * @param player the thieving player. + * @return The catching guard, or {@code null} if no guard can reach the player. + */ + private static NPC findCatchingGuard(Player player) { + for (NPC npc : RegionManager.getLocalNpcs(player.getLocation(), 8)) { + if (npc.getProperties().getCombatPulse().isAttacking() || (npc.getId() != 32 && npc.getId() != 2236)) { + continue; + } + if (canReachThief(npc, player)) { + return npc; + } + } + return null; + } + + /** + * Finds any guard within detection range that witnesses the steal, even if it + * cannot reach the player. A guard already fighting someone else is too busy to + * notice, but one already (fruitlessly) chasing this player keeps catching them. + * @param player the thieving player. + * @return The witnessing guard, or {@code null} if none is in range. + */ + private static NPC findWitnessGuard(Player player) { + for (NPC npc : RegionManager.getLocalNpcs(player.getLocation(), 8)) { + if (npc.getId() != 32 && npc.getId() != 2236) { + continue; + } + CombatPulse pulse = npc.getProperties().getCombatPulse(); + if (pulse.isAttacking() && pulse.getVictim() != player) { + continue; + } + return npc; + } + return null; + } + + /** + * Checks whether the guard's attack can actually connect: either it is already in + * melee reach, or its (dumb) chase route reaches the player. + * @param npc the guard. + * @param player the thieving player. + * @return {@code True} if the guard can reach the player. + */ + private static boolean canReachThief(NPC npc, Player player) { + if (CombatReach.canMelee(npc, player, 1)) { + return true; + } + if (npc.getLocks().isMovementLocked()) { + return false; + } + Path path = Pathfinder.find(npc, player, true, Pathfinder.DUMB); + return path.isSuccessful() && !path.isMoveNear(); + } + } diff --git a/Server/src/test/kotlin/content/StallGuardReactionTests.kt b/Server/src/test/kotlin/content/StallGuardReactionTests.kt new file mode 100644 index 000000000..7fe53ee03 --- /dev/null +++ b/Server/src/test/kotlin/content/StallGuardReactionTests.kt @@ -0,0 +1,266 @@ +package content + +import MockSession +import TestUtils +import content.global.skill.thieving.StallThiefPulse +import content.global.skill.thieving.ThievingOptionPlugin +import core.game.node.entity.Entity +import core.game.node.entity.combat.CombatMovementIntents +import core.game.node.entity.npc.NPC +import core.game.node.entity.player.Player +import core.game.node.entity.skill.Skills +import core.game.world.map.Location +import core.game.world.map.RegionManager +import core.game.world.map.path.Pathfinder +import org.junit.jupiter.api.Assertions.* +import org.junit.jupiter.api.Test + +/** + * Regression tests for the Ardougne market stall mechanic: a failed steal must never + * fail silently. Any guard in range busts the steal; a guard whose attack can actually + * connect is preferred over one boxed in behind a stall or inside the guardhouse, so + * the shout is followed by a real attack whenever possible. + */ +class StallGuardReactionTests { + init { + TestUtils.preTestSetup() + } + + private fun findBakersStall(): core.game.node.scenery.Scenery { + core.game.world.map.Region.load(RegionManager.forId(10547)) + val bakerIds = setOf(2561, 6163, 34384) + for (x in 2650..2680) { + for (y in 3295..3325) { + val obj = RegionManager.getObject(0, x, y) ?: continue + if (obj.id in bakerIds) { + return obj + } + } + } + throw AssertionError("No baker's stall found in the Ardougne market square.") + } + + private fun adjacentWalkableTile(stall: core.game.node.scenery.Scenery): Location { + val base = stall.location + val candidates = ArrayList() + for (dx in -1..2) { + for (dy in -1..2) { + if (dx in 0..1 && dy in 0..1) continue + candidates.add(base.transform(dx, dy, 0)) + } + } + return candidates.firstOrNull { RegionManager.isTeleportPermitted(it) } + ?: throw AssertionError("No walkable tile adjacent to the stall at $base.") + } + + private fun place(entity: Entity, location: Location) { + entity.location = location + RegionManager.move(entity) + entity.walkingQueue.reset() + } + + private fun assertGuardAttacks(guard: NPC, player: Player, scenario: String) { + TestUtils.advanceTicks(30, false) + assertTrue( + player.inCombat(), + "[$scenario] Player should have been hit by the guard. " + + "guard=${guard.location}, attacking=${guard.properties.combatPulse.isAttacking}, " + + "player=${player.location}, ${CombatMovementIntents.lastResolveSummary()}", + ) + } + + private val movementBlockFlag = + Pathfinder.PREVENT_NORTH or + Pathfinder.PREVENT_EAST or + Pathfinder.PREVENT_SOUTH or + Pathfinder.PREVENT_WEST + + /** Finds an origin whose east and north arms (4 tiles each) are fully walkable. */ + private fun openCrossOrigin(): Location { + val start = Location.create(3200, 3600, 0) + for (dy in -16..16) { + for (dx in -16..16) { + val candidate = start.transform(dx, dy, 0) + val armsOpen = (0..4).all { + RegionManager.isTeleportPermitted(candidate.transform(it, 0, 0)) && + RegionManager.isTeleportPermitted(candidate.transform(0, it, 0)) + } + if (armsOpen) { + return candidate + } + } + } + throw AssertionError("No open cross-shaped area found near $start.") + } + + @Test + fun blockedGuardStillBustsTheStealButReachableGuardIsPreferred() { + TestUtils.getMockPlayer("stall_guard_selection").use { player -> + val origin = openCrossOrigin() + place(player, origin) + // Wall column two tiles east of the player, between them and the guard. + val wall = (-2..2).map { origin.transform(2, it, 0) } + val blockedGuard = NPC.create(32, origin.transform(4, 0, 0)) + try { + wall.forEach { RegionManager.addClippingFlag(it.z, it.x, it.y, false, movementBlockFlag) } + blockedGuard.init() + place(blockedGuard, origin.transform(4, 0, 0)) + + assertSame( + blockedGuard, + StallThiefPulse.findGuardForFailedSteal(player), + "A guard whose chase route is blocked must still bust the steal.", + ) + + val reachableGuard = NPC.create(32, origin.transform(0, 4, 0)) + try { + reachableGuard.init() + place(reachableGuard, origin.transform(0, 4, 0)) + assertSame( + reachableGuard, + StallThiefPulse.findGuardForFailedSteal(player), + "The reachable guard should be picked over the blocked one.", + ) + } finally { + reachableGuard.clear() + } + } finally { + wall.forEach { RegionManager.removeClippingFlag(it.z, it.x, it.y, false, movementBlockFlag) } + blockedGuard.clear() + } + } + } + + @Test + fun guardBusyFightingSomeoneElseDoesNotBustTheSteal() { + TestUtils.getMockPlayer("stall_guard_busy").use { player -> + TestUtils.getMockPlayer("stall_guard_other_victim").use { other -> + val origin = openCrossOrigin() + place(player, origin) + place(other, origin.transform(4, 1, 0)) + val guard = NPC.create(32, origin.transform(4, 0, 0)) + try { + guard.init() + place(guard, origin.transform(4, 0, 0)) + guard.properties.combatPulse.attack(other) + assertNull( + StallThiefPulse.findGuardForFailedSteal(player), + "A guard busy fighting someone else should not notice the thief.", + ) + guard.properties.combatPulse.stop() + assertSame( + guard, + StallThiefPulse.findGuardForFailedSteal(player), + "Once free, the same guard should bust the thief again.", + ) + } finally { + guard.clear() + CombatMovementIntents.clear() + } + } + } + } + + @Test + fun ardougneGuardShouldAttackPlayerCaughtStealing() { + TestUtils.getMockPlayer("stall_thief").use { player -> + val stall = findBakersStall() + place(player, adjacentWalkableTile(stall)) + player.skills.setStaticLevel(Skills.HITPOINTS, 99) + player.skills.lifepoints = 5000 + player.properties.isRetaliating = false + val guardSpawn = Location.create(2661, 3309, 0) + val guard = NPC.create(32, guardSpawn) + guard.init() + try { + place(guard, guardSpawn) + val caughtBy = StallThiefPulse.findGuardForFailedSteal(player) + assertNotNull(caughtBy, "A guard should be found near the stall.") + caughtBy!!.sendChat("Hey! Get your hands off there!") + caughtBy.properties.combatPulse.attack(player) + assertGuardAttacks(caughtBy, player, "open-path stall=${stall.location}") + } finally { + guard.clear() + CombatMovementIntents.clear() + } + } + } + + @Test + fun walkingGuardShouldStillAttackWhenCaught() { + TestUtils.getMockPlayer("stall_thief_walker").use { player -> + val stall = findBakersStall() + place(player, adjacentWalkableTile(stall)) + player.skills.setStaticLevel(Skills.HITPOINTS, 99) + player.skills.lifepoints = 5000 + player.properties.isRetaliating = false + val guardSpawn = Location.create(2661, 3309, 0) + val guard = NPC.create(32, guardSpawn) + guard.init() + try { + // Guard is mid-wander when the player gets caught. + guard.walkingQueue.reset(false) + guard.walkingQueue.addPath(guardSpawn.x - 3, guardSpawn.y) + TestUtils.advanceTicks(1, false) + guard.sendChat("Hey! Get your hands off there!") + guard.properties.combatPulse.attack(player) + assertGuardAttacks(guard, player, "walking-guard stall=${stall.location}") + } finally { + guard.clear() + CombatMovementIntents.clear() + } + } + } + + /** + * End-to-end: clicking Steal-from must start the thieving attempt from every + * walkable tile around the stall. A tile from which the click silently does + * nothing (no attempt, no message) reproduces the reported bug. + */ + @Test + fun stealFromClickShouldStartAttemptFromEveryNearbyTile() { + ThievingOptionPlugin().newInstance(null) + TestUtils.getMockPlayer("stall_click_thief").use { player -> + val stall = findBakersStall() + player.skills.setStaticLevel(Skills.THIEVING, 99) + player.skills.setLevel(Skills.THIEVING, 99) + player.skills.setStaticLevel(Skills.HITPOINTS, 99) + player.properties.isRetaliating = false + val optIndex = stall.interaction.options.indexOfFirst { + it != null && it.name.equals("steal-from", ignoreCase = true) + } + assertTrue(optIndex >= 0, "Stall has no Steal-from option: ${stall.interaction.options?.map { it?.name }}") + + val base = stall.location + val silent = ArrayList() + var attempts = 0 + for (dx in -2..3) { + for (dy in -2..3) { + if (dx in 0..1 && dy in 0..1) continue // stall footprint + val tile = base.transform(dx, dy, 0) + if (!RegionManager.isTeleportPermitted(tile)) continue + place(player, tile) + player.removeAttribute("thieveDelay") + player.removeAttribute("combat-time") + player.properties.combatPulse.stop() + player.inventory.clear() + player.skills.lifepoints = 5000 + attempts++ + val session = player.session as MockSession + session.receivedPackets.clear() + TestUtils.simulateInteraction(player, stall, optIndex) + TestUtils.advanceTicks(10, false) + if (player.getAttribute("thieveDelay", null) == null) { + silent.add(tile) + } + TestUtils.advanceTicks(10, false) // let locks/stall respawn settle + } + } + assertTrue(attempts > 0, "No walkable tiles found around the stall.") + assertTrue( + silent.isEmpty(), + "Steal-from click did nothing from ${silent.size}/$attempts tiles: $silent (stall=$base)", + ) + } + } +}