-
-
Notifications
You must be signed in to change notification settings - Fork 373
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.
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
Added Unknown command event #6534
base: dev/feature
Are you sure you want to change the base?
Conversation
i'd merge under dev/feature |
idk why it didn't merge it |
a team member has to approve it |
how would i mention it to the issue that is related to it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution ⚡
Here are few notes :)
We should also add support for this in ExprCommand and likely add some support in EvtCommand (like adding a new syntax to ignore unknown commands)
src/main/java/ch/njol/skript/expressions/ExprUnknownCommandMsg.java
Outdated
Show resolved
Hide resolved
oh ight, i'll check notes in the morning |
Done, i did all notes I put the event in SimpleEvents, and the message in ExprMessage. I committed the changes but how do i show them here |
You need to push them to your fork on branch |
i think i did it? it should be committed here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also add support for event values
I did the things you said but for the check if class exists, the event isn't paper, its bukkit |
there |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for your contribution! can you please add an event value for the command sender, and support for ExprCommand/ExprArgument? let me know if you need any help
There, tell me if something's wrong sorry for having all these, its my first time :) |
Oh wait, how should i add a player event-value to the event? since it doesn't have its own class |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test should still be added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could have a test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the JUnit have any 'ensure's set up to ensure that the unknown command event gets called and returns what is thought of for the JUnit?
on unknown command: | ||
set unknown command message to "Hey, this command does not exist." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test won't ever get called on its own, you need to call the event in the java JUnit package and then ensure that you can modify the unknown command message correctly. look at other expressions for examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this fire for console commands? You might be able to get away with execute console command "fakecommand"
in a test
Co-authored-by: LimeGlass <[email protected]>
on unknown command: | ||
set unknown command message to "Hey, this command does not exist." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a test.
Adds the
[player] unknown command
event to skriptwith event-value
unknown command( |-)message
Related Issue: #1655