Applied Energistics 2

Applied Energistics 2

137M Downloads

Platform.rotateAround inconsistent rotation behavior

fdevx opened this issue ยท 1 comments

commented

What it does at the moment

It seems that this method(s) rotates a face counter clock wise (CCW) to a given axis.
( This applies to both methods the one that takes EnumFacing and the one that wants AEPartLocation)
However this is only the case for the X and Y axis.
If you are rotating around the Z axis this method performs a clockwise (CW) rotation instead

Why should this be fixed?

While this does not cause any crashes that I am aware of, this method is very hard to read and has no documentation about which direction it is rotating the faces. This means a CW rotation of a face when a CCW rotation was expected could very well cause some nasty very very hard to find bugs.
Furthermore juts because a broken (or ill documented) method did not cause any issues yet does not mean there wont be any in the future. You should always want to decrease your technical debt whenever possible.

Proposed fix

Since I'm not a fan of huge methods which only contain switch statements (because these tend to attract bugs like nothing else) I propose a mathematical solution for the method
Direction rotateAround( final Direction forward, final Direction axis ) the other method should be similar:

if (forward.getAxis() == axis.getAxis())
{
	// what we want to rotate is on the same axis we want to rotate around
	return forward;
}

// Perspective: looking at the cube from the axis face
// X Axis -> AxisDirection.POSITIVE -> CCW rotation (NORTH -> UP -> SOUTH -> DOWN)
// Y Axis -> AxisDirection.POSITIVE -> CCW rotation (NORTH -> WEST -> SOUTH -> EAST)
// Z Axis -> AxisDirection.POSITIVE -> CC rotation (EAST -> DOWN -> WEST -> UP) !!! wtf

Vec3i rotVec;
if( axis.getAxis() == EnumFacing.Axis.Z)
{
	rotVec = axis.getOpposite().getDirectionVec();
} else
{
	rotVec = axis.getDirectionVec();
}
Vec3i vec = forward.getDirectionVec();

int rz = rotVec.getZ();
int ry = rotVec.getY();
int rx = rotVec.getX();


int px = vec.getX();
int py = vec.getY();
int pz = vec.getZ();

int resX = Math.abs(rx) * px - rz*py + ry *pz;
int resY = rz*px + Math.abs(ry)*py - rx*pz;
int resZ = -ry*px + rx *py + Math.abs(rz)*pz;

return EnumFacing.getFacingFromVector( resX, resY, resZ );

Why not just use EnumFacing.rotateAround?

Well you could but not for long. If I'm not mistaken this method might be gone if you want to port to 1.13 (It definitely does not exist for 1.15).
So sooner or later you have to use this or a similar approach if you agree with me that this method cannot stay like this. Why not make the code forwards compatible now if you have the chance?

commented

The whole point of this method is just to turn something around an axis in 90 degree steps. The direction is completely irrelevant as long as it isn't randomized each call.

The behaviour is simply caused by saving the orientation of a block in a weird way with two EnumFacings. A more approachable way like storing the facing plus a second enum representing [0,90,180,270] degrees of rotation would certainly be the better solution.
Instead of turning the current semi lookup table into some vector math approach. Which should not be more complex than (currentRotation + 90) mod 360. Which certainly hides the intention even more and increases the technical debt even more.