Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using old ender pearl behavior & apply ender pearl exploit patch #11524

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

jpenilla
Copy link
Member

@jpenilla jpenilla commented Oct 27, 2024

When enabled, ender pearls will not load chunks and will save to the world instead of the player.

Also changes the exploit config to be default false, as it only makes sense when legacy behavior is enabled.
I didn't bother with a config migration since it won't do anything without legacy behavior enabled.

When enabled, ender pearls will not load chunks and will save to the world instead of the player.

Also changes the exploit config to be default false, as it only makes sense when legacy behavior is enabled.
@jpenilla jpenilla requested a review from a team as a code owner October 27, 2024 19:55
Copy link
Member

@kennytv kennytv left a comment

Choose a reason for hiding this comment

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

Someone else should test it:tm:, but the changes look fine. Bonus points for a telling config name, but I also can't think of one, so that's not a blocker

@Leguan16
Copy link
Contributor

Bonus points for a telling config name

Config name for what? legacyEnderPearlBehaviour? disableEnderpearlLoadChunks or something like that (I think that's the new behaviour or did I miss something?).

@jpenilla
Copy link
Member Author

I chose legacyEnderPearlBehaviour rather than enderPearlsTickChunks as it encompasses more than that, it also affects how pearls are saved and whether they get removed on logout, for example. I think the current name is fine and it should be addressed by docs.

@Owen1212055
Copy link
Member

Maybe we could have seperate config options for controlling if they are serialized with the player and tick chunks then?

@jpenilla
Copy link
Member Author

jpenilla commented Oct 27, 2024

doing that properly would require much more extensive changes, the behaviors are tightly coupled; I also don't see any utility there

@Leguan16
Copy link
Contributor

I chose legacyEnderPearlBehaviour rather than enderPearlsTickChunks as it encompasses more than that, it also affects how pearls are saved and whether they get removed on logout, for example. I think the current name is fine and it should be addressed by docs.

Ok yeah then i don't have another name either.

@ryantheleach
Copy link

Naming disablePlayerTickedEnderpearls ? encompasses the fact that enderpearls are loaded and ticked, and player owned by default.

@codebycam
Copy link

codebycam commented Oct 27, 2024

legacyEnderPearlBehavior is fine IMO. It indicates there has been a change, and people can find more info about it on the docs.

@ryantheleach
Copy link

ryantheleach commented Oct 27, 2024

legacyEnderPearlBehavior is fine IMO. It indicates there has been a change, and people can find more info about it on the docs.

As long as enderpearls are never changed again, otherwise you might need a legacyLegacyEnderPearlBehavior.

Personally, anything named 'legacy' or 'old' is a code smell in any coding project, unless it clarifies which legacy behaviour it is almost immediately.

But it's not the worst thing in the world.

@kennytv kennytv merged commit 580a610 into dev/1.21.2 Oct 30, 2024
4 of 6 checks passed
@kennytv kennytv deleted the 1.21.3-legacy-ender-pearls branch October 30, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

6 participants