From 8f855105b96a662ad2105de4decc08d40de92a78 Mon Sep 17 00:00:00 2001 From: dam <27978131-real_damighty@users.noreply.gitlab.com> Date: Sat, 4 Jul 2026 03:33:17 +0300 Subject: [PATCH 1/4] Cats on bridges, no more infinite familiar call loops --- .../global/skill/construction/HouseZone.java | 8 ++++ .../skill/summoning/familiar/Familiar.java | 43 +++++++++++++++++-- .../main/core/game/world/map/RegionManager.kt | 8 +++- 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/Server/src/main/content/global/skill/construction/HouseZone.java b/Server/src/main/content/global/skill/construction/HouseZone.java index 31a57f6f8..992640811 100644 --- a/Server/src/main/content/global/skill/construction/HouseZone.java +++ b/Server/src/main/content/global/skill/construction/HouseZone.java @@ -115,6 +115,14 @@ public final class HouseZone extends MapZone { return true; } } + + // Take the player's familiar along before the teardown pulse below wipes every NPC + // still left inside the house - a wiped familiar is gone until relog (GL #2313). The + // call either materializes it at the destination or parks it invisibly at the owner's + // side until there is room for it. + if (!logout && p.getFamiliarManager().hasFamiliar()) { + p.getFamiliarManager().getFamiliar().call(); + } remove_items(p); // The below tears down the house if the owner was the one who left diff --git a/Server/src/main/content/global/skill/summoning/familiar/Familiar.java b/Server/src/main/content/global/skill/summoning/familiar/Familiar.java index b561a5b30..c4f43cdef 100644 --- a/Server/src/main/content/global/skill/summoning/familiar/Familiar.java +++ b/Server/src/main/content/global/skill/summoning/familiar/Familiar.java @@ -391,10 +391,17 @@ public abstract class Familiar extends NPC implements Plugin { return false; } - + @Override public void onRegionInactivity() { call(); + // NPC.tick() short-circuits into this method while the region is inactive, so Entity.tick() + // never runs to consume a teleport that call() just set; apply it manually here or the + // familiar re-calls forever without ever moving (GL #2313). + if (getProperties().getTeleportLocation() != null && owner.isActive()) { + getWalkingQueue().update(); + getUpdateMasks().prepare(this); + } } @Override @@ -699,10 +706,20 @@ public abstract class Familiar extends NPC implements Plugin { //owner.getPacketDispatch().sendMessage("Your familiar is too big to fit here. Try calling it again when you are standing"); //owner.getPacketDispatch().sendMessage("somewhere with more space."); //spamTimer = 50; + // No room to materialize, so wait invisibly at the owner's side (like init() does when + // there is no room at summon time). Waiting in place instead means being left behind: + // once the abandoned region unloads, the familiar either gets wiped with it + // (DynamicRegion teardown) or gets stuck endlessly re-calling from an inactive region + // that no longer ticks it normally (GL #2313). + setInvisible(true); + stayWithOwner(); return false; } setInvisible(owner.getZoneMonitor().isRestricted(ZoneRestriction.FOLLOWERS) && !owner.getLocks().isLocked("enable_summoning")); - if (isInvisible()) return true; + if (isInvisible()) { + stayWithOwner(); + return true; + } getProperties().setTeleportLocation(destination); if (!(this instanceof Pet)) { if (firstCall) { @@ -723,13 +740,33 @@ public abstract class Familiar extends NPC implements Plugin { } else { face(owner); } - if (!isRenderable() && owner.isActive()) { + // A familiar pulled in from another region must move immediately rather than on its next + // tick: NPCs tick before players, so when this call runs from the owner's movement (e.g. + // leaving a POH), the familiar's tick for this cycle has already passed - and a region + // teardown pulse (POH exit portal, building-mode rebuild) runs at the start of the next + // cycle, wiping any NPC still inside before it can consume a pending teleport (GL #2313). + boolean leftInAnotherRegion = getLocation().getRegionId() != owner.getLocation().getRegionId(); + if ((!isRenderable() || leftInAnotherRegion) && owner.isActive()) { // log(this.getClass(), Log.ERR, "Familiar in inactive region!"); getWalkingQueue().update(); getUpdateMasks().prepare(this); } return true; } + + /** + * Moves a familiar that cannot materialize to its owner's location, so it is never left + * behind in a region that may unload. The teleport is applied immediately because this can + * run from an inactive region, where Entity.tick() (the normal consumer) no longer runs. + */ + private void stayWithOwner() { + if (!owner.isActive() || getLocation().equals(owner.getLocation())) { + return; + } + getProperties().setTeleportLocation(owner.getLocation()); + getWalkingQueue().update(); + getUpdateMasks().prepare(this); + } /** * Gets the spawning location of the familiar. diff --git a/Server/src/main/core/game/world/map/RegionManager.kt b/Server/src/main/core/game/world/map/RegionManager.kt index 9fb7b848b..309c582a9 100644 --- a/Server/src/main/core/game/world/map/RegionManager.kt +++ b/Server/src/main/core/game/world/map/RegionManager.kt @@ -332,10 +332,14 @@ object RegionManager { } } val l = owner.location.transform(stepX, stepY, 0) - // Check if ALL target tiles are unclipped + // Check if the NPC can stand on ALL target tiles. This must not use isClipped(), + // which rejects any tile that has a wall along ANY of its sides: every tile of a + // railed bridge deck has one (e.g. the whole Lumbridge bridge), which made familiars + // and pets unable to materialize anywhere on it (GL #1471). Materializing is the + // same question as teleporting, so ask the teleport predicate. for (x in 0 until node.size()) { for (y in 0 until node.size()) { - if (isClipped(l.transform(x, y, 0))) { + if (!isTeleportPermitted(l.transform(x, y, 0))) { continue@outer } } From bf3953325e904c99d85b8262b325f65cc3b45f63 Mon Sep 17 00:00:00 2001 From: dam <27978131-real_damighty@users.noreply.gitlab.com> Date: Mon, 20 Jul 2026 12:34:35 +0300 Subject: [PATCH 2/4] Small teardown tweaks for familiar POH entry --- .../skill/construction/HouseManager.java | 19 ++++++- .../global/skill/construction/HouseZone.java | 19 +++---- .../main/core/game/world/map/RegionManager.kt | 17 ++++++ .../construction/HouseRegionLifecycleTests.kt | 57 +++++++++++++++++++ 4 files changed, 100 insertions(+), 12 deletions(-) create mode 100644 Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt diff --git a/Server/src/main/content/global/skill/construction/HouseManager.java b/Server/src/main/content/global/skill/construction/HouseManager.java index 4db8e8191..eebcdd604 100644 --- a/Server/src/main/content/global/skill/construction/HouseManager.java +++ b/Server/src/main/content/global/skill/construction/HouseManager.java @@ -43,6 +43,11 @@ public final class HouseManager { */ private DynamicRegion dungeonRegion; + /** + * If the current regions have been handed off to delayed teardown. + */ + private boolean teardownPending; + /** * The house location. */ @@ -368,9 +373,21 @@ public final class HouseManager { } ZoneBuilder.configure(zone); + teardownPending = false; return houseRegion; } + /** + * Marks the current regions as unavailable for new entrants and returns the + * exact instances that the delayed teardown must remove. + * + * @return The current house region followed by its optional dungeon region. + */ + Region[] beginTeardown() { + teardownPending = true; + return new Region[] { houseRegion, dungeonRegion }; + } + private DynamicRegion getPreparedRegion() { ZoneBorders borders = DynamicRegion.reserveArea(8,8); DynamicRegion region = new DynamicRegion(-1, borders.getSouthWestX() >> 6, borders.getSouthWestY() >> 6); @@ -708,7 +725,7 @@ public final class HouseManager { // return (houseRegion != null) || (dungeonRegion != null); //} public boolean isLoaded() { - return (houseRegion != null && houseRegion.isActive()) || (dungeonRegion != null && dungeonRegion.isActive()); + return !teardownPending && ((houseRegion != null && houseRegion.isActive()) || (dungeonRegion != null && dungeonRegion.isActive())); } /** diff --git a/Server/src/main/content/global/skill/construction/HouseZone.java b/Server/src/main/content/global/skill/construction/HouseZone.java index 992640811..1c41001a6 100644 --- a/Server/src/main/content/global/skill/construction/HouseZone.java +++ b/Server/src/main/content/global/skill/construction/HouseZone.java @@ -128,19 +128,16 @@ public final class HouseZone extends MapZone { // The below tears down the house if the owner was the one who left if (house == p.getHouseManager()) { house.expelGuests(p); - int toRemove = previousRegion; - int dungRemove = previousDungeon; + Region[] regions = house.beginTeardown(); + Region houseToRemove = regions[0]; + Region dungeonToRemove = regions[1]; submitWorldPulse(new Pulse(1) { public boolean pulse() { - Region r = RegionManager.forId(toRemove); - Region dr = dungRemove != -1 ? RegionManager.forId(dungRemove) : null; - RegionManager.removeRegion(toRemove); - unregisterRegion(toRemove); - r.flagInactive(); - if (dungRemove != -1) { - RegionManager.removeRegion(dungRemove); - unregisterRegion(dungRemove); - dr.flagInactive(); + RegionManager.removeRegion(houseToRemove); + unregisterRegion(houseToRemove.getId()); + if (dungeonToRemove != null) { + RegionManager.removeRegion(dungeonToRemove); + unregisterRegion(dungeonToRemove.getId()); } return true; } diff --git a/Server/src/main/core/game/world/map/RegionManager.kt b/Server/src/main/core/game/world/map/RegionManager.kt index 309c582a9..ba5e31ccb 100644 --- a/Server/src/main/core/game/world/map/RegionManager.kt +++ b/Server/src/main/core/game/world/map/RegionManager.kt @@ -907,6 +907,23 @@ object RegionManager { } } + /** + * Removes a region only if it is still the cached instance for its id. + * + * Delayed cleanup tasks must use this overload so they cannot remove a newer + * dynamic region that has since been assigned the same coordinates. + */ + @JvmStatic + fun removeRegion(region: Region) { + if (lock.tryLock() || LOCK.tryLock(10000, TimeUnit.MILLISECONDS)) { + if (REGION_CACHE[region.id] === region) { + REGION_CACHE.remove(region.id) + } + region.flagInactive(true) + LOCK.unlock() + } + } + /** * Gets the regionCache. * @return The regionCache. diff --git a/Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt b/Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt new file mode 100644 index 000000000..f61321265 --- /dev/null +++ b/Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt @@ -0,0 +1,57 @@ +package content.global.skill.construction + +import TestUtils +import core.game.world.map.Region +import core.game.world.map.RegionManager +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotSame +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +class HouseRegionLifecycleTests { + companion object { + init { + TestUtils.preTestSetup() + } + } + + @Test + fun regionPendingTeardownIsNotReusedByImmediateReentry() { + TestUtils.getMockPlayer("house-region-reentry").use { player -> + val house = player.houseManager + house.createNewHouseAt(HouseLocation.RIMMINGTON) + val oldRegion = house.construct() + + val regionsToRemove = house.beginTeardown() + + assertSame(oldRegion, regionsToRemove[0]) + assertFalse(house.isLoaded) + + house.preEnter(player, false) + val replacement = house.houseRegion + + assertNotSame(oldRegion, replacement) + assertTrue(house.isLoaded) + RegionManager.removeRegion(oldRegion) + assertSame(replacement, house.houseRegion) + assertTrue(replacement.isActive) + + RegionManager.removeRegion(replacement) + } + } + + @Test + fun delayedRemovalDoesNotEvictAReplacementAtTheSameRegionId() { + val house = HouseManager() + house.createNewHouseAt(HouseLocation.RIMMINGTON) + val oldRegion = house.construct() + val replacement = Region(oldRegion.x, oldRegion.y) + RegionManager.addRegion(replacement.id, replacement) + + RegionManager.removeRegion(oldRegion) + + assertSame(replacement, RegionManager.regionCache[replacement.id]) + RegionManager.removeRegion(replacement.id) + } +} From 5a863732e9587ace2c2ed9780fd28a380c39e244 Mon Sep 17 00:00:00 2001 From: dam <27978131-real_damighty@users.noreply.gitlab.com> Date: Mon, 20 Jul 2026 14:19:49 +0300 Subject: [PATCH 3/4] Revert "Small teardown tweaks for familiar POH entry" This reverts commit bf3953325e904c99d85b8262b325f65cc3b45f63. --- .../skill/construction/HouseManager.java | 19 +------ .../global/skill/construction/HouseZone.java | 19 ++++--- .../main/core/game/world/map/RegionManager.kt | 17 ------ .../construction/HouseRegionLifecycleTests.kt | 57 ------------------- 4 files changed, 12 insertions(+), 100 deletions(-) delete mode 100644 Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt diff --git a/Server/src/main/content/global/skill/construction/HouseManager.java b/Server/src/main/content/global/skill/construction/HouseManager.java index eebcdd604..4db8e8191 100644 --- a/Server/src/main/content/global/skill/construction/HouseManager.java +++ b/Server/src/main/content/global/skill/construction/HouseManager.java @@ -43,11 +43,6 @@ public final class HouseManager { */ private DynamicRegion dungeonRegion; - /** - * If the current regions have been handed off to delayed teardown. - */ - private boolean teardownPending; - /** * The house location. */ @@ -373,21 +368,9 @@ public final class HouseManager { } ZoneBuilder.configure(zone); - teardownPending = false; return houseRegion; } - /** - * Marks the current regions as unavailable for new entrants and returns the - * exact instances that the delayed teardown must remove. - * - * @return The current house region followed by its optional dungeon region. - */ - Region[] beginTeardown() { - teardownPending = true; - return new Region[] { houseRegion, dungeonRegion }; - } - private DynamicRegion getPreparedRegion() { ZoneBorders borders = DynamicRegion.reserveArea(8,8); DynamicRegion region = new DynamicRegion(-1, borders.getSouthWestX() >> 6, borders.getSouthWestY() >> 6); @@ -725,7 +708,7 @@ public final class HouseManager { // return (houseRegion != null) || (dungeonRegion != null); //} public boolean isLoaded() { - return !teardownPending && ((houseRegion != null && houseRegion.isActive()) || (dungeonRegion != null && dungeonRegion.isActive())); + return (houseRegion != null && houseRegion.isActive()) || (dungeonRegion != null && dungeonRegion.isActive()); } /** diff --git a/Server/src/main/content/global/skill/construction/HouseZone.java b/Server/src/main/content/global/skill/construction/HouseZone.java index 1c41001a6..992640811 100644 --- a/Server/src/main/content/global/skill/construction/HouseZone.java +++ b/Server/src/main/content/global/skill/construction/HouseZone.java @@ -128,16 +128,19 @@ public final class HouseZone extends MapZone { // The below tears down the house if the owner was the one who left if (house == p.getHouseManager()) { house.expelGuests(p); - Region[] regions = house.beginTeardown(); - Region houseToRemove = regions[0]; - Region dungeonToRemove = regions[1]; + int toRemove = previousRegion; + int dungRemove = previousDungeon; submitWorldPulse(new Pulse(1) { public boolean pulse() { - RegionManager.removeRegion(houseToRemove); - unregisterRegion(houseToRemove.getId()); - if (dungeonToRemove != null) { - RegionManager.removeRegion(dungeonToRemove); - unregisterRegion(dungeonToRemove.getId()); + Region r = RegionManager.forId(toRemove); + Region dr = dungRemove != -1 ? RegionManager.forId(dungRemove) : null; + RegionManager.removeRegion(toRemove); + unregisterRegion(toRemove); + r.flagInactive(); + if (dungRemove != -1) { + RegionManager.removeRegion(dungRemove); + unregisterRegion(dungRemove); + dr.flagInactive(); } return true; } diff --git a/Server/src/main/core/game/world/map/RegionManager.kt b/Server/src/main/core/game/world/map/RegionManager.kt index ba5e31ccb..309c582a9 100644 --- a/Server/src/main/core/game/world/map/RegionManager.kt +++ b/Server/src/main/core/game/world/map/RegionManager.kt @@ -907,23 +907,6 @@ object RegionManager { } } - /** - * Removes a region only if it is still the cached instance for its id. - * - * Delayed cleanup tasks must use this overload so they cannot remove a newer - * dynamic region that has since been assigned the same coordinates. - */ - @JvmStatic - fun removeRegion(region: Region) { - if (lock.tryLock() || LOCK.tryLock(10000, TimeUnit.MILLISECONDS)) { - if (REGION_CACHE[region.id] === region) { - REGION_CACHE.remove(region.id) - } - region.flagInactive(true) - LOCK.unlock() - } - } - /** * Gets the regionCache. * @return The regionCache. diff --git a/Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt b/Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt deleted file mode 100644 index f61321265..000000000 --- a/Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt +++ /dev/null @@ -1,57 +0,0 @@ -package content.global.skill.construction - -import TestUtils -import core.game.world.map.Region -import core.game.world.map.RegionManager -import org.junit.jupiter.api.Assertions.assertFalse -import org.junit.jupiter.api.Assertions.assertNotSame -import org.junit.jupiter.api.Assertions.assertSame -import org.junit.jupiter.api.Assertions.assertTrue -import org.junit.jupiter.api.Test - -class HouseRegionLifecycleTests { - companion object { - init { - TestUtils.preTestSetup() - } - } - - @Test - fun regionPendingTeardownIsNotReusedByImmediateReentry() { - TestUtils.getMockPlayer("house-region-reentry").use { player -> - val house = player.houseManager - house.createNewHouseAt(HouseLocation.RIMMINGTON) - val oldRegion = house.construct() - - val regionsToRemove = house.beginTeardown() - - assertSame(oldRegion, regionsToRemove[0]) - assertFalse(house.isLoaded) - - house.preEnter(player, false) - val replacement = house.houseRegion - - assertNotSame(oldRegion, replacement) - assertTrue(house.isLoaded) - RegionManager.removeRegion(oldRegion) - assertSame(replacement, house.houseRegion) - assertTrue(replacement.isActive) - - RegionManager.removeRegion(replacement) - } - } - - @Test - fun delayedRemovalDoesNotEvictAReplacementAtTheSameRegionId() { - val house = HouseManager() - house.createNewHouseAt(HouseLocation.RIMMINGTON) - val oldRegion = house.construct() - val replacement = Region(oldRegion.x, oldRegion.y) - RegionManager.addRegion(replacement.id, replacement) - - RegionManager.removeRegion(oldRegion) - - assertSame(replacement, RegionManager.regionCache[replacement.id]) - RegionManager.removeRegion(replacement.id) - } -} From 5d5354dc32f48de4fd200384fa92ad0783715be3 Mon Sep 17 00:00:00 2001 From: dam <27978131-real_damighty@users.noreply.gitlab.com> Date: Mon, 20 Jul 2026 16:11:57 +0300 Subject: [PATCH 4/4] Small teardown tweaks for familiar POH entry --- .../skill/construction/HouseManager.java | 22 +++- .../global/skill/construction/HouseZone.java | 109 ++++++++--------- .../main/core/game/world/map/RegionManager.kt | 19 +++ .../construction/HouseRegionLifecycleTests.kt | 114 ++++++++++++++++++ 4 files changed, 209 insertions(+), 55 deletions(-) create mode 100644 Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt diff --git a/Server/src/main/content/global/skill/construction/HouseManager.java b/Server/src/main/content/global/skill/construction/HouseManager.java index 4db8e8191..4f63bf279 100644 --- a/Server/src/main/content/global/skill/construction/HouseManager.java +++ b/Server/src/main/content/global/skill/construction/HouseManager.java @@ -43,6 +43,11 @@ public final class HouseManager { */ private DynamicRegion dungeonRegion; + /** + * If the current regions have been handed off to delayed teardown. + */ + private boolean teardownPending; + /** * The house location. */ @@ -368,9 +373,24 @@ public final class HouseManager { } ZoneBuilder.configure(zone); + teardownPending = false; return houseRegion; } + /** + * Marks the current regions as unavailable while their delayed teardown is pending. + * Stale regions from a house rebuild must not change the lifecycle of the replacement + * regions now referenced by this manager. + * + * @param houseToRemove The house region being left. + * @param dungeonToRemove The dungeon region paired with the house, if any. + */ + void beginTeardown(Region houseToRemove, Region dungeonToRemove) { + if (houseRegion == houseToRemove && dungeonRegion == dungeonToRemove) { + teardownPending = true; + } + } + private DynamicRegion getPreparedRegion() { ZoneBorders borders = DynamicRegion.reserveArea(8,8); DynamicRegion region = new DynamicRegion(-1, borders.getSouthWestX() >> 6, borders.getSouthWestY() >> 6); @@ -708,7 +728,7 @@ public final class HouseManager { // return (houseRegion != null) || (dungeonRegion != null); //} public boolean isLoaded() { - return (houseRegion != null && houseRegion.isActive()) || (dungeonRegion != null && dungeonRegion.isActive()); + return !teardownPending && ((houseRegion != null && houseRegion.isActive()) || (dungeonRegion != null && dungeonRegion.isActive())); } /** diff --git a/Server/src/main/content/global/skill/construction/HouseZone.java b/Server/src/main/content/global/skill/construction/HouseZone.java index 992640811..647e8d3ff 100644 --- a/Server/src/main/content/global/skill/construction/HouseZone.java +++ b/Server/src/main/content/global/skill/construction/HouseZone.java @@ -2,7 +2,6 @@ package content.global.skill.construction; import core.api.Container; -import core.game.world.map.Location; import org.rs09.consts.Items; import core.game.node.entity.Entity; import core.game.node.entity.player.Player; @@ -27,14 +26,14 @@ public final class HouseZone extends MapZone { private HouseManager house; /** - * The previous house region id. + * The house region occupied before the latest transition. */ - private int previousRegion = -1; + private Region previousRegion; /** - * The previous dungeon region id. + * The dungeon region paired with the previous house region. */ - private int previousDungeon = -1; + private Region previousDungeon; /** * Constructs the house zone object. @@ -55,11 +54,11 @@ public final class HouseZone extends MapZone { } private void unregisterOldRegions() { - if (previousRegion != -1) { - unregisterRegion(previousRegion); + if (previousRegion != null && RegionManager.getRegionCache().get(previousRegion.getId()) == previousRegion) { + unregisterRegion(previousRegion.getId()); } - if (previousDungeon != -1) { - unregisterRegion(previousDungeon); + if (previousDungeon != null && RegionManager.getRegionCache().get(previousDungeon.getId()) == previousDungeon) { + unregisterRegion(previousDungeon.getId()); } } @@ -68,9 +67,8 @@ public final class HouseZone extends MapZone { if (e instanceof Player) { Player pl = (Player) e; if (house == pl.getHouseManager()) { - previousRegion = house.getHouseRegion().getId(); - if (house.getDungeonRegion() != null) - previousDungeon = house.getDungeonRegion().getId(); + previousRegion = house.getHouseRegion(); + previousDungeon = house.getDungeonRegion(); } registerLogoutListener(pl, "houselogout", (p) -> { p.setLocation(house.getLocation().getExitLocation()); @@ -95,56 +93,29 @@ public final class HouseZone extends MapZone { if (e instanceof Player) { Player p = (Player) e; - // if the player is moving between the dungeon and main house, don't trigger the leave sequence - if (house.getDungeonRegion() != null && !logout) { - // current loc - Location dest = p.getProperties().getTeleportLocation(); - int currentRegionId = p.getLocation().getRegionId(); - int houseId = house.getHouseRegion().getId(); - int dungeonId = house.getDungeonRegion().getId(); - - // check if still in house - boolean currentlyInHouse = (currentRegionId == houseId || currentRegionId == dungeonId); - - // check if moving between house regions - int destRegionId = (dest != null) ? dest.getRegionId() : -1; - boolean movingToHouse = (destRegionId == houseId || destRegionId == dungeonId); - - // if yes, return true and stop the leave sequence - if (currentlyInHouse || movingToHouse) { - return true; + // RegionManager moves the player into the destination region before notifying the old + // zone. Remaining in this manager's current regions is therefore an internal move, not + // a real house exit. When the region instances changed, the move is a house rebuild + // (such as a building-mode toggle), so only the stale instances should be torn down. + if (!logout && (p.getViewport().getRegion() == house.getHouseRegion() + || p.getViewport().getRegion() == house.getDungeonRegion())) { + if (previousRegion != house.getHouseRegion() || previousDungeon != house.getDungeonRegion()) { + callFamiliar(p); + submitRegionRemoval(previousRegion, previousDungeon); } + return true; } - - // Take the player's familiar along before the teardown pulse below wipes every NPC - // still left inside the house - a wiped familiar is gone until relog (GL #2313). The - // call either materializes it at the destination or parks it invisibly at the owner's - // side until there is room for it. - if (!logout && p.getFamiliarManager().hasFamiliar()) { - p.getFamiliarManager().getFamiliar().call(); + + if (!logout) { + callFamiliar(p); } remove_items(p); // The below tears down the house if the owner was the one who left if (house == p.getHouseManager()) { house.expelGuests(p); - int toRemove = previousRegion; - int dungRemove = previousDungeon; - submitWorldPulse(new Pulse(1) { - public boolean pulse() { - Region r = RegionManager.forId(toRemove); - Region dr = dungRemove != -1 ? RegionManager.forId(dungRemove) : null; - RegionManager.removeRegion(toRemove); - unregisterRegion(toRemove); - r.flagInactive(); - if (dungRemove != -1) { - RegionManager.removeRegion(dungRemove); - unregisterRegion(dungRemove); - dr.flagInactive(); - } - return true; - } - }); + house.beginTeardown(previousRegion, previousDungeon); + submitRegionRemoval(previousRegion, previousDungeon); } // Clear logout listener and original-loc (if appropriate) clearLogoutListener(p, "houselogout"); @@ -156,6 +127,36 @@ public final class HouseZone extends MapZone { return true; } + /** + * Takes the player's familiar along before a stale house region is cleared. + */ + private void callFamiliar(Player player) { + if (player.getFamiliarManager().hasFamiliar()) { + player.getFamiliarManager().getFamiliar().call(); + } + } + + /** + * Removes only the exact region instances captured while leaving. This prevents delayed + * cleanup from evicting a replacement region that has since reused the same coordinates. + */ + private void submitRegionRemoval(Region houseToRemove, Region dungeonToRemove) { + if (houseToRemove == null && dungeonToRemove == null) { + return; + } + submitWorldPulse(new Pulse(1) { + public boolean pulse() { + if (houseToRemove != null) { + RegionManager.removeRegion(houseToRemove); + } + if (dungeonToRemove != null) { + RegionManager.removeRegion(dungeonToRemove); + } + return true; + } + }); + } + private void remove_items(Player p) { for (int item = Items.KETTLE_7688; item <= Items.CHEFS_DELIGHT_7755; item++) {//Removes all PoH versions of tea and beer barrel-related items removeAll(p, item, Container.INVENTORY); diff --git a/Server/src/main/core/game/world/map/RegionManager.kt b/Server/src/main/core/game/world/map/RegionManager.kt index 309c582a9..8bc290b6e 100644 --- a/Server/src/main/core/game/world/map/RegionManager.kt +++ b/Server/src/main/core/game/world/map/RegionManager.kt @@ -907,6 +907,25 @@ object RegionManager { } } + /** + * Removes a region only if it is still the cached instance for its id. + * + * Delayed cleanup tasks must use this overload so they cannot remove a newer + * dynamic region that has since been assigned the same coordinates. + */ + @JvmStatic + fun removeRegion(region: Region) { + if (lock.tryLock() || LOCK.tryLock(10000, TimeUnit.MILLISECONDS)) { + if (REGION_CACHE[region.id] === region) { + REGION_CACHE.remove(region.id) + } + if (region.isActive) { + region.flagInactive(true) + } + LOCK.unlock() + } + } + /** * Gets the regionCache. * @return The regionCache. diff --git a/Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt b/Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt new file mode 100644 index 000000000..eadc48042 --- /dev/null +++ b/Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt @@ -0,0 +1,114 @@ +package content.global.skill.construction + +import TestUtils +import content.global.skill.summoning.familiar.SteelTitanNPC +import core.game.world.map.Region +import core.game.world.map.RegionManager +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotSame +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +class HouseRegionLifecycleTests { + companion object { + init { + TestUtils.preTestSetup() + } + } + + @Test + fun buildingModeRebuildKeepsReplacementHouseLoaded() { + TestUtils.getMockPlayer("house-build-rebuild").use { player -> + val house = player.houseManager + val familiar = SteelTitanNPC(player, 7343) + player.familiarManager.familiar = familiar + familiar.init() + house.createNewHouseAt(HouseLocation.RIMMINGTON) + house.enter(player, false) + player.walkingQueue.update() + familiar.call() + + val normalRegion = house.houseRegion + assertTrue(house.isInHouse(player)) + assertTrue(familiar.isActive) + assertSame(normalRegion, familiar.viewport.region) + + house.toggleBuildingMode(player, true) + val buildRegion = house.houseRegion + assertNotSame(normalRegion, buildRegion) + player.walkingQueue.update() + TestUtils.advanceTicks(2, false) + + assertTrue(house.isBuildingMode) + assertTrue(house.isLoaded) + assertTrue(house.isInHouse(player)) + assertFalse(normalRegion.isActive) + assertSame(buildRegion, RegionManager.regionCache[buildRegion.id]) + assertTrue(familiar.isActive) + assertSame(buildRegion, familiar.viewport.region) + + house.toggleBuildingMode(player, false) + val replacementNormalRegion = house.houseRegion + assertNotSame(buildRegion, replacementNormalRegion) + player.walkingQueue.update() + TestUtils.advanceTicks(2, false) + + assertFalse(house.isBuildingMode) + assertTrue(house.isLoaded) + assertTrue(house.isInHouse(player)) + assertFalse(buildRegion.isActive) + assertSame(replacementNormalRegion, RegionManager.regionCache[replacementNormalRegion.id]) + assertTrue(familiar.isActive) + assertSame(replacementNormalRegion, familiar.viewport.region) + + HouseManager.leave(player) + player.walkingQueue.update() + TestUtils.advanceTicks(2, false) + } + } + + @Test + fun regionPendingTeardownIsNotReusedByImmediateReentry() { + TestUtils.getMockPlayer("house-region-reentry").use { player -> + val house = player.houseManager + house.createNewHouseAt(HouseLocation.RIMMINGTON) + house.enter(player, false) + player.walkingQueue.update() + val oldRegion = house.houseRegion + + HouseManager.leave(player) + player.walkingQueue.update() + assertFalse(house.isLoaded) + + house.enter(player, false) + val replacement = house.houseRegion + assertNotSame(oldRegion, replacement) + player.walkingQueue.update() + TestUtils.advanceTicks(2, false) + + assertTrue(house.isLoaded) + assertTrue(house.isInHouse(player)) + assertFalse(oldRegion.isActive) + assertSame(replacement, RegionManager.regionCache[replacement.id]) + + HouseManager.leave(player) + player.walkingQueue.update() + TestUtils.advanceTicks(2, false) + } + } + + @Test + fun delayedRemovalDoesNotEvictReplacementAtSameRegionId() { + val house = HouseManager() + house.createNewHouseAt(HouseLocation.RIMMINGTON) + val oldRegion = house.construct() + val replacement = Region(oldRegion.x, oldRegion.y) + RegionManager.addRegion(replacement.id, replacement) + + RegionManager.removeRegion(oldRegion) + + assertSame(replacement, RegionManager.regionCache[replacement.id]) + RegionManager.removeRegion(replacement.id) + } +}