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

Redo XP perks #5095

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Redo XP perks #5095

wants to merge 2 commits into from

Conversation

Jsinco
Copy link

@Jsinco Jsinco commented Sep 27, 2024

This PR removes the old 'else if' chain used for xp perks and replaces it with an enum class for faster usage and other benefits. This PR also brings another important change by ensuring the highest XP multiplier amount is always used for the player.

Before this change, mcMMO would always use the custom multiplier if the player had permission for it even if it was lower than other multiplier amounts. This would cause conflicts for those who want to use the custom xp amount for a lower xp boost, and then give their players higher boost amounts later on.

@nossr50
Copy link
Member

nossr50 commented Sep 28, 2024

I appreciate this PR but I'll have to think about if I merge this or not, you see I have a complete rewrite for perks in another branch that adds new functionality, I'd just have to port the code over... 😮‍💨

@Jsinco
Copy link
Author

Jsinco commented Sep 28, 2024

It would probably be better to either:

A. Merge and fix this issue
B. Fix this yourself

I don't think my solution is bad, or even hard to reverse if needed. But leaving this unpatched shouldn't really be considered?

@nossr50
Copy link
Member

nossr50 commented Sep 28, 2024

Good points, I'll think on it, and review the PR in the near future.

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.

2 participants