search

InfernalSuite / AdvancedSlimePaper

Advanced Slime World Manager as a Paper Fork
GNU General Public License v3.0
234 stars 83 forks source link

Chunk pruning does not work at all #103

Closed davidmayr closed 3 months ago

davidmayr commented 6 months ago

Describe the issue Chunk pruning does not ignore empty chunks because ChunkPruner#areSectionsEmpty returns false in every case. This can probably be fixed by simply returning true at the end of the method instead of false. And no, the prune changes from @emmanuelvlad do not fix this.

Plugin version Issue occurs on main branch with commit dc112775adc988eac54444a215ab055a41a538f9

How to reproduce Save a world.

Crash reports (if available) Not applicable

davidmayr commented 6 months ago

Might also be a good idea to check inside ChunkPruner#areSectionsEmpty if the palette has actually a size of one, instead of just checking the first argument. Sorting order is not guaranteed, so this might be important

emmanuelvlad commented 6 months ago

I rewrote the filtering part (https://github.com/InfernalSuite/AdvancedSlimePaper/pull/99/commits/a7dcd55ea772b40f2ba0ec474abaf60bbec54971) to fix an issue where chunks got serialized twice thus making the world size double in size. Didn't check if the pruning itself worked since I didn't touch that part

kyngs commented 3 months ago

This can probably be fixed by simply returning true at the end of the method instead of false

Unfortunately, doing so deletes valid chunks, as the if statement is poorly written. It seems like the author forgot to finish the method, I'll try to fix it.

kyngs commented 3 months ago

@davidmayr May I ask you to get the latest develop build and check whether chunks are not getting lost? It works on my machine, but I just want to verify it before this gets merged into main.

davidmayr commented 3 months ago

@davidmayr May I ask you to get the latest develop build and check whether chunks are not getting lost? It works on my machine, but I just want to verify it before this gets merged into main.

I unfortunately cannot check right now, but I have my own implementation of your fix implemented for a few months now, and it works just fine. Our code seems to do the exact same thing, so you can probably merge it. I have attached my code in form of a screenshot so you can check for yourself (I do paper development in a Linux VM and the clipboard is broken right now so I cannot copy a code snippet)

With my patch, I have not encountered any issues. All maps saved just fine.

grafik

kyngs commented 3 months ago

Yeah, the code is pretty much the same. Thanks for your response!