Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Command parser fixes #54

Merged
merged 13 commits into from
Aug 30, 2023
Merged

Command parser fixes #54

merged 13 commits into from
Aug 30, 2023

Conversation

mworzala
Copy link

Created from Minestom#1893

@spannercodes
Copy link

I've figured out the reason for the argument order bug:

The problem is with the graph generation. See GraphImpl.java line 155 (or all of fromCommand). Basically, it first generates a ConversionNode for the "AB together" syntax, and in doing so adds the argument "A" to the command's children (syntaxNode.nextMap). The executor for "A"'s ConversionNode is set to null, because the converter doesn't know about the "A only" syntax (and so it is doing the right thing to say that "cmd a" would not have an executor).
The issue is that because "A" is already added to the command's children, the graph generator skips it and does not add the newly found executor.

I'm not sure how to fix this yet but I'll keep trying. I think it's going to have to be something horrible, or just rewrite parts of ConversionNode, because currently it's a record and everything is final.

@mworzala mworzala enabled auto-merge (squash) August 30, 2023 23:01
@mworzala
Copy link
Author

thank you @spannerdev!

@mworzala mworzala merged commit e9d0098 into main Aug 30, 2023
1 check passed
@mworzala mworzala deleted the spannerdev/commandparser-fixes branch August 30, 2023 23:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants