CME from Spawner Agitator
jeremiahwinsley opened this issue ยท 2 comments
Describe the bug
Unloaded spawner apparently tries to load chunks, resulting in a CME.
How to reproduce the bug
Unknown
Expected behavior
Spawner should check whether the BlockPos is loaded.
Additional details
This may have other side-effects I'm not aware of, but I was able to start the server successfully with this patch:
Index: src/main/java/me/desht/pneumaticcraft/common/entity/semiblock/AbstractSemiblockEntity.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/java/me/desht/pneumaticcraft/common/entity/semiblock/AbstractSemiblockEntity.java b/src/main/java/me/desht/pneumaticcraft/common/entity/semiblock/AbstractSemiblockEntity.java
--- a/src/main/java/me/desht/pneumaticcraft/common/entity/semiblock/AbstractSemiblockEntity.java (revision f7fc44679703b3eaf137f2e6433e66526b2f8f18)
+++ b/src/main/java/me/desht/pneumaticcraft/common/entity/semiblock/AbstractSemiblockEntity.java (date 1649376620726)
@@ -207,7 +207,7 @@
@Override
public BlockEntity getCachedTileEntity() {
- if (cachedTE == null || cachedTE.isRemoved()) {
+ if ((cachedTE == null || cachedTE.isRemoved()) && level.isLoaded(blockPos)) {
cachedTE = level.getBlockEntity(blockPos);
}
return cachedTE;
Index: src/main/java/me/desht/pneumaticcraft/common/entity/semiblock/SpawnerAgitatorEntity.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/java/me/desht/pneumaticcraft/common/entity/semiblock/SpawnerAgitatorEntity.java b/src/main/java/me/desht/pneumaticcraft/common/entity/semiblock/SpawnerAgitatorEntity.java
--- a/src/main/java/me/desht/pneumaticcraft/common/entity/semiblock/SpawnerAgitatorEntity.java (revision f7fc44679703b3eaf137f2e6433e66526b2f8f18)
+++ b/src/main/java/me/desht/pneumaticcraft/common/entity/semiblock/SpawnerAgitatorEntity.java (date 1649376903948)
@@ -45,7 +45,7 @@
// just altering the range when added isn't enough - needs to be kept updated each tick
spawner.requiredPlayerRange = Integer.MAX_VALUE;
if (tickCount == 1) {
- setSpawnPersistentEntities(getSpawner(), true);
+ setSpawnPersistentEntities(spawner, true);
}
}
}
Which Minecraft version are you using?
1.18
Crash log
https://gist.github.com/jeremiahwinsley/1bd88812db87252489556ebc5e181f0b
This fix looks good, thanks.
Although it might be better to simply return null for getCachedTileEntity() if the pos isn't loaded (right now it will continue to return the previously cached block entity, which is probably invalid if the pos isn't loaded). I'll do some testing on that.
Although, in this specific case, the CME is happening from doExtraCleanupTasks() which resets the spawner's player range and persistence to default. But we're being called here because the chunk has been unloaded (and thus the agitator entity). Resetting the spawner should only be done if the agitator entity is being removed due it or the spawner block being broken, not just chunk-unloaded, so an extra check is needed in the code for that.