-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Vanished metadata #6103
Vanished metadata #6103
Conversation
…its vanish status, it would be better to remove the metadata and move its status to the playerdata file, to ensure compatibility with plugins relying on the vanished metadata (for example when streaming and filtering players with such metadata)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
THis is very smart.
Can you explain the rationale for this? I'm not seeing an obvious benefit to this for other plugins. |
The most used vanished plugins have this technique of using the metadata to save the vanished status. Some benefits are that, for plugins developers, instead of relying on several apis, if they want to filter out from commands/tab completion the vanished players, the can check if player.hasMetadata(vanished), this is a common practice I have seen from many devs. Having multiple metadata set for a given player, will require to iterate through the vanished metadata, then check for each availabe datatype and check the datatype status, thus introducing a complexity problem. I hope its clear otherwise I can write a small example |
Hello! I'm the jerk who introduced the idea of this metadata a decade ago in my plugin. You're supposed to iterate on all the metadata to see if any are true. It's not unnecessary complexity but rather intended design. |
I understand what you mean, but this design enforces to assume that every author is currently saving a boolean value |
If some author saves a non-boolean to the thing that's been used, cross-plugin, as a boolean for a decade, that's on them for unexpected consequences. Handwritten outside IDE, apologies for error, but this is a oneliner for testing vanished state without any real complexity: boolean thisPlayerIsVanished = player.getMetadata("vanished").stream().anyMatch(MetadataValue::asBoolean); |
What about ProtocolLib, how does it find vanished players? |
I don't think that plugin has any vanish-plugin-detecting support. |
The plugin I use called PremiumVanish claims it uses ProtocalLib to tell if im vanished. I've had a issue open for 2 weeks (#6085) and I've gotten no response but something may need to be changed in /bal code to detect vanished people, or people should be able to see offline peoples balance. |
I believe PremiumVanish uses ProtocolLib to hide you, not to check if vanished. Your issue appears unrelated. Note that I'm not an EssX dev so much as someone with specific experience in certain areas. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, this pull request only seeks to break an established convention for no real benefit. It has been explained how this metadata is intended to be checked in the thread already, and the other issues mentioned appear to be bugs in EssentialsX and third-party plugins which are unrelated to this metadata. Unless a compelling reason for this change can be given, I don't see a reason for this to be merged.
Instead of setting the Metadata "vanished" on false when user toggle its vanish status, it would be better to remove the metadata and move its status to the playerdata file, to ensure compatibility with plugins relying on the vanished metadata (for example when streaming and filtering players with such metadata)