mactso / SpawnerInLight

0 stars 0 forks source link

Lag caused by scanning blocks #7

Closed someaddons closed 2 years ago

someaddons commented 2 years ago

Harder spawners is causing a bunch of lag through scanning for spawner blocks: image https://spark.lucko.me/SXZ0hHL2lT Whats especially unneeded is probably doing it twice per player tick, once on the Pre event and once on the Post event. Ideally you'd make a simple mixin to inject into the spawners when they activate and do your logic there, that'd cause no lag

mactso commented 2 years ago

Thanks for the info. I'm not familiar with mixins yet. Do you have a good trivial example of some mixin code?

In the mean time, I think I could reduce the frequency and maybe given an option for aggressive or passive scanning.

FYI tho, the scanning is not about the spawners activating. It's about player actions placing light sources that could affect the spawner. Players and mods can place light blocks at any time and in actual practice have also placed lava and other bright liquid sources above to flow down onto the spawners. the mods (torchbow esp.) are a special problem because they don't raise events but directly create the light sources.

and once the light source is placed, the spawners are disabled and no longer raise events.

someaddons commented 2 years ago

btw the most easy and free improvement you could do is to only use the player Pre event, the Player tick event is fired twice per tick and you simply execute your logic twice at once, which has no use. so using PlayerTickEvent.Pre would half the performance impact already.

For the mixin e.g. https://github.com/someaddons/farsight/tree/v1.18 has a simple example and https://docs.spongepowered.org/stable/en/contributing/implementation/mixins.html has a general documentation on them. The minecraft developtment plugin for eclipse/intellij is also very helpful for them, as it can auto-fill targets you want to hook into.

mactso commented 2 years ago

Thanks for the followup! Currently trying to get Regrowth to release with 2 bugs remaining.

I think the easiest way is to allow players to set the ticks between checks. Even having 1 tick skipped would half the impact. And skipping 9 ticks would reduce it by 90%.

There will be cases where the player can get a light in and disable the spawner. But I understand the individual op will need to balance perfectly blocking players vs performance.

This is high priority so maybe a fix tomorrow.

mactso commented 2 years ago

Oh also, it's been a while so I can't remember the specifics but the pre/post was necessary because of some mod that wasn't playing nice about putting light sources. if the player used that mod to place a light, there was no notice or event I could hook into. It bypassed all normal code and just smashed the light source into the world.

someaddons commented 2 years ago

well atm you're checking for it once when the playertick begins and once when the playertick ends, if there was an order issues then using Post should suffice^^

someaddons commented 2 years ago

^maybe the blockplacement event alone is already enough, could check if it was a light block placed

mactso commented 2 years ago

The issue was they were directly changing the data and no event was being raised. Then no further events were being raised by the spawner. But I think this tick setting will help folks balance lag vs anti-cheese.

mactso commented 2 years ago

Okay so I just finished looking at the code and while it checks every tick, it only only checks twice per second for a given player and is spreading the players over 10 different ticks using this code.

(sp.getId() + sp.level.getGameTime()) % 10 == 0

I will be using your suggestion to only use one part of the player event! As you pointed out, that should halve the impact.

What do you have your "requiredPlayerRange" set to? While I allow up to 32, which will let mobs start spawning when the player is 32 away, that's going to impact performance. the default is 11- which is a little bigger than minecraft standard value of 8.

I made a change to reduce the scanning size if the requiredPlayerRange is less.

mactso commented 2 years ago

Just finished testing. Looks good. Building now.
Testing in 18.2, no crash but Not working in 1.18.2--- stops spawning after the 1st mob.

Testing in 18.1, ...it works properly. Should be up in 30 mins to an hour. Little more testing.

I'll backport this change to 1.16.5 over the weekend most likely (have to check which other mods are broken in 1.18.2 and which ones work).

someaddons commented 2 years ago

Didnt change the "requiredPlayerRange" config, so it probably is on the default of 11. And nice the frequency reduction should reduce it a bunch so that it doesnt have a big impact anymore 👍