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

Vanished metadata #6103

Closed
wants to merge 1 commit into from
Closed

Vanished metadata #6103

wants to merge 1 commit into from

Conversation

PinoOG
Copy link

@PinoOG PinoOG commented Mar 21, 2025

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)

…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)
Copy link

@gamingjovi gamingjovi left a 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.

@mdcfe
Copy link
Member

mdcfe commented Mar 23, 2025

Can you explain the rationale for this? I'm not seeing an obvious benefit to this for other plugins.

@PinoOG
Copy link
Author

PinoOG commented Mar 23, 2025

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

@mbax
Copy link
Member

mbax commented Mar 23, 2025

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.

@PinoOG
Copy link
Author

PinoOG commented Mar 23, 2025

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

@mbax
Copy link
Member

mbax commented Mar 23, 2025

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);

@gamingjovi
Copy link

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

What about ProtocolLib, how does it find vanished players?

@mbax
Copy link
Member

mbax commented Mar 24, 2025

I don't think that plugin has any vanish-plugin-detecting support.

@gamingjovi
Copy link

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.

@mbax
Copy link
Member

mbax commented Mar 24, 2025

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.

Copy link
Member

@mdcfe mdcfe left a 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.

@JasonHorkles
Copy link
Contributor

I'd also like to add that PV itself even recommends using the metadata:
image

@JRoy JRoy closed this Mar 26, 2025
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.

7 participants