Skip to content

Make CommandSourceStack respect hidden players#11556

Closed
notTamion wants to merge 4 commits into
PaperMC:masterfrom
notTamion:fix-commandsourcestack
Closed

Make CommandSourceStack respect hidden players#11556
notTamion wants to merge 4 commits into
PaperMC:masterfrom
notTamion:fix-commandsourcestack

Conversation

@notTamion
Copy link
Copy Markdown
Contributor

@notTamion notTamion commented Nov 1, 2024

fixes #11545
post hardfork #11898

@notTamion notTamion requested a review from a team as a code owner November 1, 2024 17:18
@lynxplay
Copy link
Copy Markdown
Contributor

lynxplay commented Nov 1, 2024

We have this diff already

+ // Paper start - tell clients to ask server for suggestions for EntityArguments
+ final Collection<String> collection;
+ if (icompletionprovider instanceof CommandSourceStack commandSourceStack && commandSourceStack.getEntity() instanceof ServerPlayer sourcePlayer) {
+ collection = new java.util.ArrayList<>();
+ for (final ServerPlayer player : commandSourceStack.getServer().getPlayerList().getPlayers()) {
+ if (sourcePlayer.getBukkitEntity().canSee(player.getBukkitEntity())) {
+ collection.add(player.getGameProfile().getName());
+ }
+ }
+ } else {
+ collection = icompletionprovider.getOnlinePlayerNames();
+ }
+ // Paper end - tell clients to ask server for suggestions for EntityArguments

Either we remove this or duplicate it.
Idk what else calls this method and if that is of interest to maybe not solve as deep.

@notTamion
Copy link
Copy Markdown
Contributor Author

its only called by argument types so i think fixing this that deep is the correct solution.
image

@lynxplay
Copy link
Copy Markdown
Contributor

lynxplay commented Nov 1, 2024

Okay, thanks for the quick check 👍

@lynxplay lynxplay force-pushed the fix-commandsourcestack branch from 79c7627 to 491d15b Compare November 10, 2024 00:11
@lynxplay
Copy link
Copy Markdown
Contributor

While this works, we should probably at least discuss if we want certain commands to obey this visibility.
Banning and deop both sounds like commands where I'd never like this command to not show me the members?

@notTamion
Copy link
Copy Markdown
Contributor Author

Perhaps we can add an exclusion for operators?

@Leguan16
Copy link
Copy Markdown
Contributor

I think excluding operators makes sense.

@notTamion notTamion force-pushed the fix-commandsourcestack branch from 491d15b to c241489 Compare November 10, 2024 21:40
@notTamion
Copy link
Copy Markdown
Contributor Author

just an idea, if anyone has a solution they find better please leave a comment

@Machine-Maker
Copy link
Copy Markdown
Member

I don't like making more functionality dependant on ops. The concept of "op" is not a good thing to really encourage more. People should be using proper permissions. I thought about maybe a new permission that could be granted to bypass all visibility hiding, but I'm not sure about that either.

@Leguan16
Copy link
Copy Markdown
Contributor

I don't know. The paper command for example also works for ops and mixing op and permission requirements sucks.

@notTamion
Copy link
Copy Markdown
Contributor Author

i agree with machine, considering servers will most likely not give staff operator but instead give them the permissions they need i think it would be smarter to add a permission for this. i don't think comparing this to the paper command is fair as the paper command is more of a debug command rather than something a staff member would actually have to use frequently

@Machine-Maker
Copy link
Copy Markdown
Member

Ok, I think we want a permission, but just for command suggestions, paper.bypass-visibility.tab-completion

@notTamion notTamion force-pushed the fix-commandsourcestack branch from c241489 to 41ff252 Compare November 26, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

Players aren't hidden in the /team command

6 participants