SleepyTrousers / EnderIO-1.5-1.12

http://enderio.com/
The Unlicense
728 stars 355 forks source link

Redstone Conduits are preventing chunks from being unloaded #5308

Open LXGaming opened 4 years ago

LXGaming commented 4 years ago

Issue Description:

Redstone Conduits are preventing chunks from being unloaded.

What happens:

EnderIO has checks in place to verify if a chunk is loaded but it lacks checks for if the chunk is queued for unload, when getBlockState is called it results in the chunk having its unloadQueued field set to false, This is preventing the chunk from being unloaded.

On a Forge server all Redstone Conduits will prevent chunks from being unloaded.

On a SpongeForge server only Redstone Conduits with a Redstone Sensor Filter will prevent chunks from being unloaded.

What you expected to happen:

EnderIO shouldn't be ticking conduits that are in chunks queued for unload.

Steps to reproduce:

Forge:

  1. Place a Redstone Conduit.

SpongeForge:

  1. Place a Redstone Conduit.
  2. Right click with a Yeta Wrench.
  3. Insert a Redstone Sensor Filter.

I disconnected from the server and used IntelliJ to set breakpoints so I could see what chunks were still loaded.


Affected Versions (Do not use "latest"):

Your most recent log file where the issue was present:

Redstone Conduit Stacktrace (Forge):

getLoadedChunk:92, ChunkProviderServer (net.minecraft.world.gen)
loadChunk:107, ChunkProviderServer (net.minecraft.world.gen)
loadChunk:101, ChunkProviderServer (net.minecraft.world.gen)
provideChunk:147, ChunkProviderServer (net.minecraft.world.gen)
getChunk:374, World (net.minecraft.world)
getChunk:366, World (net.minecraft.world)
getBlockState:1007, World (net.minecraft.world)
neighborNotifyEvent:212, RedstoneConduitNetwork (crazypants.enderio.conduits.conduit.redstone)
notifyConduitNeighbours:191, RedstoneConduitNetwork (crazypants.enderio.conduits.conduit.redstone)
tickEnd:268, RedstoneConduitNetwork (crazypants.enderio.conduits.conduit.redstone)
onServerTick:57, ServerTickHandler (crazypants.enderio.base.handler)
invoke:-1, ASMEventHandler_124_ServerTickHandler_onServerTick_ServerTickEvent (net.minecraftforge.fml.common.eventhandler)
invoke:90, ASMEventHandler (net.minecraftforge.fml.common.eventhandler)
post:182, EventBus (net.minecraftforge.fml.common.eventhandler)
onPostServerTick:266, FMLCommonHandler (net.minecraftforge.fml.common)
tick:787, MinecraftServer (net.minecraft.server)
run:592, MinecraftServer (net.minecraft.server)
run:748, Thread (java.lang)

Redstone Conduit with Redstone Sensor Filter Stacktrace (SpongeForge):

getLoadedChunk:92, ChunkProviderServer (net.minecraft.world.gen)
redirect$zmg000$impl$ProvideChunkForced:145, ChunkProviderServer (net.minecraft.world.gen)
provideChunk:147, ChunkProviderServer (net.minecraft.world.gen)
getChunk:374, World (net.minecraft.world)
getChunk:366, World (net.minecraft.world)
getBlockState:1970, WorldServer (net.minecraft.world)
apply:15, ComparatorInputSignalFilter (crazypants.enderio.base.filter.redstone)
getNetworkInput:432, InsulatedRedstoneConduit (crazypants.enderio.conduits.conduit.redstone)
updateInputsForSource:114, RedstoneConduitNetwork (crazypants.enderio.conduits.conduit.redstone)
tickEnd:260, RedstoneConduitNetwork (crazypants.enderio.conduits.conduit.redstone)
onServerTick:57, ServerTickHandler (crazypants.enderio.base.handler)
invoke:-1, ASMEventHandler_145_ServerTickHandler_onServerTick_ServerTickEvent (net.minecraftforge.fml.common.eventhandler)
invoke:90, ASMEventHandler (net.minecraftforge.fml.common.eventhandler)
forgeBridge$post:253, EventBus (net.minecraftforge.fml.common.eventhandler)
post:203, EventBus (net.minecraftforge.fml.common.eventhandler)
onPostServerTick:266, FMLCommonHandler (net.minecraftforge.fml.common)
tick:787, MinecraftServer (net.minecraft.server)
run:592, MinecraftServer (net.minecraft.server)
run:748, Thread (java.lang)
CitiesXL2815 commented 4 years ago

seems to be SpongeForge problem bc are server doesn't have any problem with redstone conduits

HenryLoenwind commented 4 years ago

The code is there in vanilla/Forge, too:

image

and is documented:

image

But absolutely nothing but the ChunkProvider accesses that field:

image

Also, queueUnload() is called:

All of them are "unload if the chunk is idle" events.

LXGaming commented 4 years ago

I've retested this in a production environment with only Forge, EnderCore, EnderIO and a custom mod to list loadedChunks via commands, I've found that moving away from the chunk so that it's outside the view distance will cause it to get unloaded instantly, However disconnecting while the chunk is still within view distance will keep the chunk loaded even with no other players present on the server.

seems to be SpongeForge problem

Nope, As mentioned above and in the original post I've tested this without SpongeForge being present.

But absolutely nothing but the ChunkProvider accesses that field

Correct but if you check the World::getBlockState method which EnderIO calls and follow how it loads chunks you will end up at the ChunkProviderServer::getLoadedChunk which does change the unloadQueued field, I did provided a Stacktrace in my original post which shows the RedstoneConduitNetwork::tickEnd method resulting in ChunkProviderServer::getLoadedChunk getting called.

All of them are "unload if the chunk is idle" events.

Yes but the issue here is that EnderIO is marking the chunk as active when the chunks in question shouldn't be active.

HenryLoenwind commented 4 years ago

If the chunks are unloaded properly when moving away, this sounds more like a vanilla or Forge bug. There's absolutely no indication that a mod should check that field in addition to world.isBlockLoaded(). There are also plenty of calls to isBlockLoaded() in vanilla code and not a single one does any additional check.

LXGaming commented 4 years ago

Looking at the order in which notable things are ticked:

This explains why moving away from chunks unloads them correctly as it's happening within the same tick however when a player disconnects the chunks that they were loading are marked as inactive but won't be unloaded until the next tick.

Because the RedstoneConduitNetwork runs at the end of the server tick any chunks that were marked as inactive due to a player disconnecting are marked as active by EnderIO which then prevents the chunk from being unloaded on the next tick.

I wouldn't call it a Forge or Vanilla bug, Simply put EnderIO is essentially performing TileEntity logic where it shouldn't be performed it, at least not without additional checks in place, no mod should have to check the unloadQueued field but again you've shifted a ton of logic to be outside the order in which vanilla Minecraft does things and as such you can't expect it to work with zero issues.

idnthvn commented 4 years ago

ItemConduits on RFTools Crafters dont Unload Chunks. Energy Conduits on Mekanism Machines dont Unload Chunks. Using Sponge Forge RC4007 Forge 2847 Minecraft 1.12.2 Just adding this as my case was duplicate of this