Transform EulerAngles Error

Question

I have a condition that makes a check. Like this:

if (moveDown)
        {
            Debug.Log("hit1");
            if (RightTowers[rightIndex].transform.GetChild(2).transform.eulerAngles.z <= -40f)
            {
                moveDown = false;
                moveUp = true;
            }
            else
            {

                RightTowers[rightIndex].transform.GetChild(0).gameObject.GetComponent<SpriteRenderer>().sprite = Mirrors[mirrorIndex++];
                rotateAngle = Quaternion.Euler(0f, 0f, RightTowers[rightIndex].transform.GetChild(2).eulerAngles.z - angle);
                RightTowers[rightIndex].transform.GetChild(2).transform.rotation = rotateAngle;
            }
        }
        if (moveUp)
        {
            Debug.Log("hit2");
            if (RightTowers[rightIndex].transform.GetChild(2).transform.eulerAngles.z >= 40f)
            {
                moveDown = true;
                moveUp = false;
            }
            else
            {

                RightTowers[rightIndex].transform.GetChild(0).gameObject.GetComponent<SpriteRenderer>().sprite = Mirrors[mirrorIndex--];
                rotateAngle = Quaternion.Euler(0f, 0f, RightTowers[rightIndex].transform.GetChild(2).eulerAngles.z + angle);
                RightTowers[rightIndex].transform.GetChild(2).transform.rotation = rotateAngle;

            }
        }

The problem is that when the object is rotated to -40 degrees, and the condition is checked it does not deactivate moveDown and activate moveUp. He further rotates the object with angle index. I do this rotation when I click on a button. He must, when he reaches -40 degrees, deactivate me moveDown. Why not disable it?

score 0 · Answer 1 · answered Mar 13 '18 at 16:54

First things first, create some intermediate fields, your code is really heavy, and you're actually accessing the data each time because of all the nested calls you are making, and it's really making it unclear, especially for anyone outside of your project.

Secondly, I don't see the point of having 2 booleans which are the exact opposite value of each other.. you'd get the same result having just one since they are inversed.

That being said, I believe (from what I got from your snippet) that you're probably not in the degree range you might think ? I don't know if you have verified that but, your character could look totally normal, exactly like you have place him originally, but he might have a 360, 720 etc... degree rotation applied(can be negative too), so if you are not looking at the rotation between initial position and desired, you might not get the right values, maybe do a RotationDifference() func, or if (rotation == 360 or -360) rotation = 0 I'm just throwing out ideas, you really have many ways of doing it.

Lastly, your code seems quite heavy for a seemingly mundane task, you should look into that. Try to abstract your tasks, you are trying to do everything at once, instead of having sturdy systems taking care of it.

Pac0 · Answer 2 · 2018-03-13T18:43:09.977

(new answer after comments)

The logic you want is not appearing "clearly" while reading the code, this makes difficult to debug and understand what is wrong !

As per your comment, I understand that you need move back and forth from -40 to 40.

First, we need to clearly separate the tasks

The two separate tasks are :

Decide in which direction tower is moving next step.
Move the tower up or down. This is basically the two 3-lines blocks of code. Let's call these blocks "MoveUp()" and "MoveDown()"

Now, write some pseudo code for it

// pseudo code

// First, decide in which direction to go
if current angle is 40° (or more), 
    set direction to down for next move. 
else if current angle is -40 (or less), 
    set direction to up for next move. 
else 
    just continue in the current direction

// Second, move the tower
if direction is down, move down
else move up.

Let's translate this to real C# code

// suppose you have a private bool variable in your class
// called "isMovingDown"
private bool isMovingDown = false; // inital value, could be true if you want

// call this in your Update or FixedUpdate logic
private void MoveYourTowerLogic() {
    // First : decide in which direction to go :
    var currentAngle = RightTowers[rightIndex].transform.GetChild(2).transform.eulerAngles.z;

    if (currentAngle >= 40f) 
    {
        isMovingDown = false;
    }
    else if (currentAngle <= -40f)
    {
        isMovingDown = true;
    }
    // else current angle is between 40 and -40 
    // we simply need to o nothing, because we want to keep the previous direction set.
    // remember, the variable movingDown is declared at the class level, 
    // so it 'remembers' the last value set from last call


    // Second : Move the tower
    if (isMovingDown)
    { 
        MoveDown();
    }
    else // tower is moving up
    {
        MoveUp();
    }
    // NOTE : I would simply create one function Move(int angle)
    // to simplify this much further and avoid to write all this mostly useless code before
    // but I want to keep this simple to understand for now
}

private void MoveDown()
{
    // logic to move tower down
}

private void MoveUp()
{
    // logic to move tower up
}

Notice how reading this is much more "human-friendly" now, you can now, I hope, read this more easily, and that will help you modify it if you want to do more complex things later.

I strongly suggest you to avoid writing the same code several time. You can notice how I simplified and wrote only once the logic to get the angle into a variable named exactly like what I was thinking in my pseudo code.

Also, to write the methods "MoveUp" and "MoveDown", you can simply copy paste your actual code into them. But then after, you could even refactor them to avoid code repetition.

Like this :

(Edit)

Let's refactor this further

Basically, the moving logic is a completely separate task, and the only difference between moving up and down is the + or - angle.

Also, maybe you want to move your towers with a different angle later. It make sense to say that the rotation of tower depends only on the angle.

So let's create a method for that has only the angle as parameter.

With something like, instead of MoveUp() and MoveDown() :

private void MoveTower(int angle) 
{
   RightTowers[rightIndex].transform.GetChild(0).gameObject.GetComponent<SpriteRenderer>().sprite = Mirrors[mirrorIndex++];
   rotateAngle = Quaternion.Euler(0f, 0f, RightTowers[rightIndex].transform.GetChild(2).eulerAngles.z + angle);
   RightTowers[rightIndex].transform.GetChild(2).transform.rotation = rotateAngle;
}

The second part will be greatly simpler :

    // Second, move the tower
    MoveTower(isMovingDown ? angle : -angle);

the problem is that I rotate from 40 to 0 and from 0 to -40, then the other way around — Sergiu A, Mar 13 '18 at 17:07
@SergiuA I rewrote my answer, also with the thinking process and using pseudocode. Hope it helps ! — Pac0, Mar 13 '18 at 17:55

Transform EulerAngles Error

2 Answers2

First, we need to clearly separate the tasks

Now, write some pseudo code for it

Let's translate this to real C# code

Let's refactor this further