Skip to content

Add Support For Listing and Unlisting Hidden Players #12532

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Y2Kwastaken
Copy link
Contributor

@Y2Kwastaken Y2Kwastaken commented May 6, 2025

This PR is addresses #12314

The goal of this pull request is to enhance the current Player#[hide:show]Entity and Player#[hide:show]Player. It is somewhat frequently requests that these methods should have some ability to list hidden players. To address this I slightly tweaked the Player#listPlayer Player#unlistPlayer api to not check for can seeing and instead allow for listing unseen players. I did this over adding a boolean to Player#[hide:show][Entity:Player] to reduce adding method overloads where tweaking existing api would work and not cause issues

@Y2Kwastaken Y2Kwastaken requested a review from a team as a code owner May 6, 2025 23:49
@github-project-automation github-project-automation bot moved this to Awaiting review in Paper PR Queue May 6, 2025
Copy link
Member

@notTamion notTamion left a comment

Choose a reason for hiding this comment

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

This api is a sort of a one way override for ensuring a player stays unlisted. e.g. isListed also just return whether the player has such an override set for the passed player or not. So this will cause some inconsistent behaivour between listing and unlisting (so it made sense to disallow this api for hidden players, as normally players are always listed). I would be in favor of adding a new sort of tristate override system that supports both overriding listing and unlisting

@github-project-automation github-project-automation bot moved this from Awaiting review to Changes required in Paper PR Queue May 12, 2025
@Y2Kwastaken
Copy link
Contributor Author

Y2Kwastaken commented May 12, 2025

This api is a sort of a one way override for ensuring a player stays unlisted. e.g. isListed also just return whether the player has such an override set for the passed player or not. So this will cause some inconsistent behaivour between listing and unlisting (so it made sense to disallow this api for hidden players, as normally players are always listed). I would be in favor of adding a new sort of tristate override system that supports both overriding listing and unlisting
e.g.

I think the current behavior is also somewhat confusing I was debating opening an issue for this. Somewhat unintuitively, the #isListed method also returns true for players that are hidden. This isn't specified in the java docs and somewhat circumvents the behavior I'd intuitively assume #isListed performs.

var player1 = ...
var player2 = ...

player1.hideEntity(plugin, player2);
System.out.prinln(player1.isListed(player2)); // true, I'd expect this to be false as the player isn't listed

I also like the idea of the tristate system. Have you thought about how that might take shape? I feel like doing too much overloading might clutter this API.

It could be as simple as

public void hidePlayer(Plugin, Player, Boolean); // where boolean overrides the hidden listing status

but I do think it would be nice to have more fine tuned control over the status

@notTamion
Copy link
Member

notTamion commented May 13, 2025

It could be as simple as

that wouldn't provide a way to list them once they are already hidden without listing

I also like the idea of the tristate system. Have you thought about how that might take shape? I feel like doing too much overloading might clutter this API.

I would have just deprecated the old methods and added new ones considering the current method names are also kinda misleading

setter: listingOverride(Player, Tristate)
getter: listingOverride(Player)

while making sure the deprecated methods keep their current behaivour

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Changes required
Development

Successfully merging this pull request may close these issues.

2 participants