2009Scape/Server
1
0
Fork 1
mirror of https://gitlab.com/2009scape/2009scape.git synced 2026-08-23 19:35:10 -06:00
Code Issues Projects Releases Packages Wiki Activity

Merge branch 'familiar-call-loop-fix' into 'master'

Browse source 
Fixed "familiar/pet permanently gone until relog" bugs

Closes #1471 and #2313

See merge request 2009scape/2009scape!2477
This commit is contained in:
dam 2026-08-01 17:42:34 +03:00
commit d6f12310e7
5 changed files with 256 additions and 53 deletions
Download patch file Download diff file Expand all files Collapse all files

22
Server/src/main/content/global/skill/construction/HouseManager.java
View file

@ -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()));
}
/**

103
Server/src/main/content/global/skill/construction/HouseZone.java
View file

@ -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,48 +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;
}
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");
@ -148,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);

43
Server/src/main/content/global/skill/summoning/familiar/Familiar.java
View file

@ -393,10 +393,17 @@ public abstract class Familiar extends NPC implements Plugin<Object> {
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
@ -703,10 +710,20 @@ public abstract class Familiar extends NPC implements Plugin<Object> {
//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) {
@ -727,13 +744,33 @@ public abstract class Familiar extends NPC implements Plugin<Object> {
} 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.

27
Server/src/main/core/game/world/map/RegionManager.kt
View file

@ -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
}
}
@ -903,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.

114
Server/src/test/kotlin/content/global/skill/construction/HouseRegionLifecycleTests.kt Normal file
View file

@ -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)
}
}