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

Update EnumWrappers.java #2928

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

EuSouVoce
Copy link
Contributor

@EuSouVoce EuSouVoce commented May 13, 2024

Simply add an alias.
Fixes #2926

Some versions may have a typo, and be "ChatVisiblity" instead of "ChatVisibility"

Copy link
Contributor

@Ingrim4 Ingrim4 left a comment

Choose a reason for hiding this comment

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

lgtm

@0utplay
Copy link

0utplay commented May 14, 2024

Shouldn't we maybe create a more generalized version? I think the mappings might diverge more with every update and just adding more class names won't last very long @Ingrim4 ?

@Ingrim4
Copy link
Contributor

Ingrim4 commented May 14, 2024

I completely agree that splitting the code into more generalized components for increased fail-safety would be advantageous. However, it's important to acknowledge that such a redesign could potentially break compatibility with certain plugins if not executed carefully. This is definitely something that could be done in a future redesign of a new API which tries to separate the API from the implementation (Somewhat related to this comment).

@DoubleNico
Copy link
Contributor

I have an idea how to make it work for mojang mappings and spigot obf, we could use https://github.com/jpenilla/reflection-remapper library to make it so we can use only spigot obf then use this library to convert it to mojang mappings so we can make it generalized and not add additional classes to every method out there. Tho we need to download the mappings files in the plugin(but it will take a lot of space) or download it from the web and put it in the plugins folder

@Ingrim4
Copy link
Contributor

Ingrim4 commented May 16, 2024

I have reservations about mandating unrestricted web requests for the plugin's functionality. Additionally, the suggested library appears to offer more features than necessary, possibly introducing unnecessary complexity. For ProtocolLib, a solution tailored specifically to its needs might be more appropriate. Ultimately, these considerations tie into the potential redesign I'm planing for certain aspects of ProtocolLib. While this may not be immediately feasible, it's certainly worth exploring in the future. (Also mappings don't ensure mojang won't rename things in the future)

Copy link

@Jeppa Jeppa left a comment

Choose a reason for hiding this comment

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

This will work for the moment...
But it's not a "typo" ! ;)
The unobfuscated server has a lot of different names than the obfuscated one...

@0utplay
Copy link

0utplay commented May 20, 2024

This will work for the moment...
But it's not a "typo" ! ;)
The unobfuscated server has a lot of different names than the obfuscated one...

How is Visiblity not a typo. Its missing the third I

@Jeppa
Copy link

Jeppa commented May 20, 2024

How is Visiblity not a typo. Its missing the third I

oops, didn't see the second commit ;)
Yes , you're right...

@dmulloy2 dmulloy2 merged commit 41a0c9d into dmulloy2:master Jun 3, 2024
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.

Spigot & Paper class name mismatches for EnumChatVisibility
6 participants