search

CobbleSword / NachoSpigot

NachoSpigot is a fork of TacoSpigot 1.8.9 that offers several enhancements to performance as well as bug fixes.
GNU General Public License v3.0
237 stars 86 forks source link

Bunch of patches + hopper fix #352

Closed crafter23456 closed 2 years ago

crafter23456 commented 2 years ago

Fixes a huge hopper exploit, caused by #76. @Outfluencer probably you have a better fix than removing it. If the world was saves or with /save-all, the hopper got cleared. So I removed that function and now its working like expected.

Some commits has incorrectly the name SportPaper. But in readme its better.

Fixes/Adds

See readme.

How has this been tested?

Hopper bug got tested. Other patches only with building without errors and running in the MC world with no console errors.

Checklist:

ghost commented 2 years ago

You're combining patches from others projects without testing them properly (From your desc, you only tested the hopper bug), why?

crafter23456 commented 2 years ago

Yeah, the hopper bug was my original intention to make that PR because that was reported a bunch of times on the Discord. But then I added a few more things. For the lava change I placed lava and it flowed. The chunks were also loading normally so I expected the chunk things worked. If somethings didnt get called I didnt test because I didnt write a plugin to test it :D

crafter23456 commented 2 years ago

Looks good! Can you also check if this fixes #202 and if #75 occurs?

I coudnt reproduce #75. Same on normal nacho. The whole hopper thing #202 is weird. With normal nacho + my PR the lowest and the highest chest fills. so 50% highest and 50% lowest. Beside of that it takes 4 items, than 2 items like in config. So my PR only fix that the items disappear, but isnt related to these bugs. I still could have a look at it. EDIT: I wasnt able

Sculas commented 2 years ago

I won't merge this yet since some things aren't sure according to reviews and comments. In the case I forget, please ping me when this PR is fully ready to merge (and was tested) :)

crafter23456 commented 2 years ago

I won't merge this yet since some things aren't sure according to reviews and comments. In the case I forget, please ping me when this PR is fully ready to merge (and was tested) :)

So all hints from elier got merged. so only testing even if i dont see anything at first look :D

ghost commented 2 years ago

So all hints from elier got merged. so only testing even if i dont see anything at first look :D

I’m going to review this again later since there’s more changes

crafter23456 commented 2 years ago

All got accepted

Sculas commented 2 years ago

Thanks to all the participants in this PR for helping out!