-
-
Notifications
You must be signed in to change notification settings - Fork 370
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
Add piglin bartering event #6768
Conversation
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.
just some minor formatting
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.
Looks good, the only thing missing is some tests! This would need JUnit tests, so if you're not comfortable with adding those, please let me know and I can add some for you.
thanks for the offer :) idm trying myself though. do you mean this? if not, where can i find the appropriate format or example? |
Yup, the tests in that folder as well as this package are what you want to look at |
I'm not so sure on how to fix the current error that occurs in <1.16.5 versions, should I use reflection or is there a better and prefered way? It should be visible in the |
Your expressions aren't gated behind class exists checks |
which expressions in which file do you mean? in the java file i have this rn Skript/src/test/java/org/skriptlang/skript/test/tests/syntaxes/events/EvtPiglinBarterTest.java Line 47 in cb4f148
|
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.
looks good, though you should ensure your files end with a new line! (see the red do-not-enter-y mark on github's diffs)
src/test/java/org/skriptlang/skript/test/tests/syntaxes/events/EvtPiglinBarterTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/skriptlang/skript/test/tests/syntaxes/events/EvtPiglinBarterTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Patrick Miller <[email protected]>
Co-authored-by: Patrick Miller <[email protected]>
Co-authored-by: Patrick Miller <[email protected]>
Co-authored-by: Patrick Miller <[email protected]>
Didn't realise this existed until now. Oops! Co-authored-by: Patrick Miller <[email protected]>
Co-authored-by: Patrick Miller <[email protected]>
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.
Great work! A few minor formatting suggestions that can be applied to all syntax files
src/test/java/org/skriptlang/skript/test/tests/syntaxes/events/EvtPiglinBarterTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/skriptlang/skript/test/tests/syntaxes/events/EvtPiglinBarterTest.java
Show resolved
Hide resolved
src/test/java/org/skriptlang/skript/test/tests/syntaxes/events/EvtPiglinBarterTest.java
Outdated
Show resolved
Hide resolved
You can use EventValueExpression to specify that it's exclusively from an event |
Description
Adds support for the PiglinBarterEvent and adds the
barter input
andbarter output
expressions.Target Minecraft Versions: 1.16.5+ (event was added in Spigot 1.16.5)
Requirements: none
Related Issues: none