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

Feat/validate commands #591

Merged
merged 31 commits into from
Aug 31, 2022
Merged

Feat/validate commands #591

merged 31 commits into from
Aug 31, 2022

Conversation

outercloudstudio
Copy link
Member

Description

Adds validation for the MC Function language.

Motivation and Context

This allows bridge to catch command syntax errors within editor before you go into game.

Screenshots (if appropriate):

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the code style of this project.
  • I have tested my changes and confirmed that they are working

Resolves #77
Resolves #167

outercloudstudio and others added 23 commits August 21, 2022 13:41
@outercloudstudio
Remove Old Validator Files / References
8123959
@outercloudstudio
First Working Validation
20f9bb8 
Validates:
- Command Name
- Argument Types
- Argument Count
@solvedDev
Merge branch 'dev' of https://github.com/bridge-core/editor into feat…
d7f2b62 
…/validate-commands
@solvedDev
upd: use diagnostics array instead of throwing error
38995b2
@solvedDev
fix: skip comment lines
8fdd1ff
@solvedDev
fix: don't continue validating line if it's not a valid command name
70ba25f
@outercloudstudio
Begin evaluating subcommands
6a15a92
@outercloudstudio
Change definition validation to be token index independent
bc40fd2
@outercloudstudio
Validating Subcommands
4e71bb7
@outercloudstudio
Bug Fixes + Handle Subcommands
507225a
@outercloudstudio
Remove most debug logs, More comments
561514a
@outercloudstudio
Handle recursive command validation
40648dc
@outercloudstudio
Subcommand argument validation supports additional values
6f860e9
@outercloudstudio
Warning Support + Schema Reference Support
ed079a7
@outercloudstudio
Change how diagnostics are stored
c29896f 
Diagnostics and warnings are stored per definition until the definition passes. This allows warnings in one command to still appear if one of its command arguments has an error.
@outercloudstudio
Optional arguments
a3639f6
@outercloudstudio
Fix option values
b0bb122
@outercloudstudio
Translation Key Support + En Translations
207f1c1
@outercloudstudio
Fix invalid argument translation
3a8c22a
@outercloudstudio
Validate Selectors
ba9253a
@outercloudstudio
Restructure Json Data
89f901d 
Needs further testing to see if this functions properly
@outercloudstudio
Score Data
10b6f8e
@outercloudstudio
Block State
449ef39
@Joelant05
Copy link
Member

Joelant05 commented Aug 28, 2022 •
edited
Loading

This is awesome! I've came across a few things I think should be fixed before this is merged:

  • Target selector argument negation isn't supported; it looks like the code is there for this, but tries to validate the argument value with the "!" included
    image

  • "Argument is not valid" error message can be misleading where the user might not have finished writing the command yet, because the argument is actually valid. In this case, I think the message should state "Additional arguments are required for command <COMMAND>"
    image

  • The say commands supports spaces between words, but the validator sees these as separate tokens
    image

  • The token displayed in the unknown schema value warning message is incorrect
    image

@outercloudstudio
Copy link
Member Author

I will work on fixing the current bugs atm!
One note: Since the this is based off the tokenizer I'll have to hardcode the the fact the the say command can take multiple strings since I don't want to touch the validator for fear of breaking something else.

@outercloudstudio
Copy link
Member Author

Alright everything should be fixed now just waiting for review approval!

@Joelant05
Copy link
Member

Almost perfect now :D One last thing I've spotted is @initiator validation doesn't seem to work correctly
image

@outercloudstudio
Copy link
Member Author

Alright should be fixed now!

@solvedDev
Copy link
Member

Awesome, I'll take a look and hopefully merge this for today's nightly build 👏

solvedDev
solvedDev approved these changes Aug 31, 2022
View reviewed changes
Copy link
Member

@solvedDev solvedDev left a comment

Choose a reason for hiding this comment

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

Awesome work; LGTM!

@solvedDev solvedDev merged commit 6432865 into main Aug 31, 2022
solvedDev added a commit that referenced this pull request Aug 31, 2022
@solvedDev
Revert "feat: validate commands (#591)"
75d4228 
This reverts commit 6432865.
@solvedDev solvedDev mentioned this pull request Aug 31, 2022
Merged
solvedDev added a commit that referenced this pull request Aug 31, 2022
@solvedDev
Revert "feat: validate commands (#591)" (#593)
4f08ae7 
This reverts commit 6432865.
@solvedDev
Copy link
Member

PR had the wrong base branch selected

@solvedDev solvedDev mentioned this pull request Aug 31, 2022
Merged
@solvedDev solvedDev deleted the feat/validate-commands branch August 31, 2022 11:59
@Joelant05 Joelant05 mentioned this pull request Sep 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@solvedDev solvedDev solvedDev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Mcfunction file validation Validate Commands
3 participants
@outercloudstudio @Joelant05 @solvedDev