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.

Sign up for GitHub

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

Jump to bottom

Unexplanable command completion conflict with aliases in API-8 #3349

Closed
thibaulthenry opened this issue Mar 24, 2021 · 4 comments
Closed

Unexplanable command completion conflict with aliases in API-8 #3349

thibaulthenry opened this issue Mar 24, 2021 · 4 comments
Labels
status: waiting for reply there was a specific question for the author system: command version: 1.16 (u) API: 8

Comments

@thibaulthenry
Copy link
Contributor 

SpongeVanilla version: 1.16.4-8.0.0 (commit f213985571fcc41fb69f0a5fcfcabafc2ad44b6a)
Java version: 8
Operating System: Windows 10
Plugins: Cosmos

Hey,

I have this command in my plugin :

Move:
/cm move [<entities-moved>] [<world>] [<x> <y> <z>] [<rx> <ry> <rz>]

Move has two aliases : mv and tp

For a reason I can't explain, I do not have the same completion when using /cm move or /cm tp :

move

tp

This is the same command so the completion should be the same.
Morever, when I start to complete the tp command I have parsing error :

tpwrong

Whereas when I press enter, the command runs normally.

Weirdest thing is that this bug happens only for the tp alias, mv works perfectly fine

Thanks for reading this issue
@limbo-app limbo-app added the status: needs triage This label is automatically applied to new issues and pull requests to indicate they require triage label Mar 24, 2021
@dualspiral
Copy link
Contributor

Provide code to reproduce this. I think I know what's going on but having a minimal test case that I can just copy/paste will help.

@limbo-app limbo-app removed status: needs triage This label is automatically applied to new issues and pull requests to indicate they require triage labels Mar 27, 2021
@dualspiral dualspiral added the status: waiting for reply there was a specific question for the author label Mar 28, 2021
@thibaulthenry
Copy link
Contributor Author

Unfortunately I do not manage to extract a standard example from my command registration system so I'm not able to identify the real issue of this problem.

But this is a bug that is persistent. Each time I run the server the completion fails for this specific alias.

I'll try to find a clear reproduction of this bug with standard Sponge parameters

dualspiral added a commit that referenced this issue Mar 28, 2021
@dualspiral
Add children instead of redirect where possible due to a bug in Briga…
3874fc4 
…dier

Rather, this looks like an intentional decision which actually causes huge problems with the client. A lot of our changes are designed to work around Brig but we can't affect the vanilla client. The problem is that the equality contract for nodes don't account for redirects, so if you have two literal nodes with the same name but two different redirects (or none at all), Brig (incorrectly) assumes they are the same and the client just merges them, so the client completion for one command may appear on another.

Redirects would be better simply to reduce the tree size, so if/when fixed on the client we should revert this.

This may solve #3349 and is also the same issue with command flags, but because of the way flags work we need the redirects and therefore needs a better workaround (or for Mojang to fix this)

Also fixes compile error introdueced by 5e202d4 as best as I can.
@dualspiral
Copy link
Contributor

Try the latest commit - I think this is the problem.

@thibaulthenry
Copy link
Contributor Author

Indeed, it's now working correctly!

@dualspiral dualspiral mentioned this issue Apr 4, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
status: waiting for reply there was a specific question for the author system: command version: 1.16 (u) API: 8
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dualspiral @thibaulthenry @limbo-app