-
Type:
Bug
-
Resolution: Fixed
-
Priority:
Minor
-
None
-
Affects Version/s: None
-
None
-
This server is running CraftBukkit version 3133-Spigot-66f9d3c-3fa79d3 (MC: 1.17) (Implementing API version 1.17-R0.1-SNAPSHOT)
-
Yes
When loading chunks, the entities in those chunks are not loaded anymore!
I'm using World#loadChunk or Chunk#load to load unloaded/inactive chunks.
It returns 'true'... so chunk is loaded.
But i can't get the entities from that chunk using Chunk#getEntities or World#getEntitiesByClasses etc.
Checking again a few ticks later some of the entities can be found.
TestCode:
Location testLoc=new Location(world,800,90,-1500); Chunk testChunk=world.getChunkAt(testLoc); testChunk.load(); Entity[] enderman=testChunk.getEntities(); logger.warning("Found Entities before spawn #:"+enderman.length); world.spawnEntity(testLoc, EntityType.ENDERMAN); testChunk.unload(); testChunk.load(); enderman=testChunk.getEntities(); logger.warning("Found Entities after spawn #:"+enderman.length);
Calling this multiple times results in the following output:
[15:31:14] [Server thread/WARN]: [plugin] Found Entities before spawn #:0 [15:31:14] [Server thread/WARN]: [plugin] Found Entities after spawn #:0 [15:31:14] [Server thread/WARN]: [plugin] Found Entities before spawn #:0 [15:31:14] [Server thread/WARN]: [plugin] Found Entities after spawn #:2 [15:31:14] [Server thread/WARN]: [plugin] Found Entities before spawn #:2 [15:31:14] [Server thread/WARN]: [plugin] Found Entities after spawn #:3
With servers running 1.16 or older, the result is like this:
[15:39:33] [Server thread/WARN]: [plugin] Found Entities before spawn #:0 [15:39:33] [Server thread/WARN]: [plugin] Found Entities after spawn #:1 [15:39:33] [Server thread/WARN]: [plugin] Found Entities before spawn #:1 [15:39:33] [Server thread/WARN]: [plugin] Found Entities after spawn #:2 [15:39:33] [Server thread/WARN]: [plugin] Found Entities before spawn #:2 [15:39:33] [Server thread/WARN]: [plugin] Found Entities after spawn #:3
- is duplicated by
-
-
- Resolved
-
[SPIGOT-6547] getEntities() not working as expected
Now i had some time to test the fix 
It seems, this only fixes the issue when a chunk is checked.
World#getChunkAt#getEntities() works!
But World#getEntities() or World#getEntitiesByClass() still return 0 !
I can live with that, as i'm doing a getChunkAt#getEntities() before doing World#getEntities()...
The valid state of an entity depends on three conditions 1: If the entity is still alive 2: If the chunk is loaded 3: If the entity is tracked.
Since the entity loading occurs later and some what independent of the chunk loading it can happen that a chunk gets unloaded before the entities are loaded. The EntitiesLoadEvent is still called but because the chunk is not loaded the entities will be not valid if this is the case a EntitiesUnloadEvent should be called some ticks later. When you call Chunk#getEntities the chunk gets loaded (if it is not already loaded) resulting in the entities becoming valid. When you call Chunk#getEntities (or Chunk#load) during EntitiesLoadEvent the entities in the return list from EntitiesLoadEvent should become valid and Chunk#getEntities should return the same entities objects as EntitiesLoadEvent.
You can either only check if the entity is removed or check in addition to the valid state also if the chunk is loaded or not.
If you find / have other behaviour such as only a part of the entities in the list returned by EntitiesLoadEvent being not valid than consider opening a new bug report with a test plugin and steps to reproduce.
Nathan Wolf
added a comment - I'd like to second the thank you for this fix!
I am just hooking it up now, and I wanted to check- is it expected that entities (sometimes?) will still be in an invalid state (entity.isValid() == false) when this event is fired?
If so, is there a way to tell that they will be valid, versus a genuinely invalid entity that I need to respawn?
Similarly they don't appear in the chunk.getEntities() array, but I think if my understanding is correct that part is to be expected, hence the secondary List<Entity> parameter.
Nathan Wolf
added a comment - I'd like to second the thank you for this fix!
I am just hooking it up now, and I wanted to check- is it expected that entities (sometimes?) will still be in an invalid state (entity.isValid() == false) when this event is fired?
If so, is there a way to tell that they will be valid, versus a genuinely invalid entity that I need to respawn?
Similarly they don't appear in the chunk.getEntities() array, but I think if my understanding is correct that part is to be expected, hence the secondary List<Entity> parameter. Thanks for the fix!
https://hub.spigotmc.org/stash/projects/SPIGOT/repos/bukkit/commits/ed7bba95fc031ca8b89d353426990b4bf7f9372f#src/main/java/org/bukkit/event/world/EntitiesLoadEvent.java
Delaying the check or the event is no solution if i need a result in realtime...
If i check for existing entities in a chunk (or in the whole world!) and this is NOT started by an event,
but by any routine in realtime (main task)... and if i need the amount of existing entities as a return value,
any delay is unacceptable or even possible to handle...
One important note:
Make sure to check if the chunk is still loaded when running the delayed task or it will be loaded again and cause TPS drops.
This issue is causing countless plugins to not load entities correctly and resulting in progress lost for players. This most certainly should be a Major issue. Some developers have tried countless possible work around and the issue is STILL there.
If the PR is ready, merge it and relieve devs and servers. Then clean it up later. This has gone on for over 2 months.
Frank Saibert
added a comment - That unreliability prevents a whole minecraft server from being updated.
We have a few custom plugins which rely on getEntities(). I hate currently available workarounds like "2 ticks later" or a timer that crawls through Bukkit.getWorld().getEntities() every few ticks to spot changes. These solutions are terrible, not reliable and making these plugins unstable (entities glitches through unprocessed here and there when an event fires before the delay is over).
We've seen such issues in third-party plugins whose developers used, if they were aware of this behavior change at all, one of these bad workarounds. We can't go live with such issues. Backward compatibility is broken.
How about a hybrid solution?
- delaying the existing ChunkLoadEvent until both, chunks and entities, are fully loaded and ready. This is not the best soluton, but a solution.
- properly separated ChunkLoadBlocksEvent and ChunkLoadEntitiesEvent, firing independently but reliable and without the delay. In this case, the plugin has to manage the cases where only blocks or only entities of a chunks are loaded.
- If you want: deprecate ChunkLoadEvent
- The very same thing for ChunkUnloadEvent
Frank Saibert
added a comment - That unreliability prevents a whole minecraft server from being updated.
We have a few custom plugins which rely on getEntities(). I hate currently available workarounds like "2 ticks later" or a timer that crawls through Bukkit.getWorld().getEntities() every few ticks to spot changes. These solutions are terrible, not reliable and making these plugins unstable (entities glitches through unprocessed here and there when an event fires before the delay is over).
We've seen such issues in third-party plugins whose developers used, if they were aware of this behavior change at all, one of these bad workarounds. We can't go live with such issues. Backward compatibility is broken.
How about a hybrid solution?
delaying the existing ChunkLoadEvent until both, chunks and entities, are fully loaded and ready. This is not the best soluton, but a solution.
properly separated ChunkLoadBlocksEvent and ChunkLoadEntitiesEvent, firing independently but reliable and without the delay. In this case, the plugin has to manage the cases where only blocks or only entities of a chunks are loaded.
If you want: deprecate ChunkLoadEvent
The very same thing for ChunkUnloadEvent
I think that getEntities() method must wait on the main thread until the entities are loaded correctly.
This will make old plugins working and I think it's not a that dirty solution.
Implementing a new event for async entities loading will also do the trick, but getEntities() will still be unreliable.
EDIT:
I think that a new event called ChunkEntitiesLoaded would be cleaner.
It seems that
event.getChunk().getEntities()
inside of
ChunkUnloadEvent
results in a loop...
--> crash!
This happens to me using HolographicDisplays_2.4.9 with latest spigot compiles...
Don't know if HolographicDisplays' developer could fix it somehow... his code looks good to me...