[SPIGOT-6547] getEntities() not working as expected Created: 13/Jun/21 Updated: 17/Oct/21 Resolved: 01/Sep/21 |
|
Status: | Resolved |
Project: | Spigot |
Component/s: | None |
Affects Version/s: | None |
Fix Version/s: | None |
Type: | Bug | Priority: | Minor |
Reporter: | Jeppa | Assignee: | Marvin Rieple |
Resolution: | Fixed | Votes: | 10 |
Labels: | None |
Issue Links: |
|
||||||||
Version: | This server is running CraftBukkit version 3133-Spigot-66f9d3c-3fa79d3 (MC: 1.17) (Implementing API version 1.17-R0.1-SNAPSHOT) | ||||||||
Guidelines Read: | Yes |
Description |
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. 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 |
Comments |
Comment by Jeppa [ 17/Oct/21 ] |
It seems that event.getChunk().getEntities() inside of ChunkUnloadEvent results in a loop... 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... |
Comment by Jeppa [ 22/Sep/21 ] |
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()...
|
Comment by Marvin Rieple [ 05/Sep/21 ] |
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. |
Comment by Nathan Wolf [ 03/Sep/21 ] |
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. |
Comment by LoneDev [ 01/Sep/21 ] |
Thanks for the fix! |
Comment by Jeppa [ 24/Aug/21 ] |
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... |
Comment by LoneDev [ 24/Aug/21 ] |
One important note: |
Comment by Skizzles [ 24/Aug/21 ] |
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. |
Comment by Frank Saibert [ 24/Aug/21 ] |
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?
|
Comment by LoneDev [ 23/Aug/21 ] |
I think that getEntities() method must wait on the main thread until the entities are loaded correctly. 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. |
Comment by party [ 22/Aug/21 ] |
waiting 10 ticks after chunk load event to loop through entities seems to work most of the time.
this is a major bug... |
Comment by Eric [ 22/Aug/21 ] |
Absolutely agree with the above. This particular issue breaks a lot of plugins relying on updating/processing entities on chunkload, including plugins using Armorstands they need to update or custom entities (like mine). It therefore really doesn't make sense that it is downgraded to minor after not being fixed for months. No single other event can truly replace it either. The only workaround currently is waiting some ticks after chunkload and then looping its entities, which is not at all fail-proof due to async loading possibly taking many ticks. |
Comment by Studio Code [ 22/Aug/21 ] |
This is not a minor issue, I don't agree with the priority change to minor. The pull request is ready (and has been for quite some time now) and should be merged asap. |
Comment by LoneDev [ 21/Aug/21 ] |
Ii can confirm that this issue is still valid in 1.17.1 |
Comment by Jeppa [ 16/Aug/21 ] |
Why did this bug change from major to minor ? There is still no solution available that fixes all possible issues with this...
|
Comment by Nathan Wolf [ 17/Jun/21 ] |
I'm still not really sure what you're saying or why you're saying it to me, I didn't mean to imply it was only an event problem, just that sending spawn events when the entities load would be a workable solution for me. Anyway, doesn't really matter so I'm going to stop responding to avoid cluttering up everyone's email.
|
Comment by Jeppa [ 17/Jun/21 ] |
I said, it's not only in ChunkLoadEvent or any other event. It's a general issue when loading chunks.
PS: OK, maybe i got your first comment wrong. |
Comment by Nathan Wolf [ 17/Jun/21 ] |
Jeppa, I don't think I understand that comment. When does async entity loading happen outside of chunk loading? I would just like a reliable way to know when an entity gets spawned in the world, whether from chunk loading or elsewhere. This could be the EntitySpawnEvent or some other more specific event, just need some way to know when they're there. |
Comment by Jeppa [ 17/Jun/21 ] |
@NathanWolf: This is not only about chunk events. It's a general issue with chunk loading and entitys... |
Comment by Nathan Wolf [ 17/Jun/21 ] |
I would love to see an event for when these entities load, I currently track persistent NPC entities and that would be the best solution for me to find them. Alternately, if it makes sense to issue an Entity/Creature Spawn event, I believe that was the behavior maybe pre 1.14 (rather than them appearing in the ChunkLoadEvent)? |
Comment by Marvin Rieple [ 14/Jun/21 ] |
Made a PR for this: craftbukkit#866 |