-
-
Notifications
You must be signed in to change notification settings - Fork 415
Update ExprLeashHolder to use Leashable instead of LivingEntity #8241
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
base: dev/patch
Are you sure you want to change the base?
Update ExprLeashHolder to use Leashable instead of LivingEntity #8241
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.
Would also benefit from some tests, if possible. Test that you can leash a living entity, a non-living entity, and ensure that a non-leashable just returns null and doesn't throw exceptions. See the README in the tests directory for guidance on adding a test file!
Okay will do |
…o feature/update-leash-syntax
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.
looking great
| public boolean check(LivingEntity entity) { | ||
| return entity.isLeashed(); | ||
| public boolean check(Entity entity) { | ||
| if(entity instanceof Leashable leashable) { |
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.
| if(entity instanceof Leashable leashable) { | |
| if (entity instanceof Leashable leashable) { |
| leash {_cow} to {_holder} | ||
| unleash {_cow} |
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.
should assert that the leash holder of cow is properly set after each line
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.
yes this is just a basic template, I was having some trouble with unrelated tests failing after adding this test. Probably some issue with cleanup and state restoring. I'll be adding assertions soon
|
Looks like Leashable isn't available on all versions we support, so you'll have to do some Skript.classExists() checks to make sure it's safe to use it. |
Problem
Currently none of the Leash related syntax work with boats meaning you will have to use Reflect and paper syntax instead.
Solution
Updated
ExprLeashHolderandEffLeashto use Paper'sLeashableinstead of theLivingEntityinterface.Testing Completed
Tested with
quickTest,skriptTestandJunittasksCompletes: #8239