Skip to content
This repository has been archived by the owner on May 29, 2022. It is now read-only.

Bunch of patches + hopper fix #352

Merged
merged 42 commits into from
Feb 26, 2022
Merged

Conversation

crafter23456
Copy link
Contributor

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:

  • [Im not good so no] I have reviewed my code thoroughly
  • I have tested my code
  • My changes generate no new warnings

@ghost
Copy link

ghost commented Feb 19, 2022

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

@crafter23456
Copy link
Contributor Author

crafter23456 commented Feb 19, 2022

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

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@crafter23456
Copy link
Contributor Author

crafter23456 commented Feb 19, 2022

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
Copy link
Collaborator

Sculas commented Feb 20, 2022

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
Copy link
Contributor Author

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
Copy link

ghost commented Feb 24, 2022

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

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly just formatting issues

crafter23456 and others added 15 commits February 25, 2022 07:52
Co-authored-by: Elier <[email protected]>
Co-authored-by: Elier <[email protected]>
Co-authored-by: Elier <[email protected]>
@crafter23456
Copy link
Contributor Author

All got accepted

@Sculas
Copy link
Collaborator

Sculas commented Feb 26, 2022

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

@Sculas Sculas merged commit ec731d5 into CobbleSword:master Feb 26, 2022
@crafter23456 crafter23456 deleted the hopper-patch branch March 6, 2022 21:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants