ExprDirection - deep cleaning #6637
Conversation
| if (relativeObject == null) | ||
| return new Direction[0]; | ||
| if (o instanceof Block) { | ||
| final BlockFace f = Direction.getFacing((Block) o); | ||
| if (f == BlockFace.SELF || horizontal && (f == BlockFace.UP || f == BlockFace.DOWN)) | ||
| return new Direction[] {Direction.ZERO}; | ||
| return new Direction[] {new Direction(f, ln)}; | ||
| } else { | ||
| final Location l = ((Entity) o).getLocation(); | ||
| if (!horizontal) { | ||
| if (!facing) { | ||
| final Vector v = l.getDirection().normalize().multiply(ln); | ||
| assert v != null; | ||
| return new Direction[] {new Direction(v)}; | ||
| } | ||
| final double pitch = Direction.pitchToRadians(l.getPitch()); | ||
|
|
||
| if (relativeObject instanceof Block) { |
There was a problem hiding this comment.
This little guy can probably join the if/else chain :)
| this.amount != null ? amount.toString(event, debug) + " meter(s) " : "", | ||
| (isHorizontal ? "horizontally " : ""), (isFacing ? "facing " : ""), | ||
| this.directionVector != null ? Direction.toString(directionVector) : | ||
| this.relativeTo != null ? "of " + relativeTo.toString(event, debug) : | ||
| Direction.toString(0, yaw, 1) |
There was a problem hiding this comment.
I think it would look nicer if you assigned these variables first rather than having 90 ternaries in the return, it's not like we're saving lines doing this anyway.
There was a problem hiding this comment.
Will see what I can do
| {"[%-number% [(block|meter|metre)[s]]] [to the] south[er(n|ly)|ward[s|ly]] [[side] of] [[and|,] %-direction%]", BlockFace.SOUTH}, | ||
| {"[%-number% [(block|meter|metre)[s]]] [to the] west[er(n|ly)|ward[s|ly]] [[side] of] [[and|,] %-direction%]", BlockFace.WEST}, | ||
| {"[%-number% [(block|meter|metre)[s]]] (above|over|up[ward[s|ly]]) [%-direction%]", BlockFace.UP}, | ||
| {"[%-number% [(block|meter|metre)[s]]] (below|under|down[ward[s|ly]]) [%-direction%]", BlockFace.DOWN}, |
There was a problem hiding this comment.
| {"[%-number% [(block|meter|metre)[s]]] (below|under|down[ward[s|ly]]) [%-direction%]", BlockFace.DOWN}, | |
| {"[%-number% [(block|meter|metre)[s]]] (below|under[neath]|beneath|down[ward[s|ly]]) [%-direction%]", BlockFace.DOWN}, |
| {"[%-number% [(block|meter|metre)[s]]] [to the] west[er(n|ly)|ward[s|ly]] [[side] of] [[and|,] %-direction%]", BlockFace.WEST}, | ||
| {"[%-number% [(block|meter|metre)[s]]] (above|over|up[ward[s|ly]]) [%-direction%]", BlockFace.UP}, | ||
| {"[%-number% [(block|meter|metre)[s]]] (below|under|down[ward[s|ly]]) [%-direction%]", BlockFace.DOWN}, | ||
| {"[%-number% [(block|meter|metre)[s]]] [in [the]] [:horizontal] (direction|:facing) of %entity/block% [of|from]", BlockFace.SELF}, |
There was a problem hiding this comment.
Conflicts with exprfacing, I believe
There was a problem hiding this comment.
Will neee to look into this or ExprFacing as a whole. Iirc, this is a pattern existing before I edited, so it existed for a while.
Optionally, if this is creating conflicts, it might be worth seeing about a merge.
I intend to add opposite facing into a few, and I know there is a pr that wants to add it to expr facing.
Could also remove pattern, but I dont think I want them in two places. Thoughts on if a merge is ideal?
Both return directions, iirc.
There was a problem hiding this comment.
The previous pattern had a required in
|
Re: tests, you can do all the testing in one tick, no time for the entity to turn. |
sovdeeth
added
the
enhancement
|
Converting to draft, due to health issues arising and being unsure how much freetime I'll have during the next week or longer. |
Description
This pr is a continuation of #6626 along side fixing my mistake in it. This pr now target dev/feature as that should be the next release and allowed me to remove some patterns I found in poor taste.
Test is possibly a bit too excessive however with direction being a huge part in skript ensuring nothing breaks is important as such not only is there testing in action but also use of
parsesection ensuring patterns are passing and not randomly breaking down. Sadly the parse section is a huge hinder at the same time as it doesn't explicitly state the line number that errored only the error. So if a test breaks and the error is something like "cannnot use variables here" loading on the test server is kind of needed.Unlike the previous pr this one makes use of the
Patternsclass instead to handle pattern splitting, patterns usingBlockFace.SELFare both technically correct and incorrect sadly since the class refuses to acceptnullas a value and will throw a stack trace I am forced into that decision.ENTITY TESTING
I was unable to think of a safe way to do entity test that ensure there's never a possibility in the entity turning causing a slight offset of locations. As such only block test were included if you have any ideas on how to test entities without unforeseen failures please give me feedback.
Target Minecraft Versions: any
Requirements: none
Related Issues: none