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

Allow developers to control Arguments' CommandSyntaxExceptions #370

Closed
wants to merge 86 commits into from
Closed

Allow developers to control Arguments' CommandSyntaxExceptions #370

wants to merge 86 commits into from

Conversation

willkroboth
Copy link
Collaborator

@willkroboth willkroboth commented Nov 27, 2022 •
edited
Loading

When a raw command gets parsed into an Object array, there are two parts: Brigadier's initial parse from a raw command String to Minecraft objects, and the CommandAPI's parse from those Minecraft objects to Bukkit objects. Both of these steps may throw CommandSyntaxExceptions, and this PR intercepts those exceptions and adds API methods that allow developers to do something when this happens. You can read the following comments for details about each of the parts of this system.

Todo List

Things that should be done before this PR is ready to merge:

  • Test the implementation of registerCustomArgumentType for each Minecraft version (at least 1.16.5+)
    • Need to test implementation for 1.19.3
    • Merge update and test for 1.19.4
  • Expand the ArgumentParseExceptionContext and InitialParseExcpetionContext records
  • Make sure it works with CommandTrees
  • Test with optional arguments
  • Make sure it works outside of the CommandAPI project (without Brigadier dependency)
  • Make sure it works when shading
  • Make it easier to recognize what caused an exception
    • Something here could add more information about the reason why the exception was thrown?
  • Documentation
  • Basic tests for the feature
    • InitialParseExceptionHandler
    • ArgumentParseExceptionHandler
      • Changing exceptions
      • Returning replacement value
  • Address any code review
  • Remove test command from CommandAPIMain

Things that could be done before this PR is merged:

  • Make it work on Velocity
  • Make tests for intercepting every possible error for every argument?
  • Document all the possible errors for every argument?

willkroboth and others added 30 commits November 19, 2022 10:11
@willkroboth
Add option to add custom error handling when Argument parse fails
db13e38
@willkroboth
Merge pull request #366 from willkroboth/dev/dev
dc89c3a 
Add option to add custom error handling when Argument parse fails
@willkroboth
Add Javadocs to new methods and classes
25b975d
@willkroboth
Add sender and input to ArgumentParseExceptionContext
e601e74
@willkroboth
Intercept errors during Brigadier's initial parse
c07603a 
revert last commit for now
@willkroboth
Attempt serializing custom ArgumentType
66c3284
@willkroboth
Fix ExceptionHandlingArgumentType serialization
6aab75d
@willkroboth
Clean up code and add API methods
17fa93f
@willkroboth
Add InitialParseExceptionArgument to testing arguments
06aca10
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
6190486
@willkroboth
Implement InitialParseExceptionArgument for other applicable arguments
417fccb
@willkroboth
Reduce code duplication by storing InitialParseExceptionHandlers in a…
bd5fbf8 
… static map in InitialParseExceptionArgument
@willkroboth
Implement NMS#registerCustomArgumentType
d591895
@willkroboth
Clean up code
c2de33a
@willkroboth
Make CodeFactor happier (and me a little) by reordering methods in th…
cfc1be4 
…e ExceptionHandlingArgumentType serializer
@willkroboth
Fix 1.16.2-3 & 1.16.4-5 ExceptionHandlingArgumentSerializer method re…
ad65892 
…flection
@willkroboth
Fix 1.13-1.16 method reflection
fea28da
@willkroboth
Make GitHub Actions check NMS Common builds on all versions it is sup…
84e0f39 
…posed to
@willkroboth
Merge remote-tracking branch 'origin/dev/argument-exceptions' into de…
6fde891 
…v/argument-exceptions
@willkroboth
Fix 1.17 registerCustomArgumentType
15f2b3a
@willkroboth
Fix 1.18 registerCustomArgumentType
537cf5a
@willkroboth
Remove unused (and sometimes invalid) imports from NMS_Common
e5cc28b
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
b25b3cd 
# Conflicts:
#	commandapi-nms/commandapi-1.17-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_17_Common.java
#	commandapi-nms/commandapi-1.18.2/src/main/java/dev/jorel/commandapi/nms/NMS_1_18_R2.java
#	commandapi-nms/commandapi-1.19-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_19_Common.java
@willkroboth
Clean up after merge
eab3466
@willkroboth
Add WrapperStringReader to InitialParseExceptionContext
f5bef67
@willkroboth
Add sender, input, and previousArguments to ArgumentParseExceptionCon…
5e1f02d 
…text
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
597d609 
# Conflicts:
#	commandapi-core/src/main/java/dev/jorel/commandapi/arguments/SoundArgument.java
#	commandapi-nms/commandapi-1.16.2/src/main/java/dev/jorel/commandapi/nms/NMS_1_16_R2.java
#	commandapi-nms/commandapi-1.16.4/src/main/java/dev/jorel/commandapi/nms/NMS_1_16_4_R3.java
#	commandapi-nms/commandapi-1.17-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_17_Common.java
#	commandapi-nms/commandapi-1.18.2/src/main/java/dev/jorel/commandapi/nms/NMS_1_18_R2.java
#	commandapi-nms/commandapi-1.18/src/main/java/dev/jorel/commandapi/nms/NMS_1_18_R1.java
#	commandapi-nms/commandapi-1.19-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_19_Common.java
@willkroboth
Clean up imports after merging
c68c450
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
f48dc47
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
ed01fbf
willkroboth and others added 11 commits February 20, 2023 10:14
@willkroboth
Fix MethodHandle reflection for all versions
3ba0782
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
56ecf24
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
13f94ea 
# Conflicts:
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.15/src/main/java/dev/jorel/commandapi/nms/NMS_1_15.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.17-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_17_Common.java
@willkroboth
Refactor variable names in MethodHandles reflection
e40d9bb
@willkroboth
Merge remote-tracking branch 'origin/dev/argument-exceptions' into de…
b6cc9f7 
…v/argument-exceptions
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
c915f7e 
# Conflicts:
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-core/src/main/java/dev/jorel/commandapi/nms/NMS.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.15/src/main/java/dev/jorel/commandapi/nms/NMS_1_15.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.16.1/src/main/java/dev/jorel/commandapi/nms/NMS_1_16_R1.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.16.2/src/main/java/dev/jorel/commandapi/nms/NMS_1_16_R2.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.16.4/src/main/java/dev/jorel/commandapi/nms/NMS_1_16_4_R3.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.17-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_17_Common.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.18.2/src/main/java/dev/jorel/commandapi/nms/NMS_1_18_R2.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.18/src/main/java/dev/jorel/commandapi/nms/NMS_1_18_R1.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.19-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_19_Common.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.19.3/src/main/java/dev/jorel/commandapi/nms/NMS_1_19_3_R2.java
@willkroboth
Revise reflection in 1_15
f5d6c89
@willkroboth
Add and use SafeVarHandle#getUnknownInstanceType
1b22e55
@willkroboth
Update use of reflection in other ArgumentSerializers
0e25e98
@willkroboth
Fix ArgumentSerializer reflection
4b42d10
@willkroboth
Share code between ArgumentSerializers
868985f
Repository owner deleted a comment from sonarcloud bot Feb 23, 2023
willkroboth and others added 12 commits March 9, 2023 16:07
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
0a195df 
# Conflicts:
#	commandapi-core/src/main/java/dev/jorel/commandapi/arguments/AbstractArgument.java
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
9c27823 
# Conflicts:
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-nms-common/src/main/java/dev/jorel/commandapi/nms/NMS_Common.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-test/commandapi-bukkit-test-impl-1.19.2/src/main/java/dev/jorel/commandapi/test/MockNMS.java
@willkroboth
Don't include Object in the platform load log
fde031f
@willkroboth
Add simple test for ArgumentParseException
a7570d0
@willkroboth
You gotta register the test command to use it!
deda323 
Also other fixes
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
2e2c5dc
@willkroboth
Update 1.19.4 NMS
08f5218
@willkroboth
Fix frozen field access in 1.19 NMS when testing
5652dab
@willkroboth
Fix register method access in 1.19 NMS when testing
b13102c
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
a36f3e0
@willkroboth
Merge branch 'dev/dev' into dev/argument-exceptions
977a2c8 
# Conflicts:
#	commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIHandler.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.15/src/main/java/dev/jorel/commandapi/nms/NMS_1_15.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.16.1/src/main/java/dev/jorel/commandapi/nms/NMS_1_16_R1.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.16.2/src/main/java/dev/jorel/commandapi/nms/NMS_1_16_R2.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.16.4/src/main/java/dev/jorel/commandapi/nms/NMS_1_16_4_R3.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.17-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_17_Common.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.18.2/src/main/java/dev/jorel/commandapi/nms/NMS_1_18_R2.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.18/src/main/java/dev/jorel/commandapi/nms/NMS_1_18_R1.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-test/commandapi-bukkit-test-impl-1.16.5/src/main/java/dev/jorel/commandapi/test/MockNMS.java
#	commandapi-platforms/commandapi-bukkit/commandapi-bukkit-test/commandapi-bukkit-test-impl-1.19.2/src/main/java/dev/jorel/commandapi/test/MockNMS.java
@willkroboth
Provide the parametrized type for SafeVarHandles in ArgumentSerializers
e3d5679
willkroboth added a commit that referenced this pull request Jul 30, 2023
@willkroboth
Remake argument-exceptions branch
eeca0c9 
Add API to use custom error handling when Argument parsing fails

See #370 for the basis of these changes

New changes here:

- Arguments can only have an ArgumentParseExceptionHandler attached if they implement ArgumentParseExceptionArgument
- The substitute value from ArgumentParseExceptionHandler doesn't have to be returned directly
- ExceptionInformation can be provided by arguments
- New NMS method to extract translation keys from CommandSyntaxExceptions
@willkroboth willkroboth mentioned this pull request Aug 2, 2023
Draft
23 tasks
@willkroboth
Copy link
Collaborator Author

Closed in favor of #476

@willkroboth willkroboth closed this Aug 2, 2023
willkroboth added a commit that referenced this pull request Aug 12, 2023
@willkroboth
Remake argument-exceptions branch
30287e2 
Add API to use custom error handling when Argument parsing fails

See #370 for the basis of these changes

New changes here:

- Arguments can only have an ArgumentParseExceptionHandler attached if they implement ArgumentParseExceptionArgument
- The substitute value from ArgumentParseExceptionHandler doesn't have to be returned directly
- ExceptionInformation can be provided by arguments
- New NMS method to extract translation keys from CommandSyntaxExceptions
willkroboth added a commit that referenced this pull request Aug 17, 2023
@willkroboth
Remake argument-exceptions branch
da16444 
Add API to use custom error handling when Argument parsing fails

See #370 for the basis of these changes

New changes here:

- Arguments can only have an ArgumentParseExceptionHandler attached if they implement ArgumentParseExceptionArgument
- The substitute value from ArgumentParseExceptionHandler doesn't have to be returned directly
- ExceptionInformation can be provided by arguments
- New NMS method to extract translation keys from CommandSyntaxExceptions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JorelAli JorelAli JorelAli left review comments

Assignees

@JorelAli JorelAli

Labels
None yet
Projects
CommandAPI Release Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@willkroboth @JorelAli