-
-
Notifications
You must be signed in to change notification settings - Fork 416
Adds interaction entity syntaxes #8291
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/feature
Are you sure you want to change the base?
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.
Haven't looked at code in detail, but this is missing last attacker support
See ExprLastInteractionPlayer, same goes for last attack date in ExprLastInteractionDate. |
No, ExprLastAttacker (unless it already works?) |
Interaction extends Entity, so ExprLastAttacker should work fine (?). Though idk if it would due to getLastDamageCause()? |
sovdeeth
left a comment
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.
lot of edits needed for code quality and conventions
requested comments don't cover everything
|
|
||
| @Name("Is Responsive") | ||
| @Description({ | ||
| "Checks whether an interaction entity is responsive" |
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.
responsive to what? what does this mean?
| isNegated = parseResult.mark == 1; | ||
| return super.init(exprs, matchedPattern, isDelayed, parseResult); |
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.
isnegated is going to be overridden here by the propertycondition init()
| @Override | ||
| protected String getPropertyName() { | ||
| return isNegated ? "unresponsive" : "responsive"; | ||
| } |
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.
| } | |
| } | |
| protected String getPropertyName() { | ||
| return isNegated ? "unresponsive" : "responsive"; | ||
| } | ||
| } No newline at end of file |
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.
| } | |
| } | |
| "set interaction height of last spawned interaction to 5.3" | ||
| }) | ||
| @Since("INSERT VERSION") | ||
| public class ExprInteractionHeight extends SimplePropertyExpression<Entity, Number> { |
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 probably be a type property but that edit can be done in a later pr that adds the property
| } | ||
| } | ||
|
|
||
| } No newline at end of file |
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.
| } | |
| } | |
| final Interaction i = (Interaction) getExpr().getSingle(e); | ||
| if (i == null) | ||
| return; | ||
| float n = ((Number) delta[0]).floatValue(); |
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.
use full variable names please
|
|
||
| @Name("Interaction Responsiveness") | ||
| @Description({ | ||
| "Returns the responsiveness of an interaction entity" |
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.
what's responsiveness
| public class ExprInteractionResponsiveness extends SimplePropertyExpression<Entity, Boolean> { | ||
|
|
||
| static { | ||
| register(ExprInteractionResponsiveness.class, Boolean.class, "interaction Responsiveness[es]", "entities"); |
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.
| register(ExprInteractionResponsiveness.class, Boolean.class, "interaction Responsiveness[es]", "entities"); | |
| register(ExprInteractionResponsiveness.class, Boolean.class, "interaction responsiveness", "entities"); |
Should this even be an expression? Seems more appropriate as an effect + condition.
| protected String getPropertyName() { | ||
| return "interaction responsiveness"; | ||
| } | ||
|
|
||
| @Override | ||
| public Class<? extends Boolean> getReturnType() { | ||
| return Boolean.class; | ||
| } | ||
|
|
||
| @Override | ||
| @Nullable | ||
| public Class<?>[] acceptChange(final ChangeMode mode) { | ||
| if ((mode == ChangeMode.SET || mode == ChangeMode.RESET) | ||
| && getExpr().isSingle()) | ||
| return new Class[] {Boolean.class}; | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public void change(final Event e, final @Nullable Object[] delta, final ChangeMode mode) throws UnsupportedOperationException { | ||
| assert !(getExpr().getSingle(e) instanceof Interaction) || delta != null; | ||
| final Interaction i = (Interaction) getExpr().getSingle(e); | ||
| if (i == null) | ||
| return; |
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.
fix ordering and code quality
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.
As per Sovde's review of following coding conventions, things I've noticed
- Use SyntaxRegistry to register elements
- Forgot to load the InteractionModule via main Skript class
- No
finals - Use
@Exampleinstead of@Examples - Each example should be in it's own
@Example - One liner
@Exampleand@Descriptionshould not be encased in{} - Inline Nullable annotation, i.e.
@Nullable blahorblah @Nullable [] - No 1 letter variable names
- Tests
- Expression change should allow changing of multiple interactions, not just one
Problem
No syntax currently exists for interaction entities. SkBee used to have it, but it is disabled along with its display syntaxes in favor of Skript's.
Solution
This PR introduces several new syntaxes for each aspect of interaction entities.
Includes Height, Width, Responsiveness, Last interaction/attack date/player and a condition for responsiveness.
Testing Completed
Manually tested scripts, all syntax works locally. Can add tests if wanted :D
Supporting Information
None
Completes: none
Related: none