-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 methods for Armadillo #12031
base: main
Are you sure you want to change the base?
Add methods for Armadillo #12031
Conversation
You don't need to specify implicit modifier (public) in interface. Same for your previous PRs. |
paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftArmadillo.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Nassim Jahnke <[email protected]>
paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftArmadillo.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
@Override | ||
public void rollUp() { |
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.
I'm not sure if it's intended but those methods should probably set/reset the DANGER_DETECTED_RECENTLY memory module type otherwise the changes are immediately reverted. Alternatively the key could be exposed in the api ig.
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.
I would argue re/setting the memory and and also exposing the memory key to API would be a good call
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.
yeah i forget about that checks.. i set the value (based in the damage call) and call erase for the roll methods for handle that, also expose the memory key for allow modify this.
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.
currently rollUp looks like working but the rollOut not.. when call rollOut and rollUp again not sure what behaviour im missing for that.
or just maybe document the conditions for roll
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.
I think you need to tweaks a bit more its brain.
The issue is that rollOut is called at the very end of the animation, if you want to keep the animation you would need to adjust the ttl to the duration of the unrolling animation.
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.
yeah but the animation are not a client side thing?
about the brain im a little lost for what touch i try things but still have the issue with rollOut roll out but the Armadillo rollUp again
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.
iirc armadillos have "state ticks" which tell the server to roll out again immediately if not defined properly
Resolve #12026 by adding methods to the State of the Armadillo and expose the roll behaviours in the API.