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

Make tag presets unmodifiable #7378

Merged
merged 1 commit into from
Jan 21, 2022

Conversation

Machine-Maker
Copy link
Member

Tag presets that Paper adds should not be modifiable as they are global which can lead to unexpected behavior.

If this breaks any behavior for plugins, its bad behavior. Tags are not locked by default, so no proper behavior with custom tags plugins add should be affected.

@Machine-Maker Machine-Maker requested a review from a team as a code owner January 20, 2022 21:06
@Machine-Maker Machine-Maker force-pushed the fix/modifiable-tags branch 2 times, most recently from 0a7480f to e2708e0 Compare January 21, 2022 01:25
@electronicboy
Copy link
Member

ensure size is a validation check, not a "last function" type call, really should just use an explicit lock call rather than hijacking ensureSize, imho

@Machine-Maker
Copy link
Member Author

It’s not hijacking it, it’s a whole other method. Default is that the existing ensureSize won’t lock it.

@electronicboy
Copy link
Member

you're adding a param onto a method which does 1 thing to make it do another behind the context of a boolean, I don't feel that locking should be part of the ensureSize method

@Machine-Maker
Copy link
Member Author

Ok, changed it to just use the lock method. Also added a unit test to make sure all the tag presets are locked.

@Machine-Maker Machine-Maker merged commit dcca6cb into PaperMC:master Jan 21, 2022
@Machine-Maker Machine-Maker deleted the fix/modifiable-tags branch January 21, 2022 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants