Slimefun-Addon-Community / Galactifun

A Slimefun addon inspired by ClayTech
GNU General Public License v3.0
23 stars 23 forks source link

Change Ticking System #138

Closed JustAHuman-xD closed 1 year ago

JustAHuman-xD commented 1 year ago

Changes

Instead of iterating through all of the entities on the Server with a heavy method such as world#getLivingEntities(). It will now cache the entities UUID when it spawns it in. Then in the Ticking method we retrieve all of the entities with Bukkit#getEntity(UUID).

Checklist

GallowsDove commented 1 year ago

Don't currently have the time to test unfortunately, but I'm not really confident that this will work in between server restarts. Please test it thoroughly.

JustAHuman-xD commented 1 year ago

Oh yeah sorry, forgot that you dont remove the entities on server stop, will do rq.

GallowsDove commented 1 year ago

I think there might still be an issue with this. world.getLivingEntities() goes only through currently loaded entities, so any unloaded aliens that exist will not get looped, so will not get ticked later.

JustAHuman-xD commented 1 year ago

I could listen to the chunk load event, not sure how heavy that would be though.

GallowsDove commented 1 year ago

Yeah either that, or periodically update cache e.g. every 2 seconds. Would need performance testing to figure out

Seggan commented 1 year ago

I could listen to the chunk load event, not sure how heavy that would be though.

Isn't there a Chunk#getLivingEntities?

JustAHuman-xD commented 1 year ago

Isn't there a Chunk#getLivingEntities?

Its Chunk#getEntities, but yeah the thought was I would listen for that event and get the entities, but I wasn't sure how heavy listening to the chunk load event. Maybe I also cache the chunk coords that way I dont bother running that again after the chunk has been loaded for the first time.

Seggan commented 1 year ago

Nah, caching would cause some stuff with respawning stuff. In any case, its still lighter than using World#getLivingEntities on every 2 ticks

JustAHuman-xD commented 1 year ago

Caching coordinates & uuids, shouldnt affect anything with respawning

JustAHuman-xD commented 1 year ago

I think there might still be an issue with this. world.getLivingEntities() goes only through currently loaded entities, so any unloaded aliens that exist will not get looped, so will not get ticked later.

Also I believe using world#getEntities() upon say startup it does actually return all of the entities of the world but unsure atm. I used it with a separate project and it worked regardless of the chunks being loaded but I need to look into it further.

JustAHuman-xD commented 1 year ago

This theoretically works, not sure the performance difference but I can test it soon™️

Seggan commented 1 year ago

Will merge once you confirm that it works

JustAHuman-xD commented 1 year ago

👍 alright, did a little bit of testing prior and had no issues but definitely need to do more. I will also get 2 spark reports to compare the new system and the old.

JustAHuman-xD commented 1 year ago

I'll also check if the whole chunk system is necessary. I don't think it will be because of the

mob.setRemoveWhenFarAway(true);

But I'll definitely check. (I didn't earlier because didn't have time to do much more than write the system.)

JustAHuman-xD commented 1 year ago

Sorry that this PR completely died, I'll take a look at the remaining issues tonight and hopefully wrap it up

JustAHuman-xD commented 1 year ago

Okay yeah I have confirmed that the mobs are persistent after a restart time to do some other checks

JustAHuman-xD commented 1 year ago

though it might be because they were in spawn chunks in any case Im doing a lot of testing rn

JustAHuman-xD commented 1 year ago

Going to close this so that I can do this from not the main branch of my fork, and so I can do the changes that have been discussed in addon-devs in discord