Issues with client completion and flags

[dualspiral]

This is related to #3349, however this is a tougher problem to fix due to the nature of flags.

Basically, the problem is this - if you have two or more nodes that are identical except for their redirects, due to the equality check not including redirects, the nodes are "deduplicated" on the client meaning that the two different nodes on the server act as the same on the client. This results in the completion after a flag redirecting to the first command that specifies the flag.

If we could control the vanilla client, the solution is to add redirects to the equality check - however the reason why I think this wasn't done is that /execute does something broadly similar to flags in that it allows for redirecting to a point earlier in the command where it is then feasible to get back to the node, causing an infinite loop when checking for equals and thus cause a stack overflow. The real solution would be to patch Brig to handle this properly by doing a reference equality check for any redirected nodes (that's what we do on the server) - a redirect should be targetting a node in the tree so that should suffice, but without control of the client this is not easy.

I'll likely PR a patch in to Brig soon, though most PRs there have not seen any action over the last couple of years (one of aikar's PRs was pulled along with a couple of smaller fixes directly committed, but that's it). So we need to see if we can creatively work our way around the problem in Sponge because this might be a problem for some time.

Note that like in #3349, entering the command string will parse correctly, this is purely to do with tricking the client to make completions work there.

We may be able to do something using standard argument nodes, but that's going to make completion more ambiguous.

[dualspiral]

I've just been going through the code in Minecraft to figure out where the problem lies and whether we can do anything about it server side - and the answer is "no, we cannot". The problem lies on line 96 of ClientboundCommandPacket - the method enumerateNodes checks to see if a node is already in the command map that the client is building. Due to the fact that redirects are not considered in the equality contract at all, you can see where this is going.

Because children are included in it, making a flag a child of itself (or causing some loop) will cause a stack overflow - so we can't go for that solution either.

So, simply, I can't do anything from our side. I just have to hope that Mojang pulls in the fix I'm planning on writing or fixes it themselves.