-
-
Notifications
You must be signed in to change notification settings - Fork 391
ExprMidpointLocations #7888
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?
ExprMidpointLocations #7888
Conversation
src/main/java/ch/njol/skript/expressions/ExprCenterLocations.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprCenterLocations.java
Outdated
Show resolved
Hide resolved
Just going to throw my 2 cents in here. Skript should focus on the language and the Bukkit API itself. Skript shouldn't be putting on focus on doing math that a Skript user could do themselves. Again, I repeat, just my opinion. |
I'm kind of ambivalent on this, since it seems like a really easy thing to do with existing tools. It's a bit clunky since you have to get a vector or do math on x/y/z, though, so i see the appeal of a simpler manner. I don't mind it either way. That said, I'm strongly against calling it the center point. I think that muddles the waters between the existing center expression (and could cause syntax conflicts as is currently written). I would stick entirely to the term I'd like to comment that the pr template does ask you to argue why the given problem needs solving, and I think just the fact that no QOL expression exists is not reason enough. I'd like to see some arguments as to why it needs to be added and why it's better than whatever you can currently do. This is also missing any documentation of testing completed. |
So you want only |
well |
src/main/java/ch/njol/skript/expressions/ExprMidpointLocations.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprMidpointLocations.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprMidpointLocations.java
Outdated
Show resolved
Hide resolved
Class<?> type1 = object1.canReturn(Location.class) | ||
? Location.class : (object1.canReturn(Vector.class) ? Vector.class : null); | ||
Class<?> type2 = object2.canReturn(Location.class) | ||
? Location.class : (object2.canReturn(Vector.class) ? Vector.class : null); |
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 this is the best way to approach this. Consider midpoint between {_vec} and vector(1,2,3)
. Wouldn't {_vec} be able to return a location?
Object object2 = this.object2.getSingle(event); | ||
if (object1 == null || object2 == null) { | ||
return null; | ||
} else if (!parseCheck) { |
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.
checks need to be done regardless, the parse check is not air-tight
Problem
Currently is no quick and easy route of getting the center location from two locations.
The only current way I know to achieve this, is
set {_center} to {_loc1} ~ ((vector from {_loc1} to {_loc2}) / vector(2,2,2))
Solution
Adds
ExprCenterLocations
that does this.Testing Completed
ExprCenterLocations.sk
Completes: none
Related: none