Stepwise rotation occasionally hangs stepwise movement.

I’m building a learning project that uses stepwise movement in a 2.5D space–imagine a first person 3d maze where you can move from square to square in discrete steps. For translation in X,Z I’ve followed what seems to be the standard smooth movement technique. Works like a charm.

When I try and introduce rotation an intermittent bug appears. The ability to translate, to move in the X or Z direction, occasionally stops. Rotation is still possible and eventually will ‘unlock’ translation again, but I can’t spot the pattern of what is causing translation to lock up. I’ve tried to simplify the rotation part as much as possible. The current code doesn’t interpolate it just jumps the transform.rotation as needed.

private bool isMoving; // disable input while executing movement  

private void Update()
    {
        GetInput();
    }

    private void GetInput()
    {
        // NB: Need to enforce one key at a time

        bool turnRight = Input.GetKeyDown("e");
        bool turnLeft = Input.GetKeyDown("q");

        float forwardBack = Input.GetAxis("Vertical");
        float leftRight = Input.GetAxis("Horizontal");

        if (!isMoving)
        {
            if (forwardBack != 0 || leftRight != 0)
            {
                Vector3 direction = MovementDirection(forwardBack, leftRight);
                Move(direction);
            }
            if (turnRight)
            {
                Rotate(90);
            }
            if (turnLeft)
            {
                Rotate(-90);
            }
        }
    }

    private Vector3 MovementDirection(float forwardBack, float leftRight)
    {
        string facing = GetFacingString(); // This method looks at transform.rotate and returns a string (NSEW)
        Vector3 direction = Vector3.zero;

        if (forwardBack > 0)
        {
            // Move forward
            switch (facing)
            {
                case "N":
                    direction = Vector3.forward;
                    break;

                case "S":
                    direction = Vector3.back;
                    break;

                case "E":
                    direction = Vector3.right;
                    break;

                case "W":
                    direction = Vector3.left;
                    break;
            }
        }
        // ... I've truncated the code here for readability. MovementDirection is just a 
        // series of if/switch statements determining the correct Vector3 to return 
        //(Vector3.forward, back, left, right) based on player facing ...//

        return direction;
    }

    private bool Move(Vector3 direction)
    {
        // Scale direction by map scale
        direction = direction * LevelManager.scale;  
        // LevelManager.scale = 5. The cells are 5x5 in size and the player is in the center.
        Vector3 end = transform.position + direction;

        // Add code to test if movement is blocked--return false if fails

        StartCoroutine(SmoothMovement(end));

        return true;
    }

    private IEnumerator SmoothMovement(Vector3 end)
    {
        // create variable to store remaining translation distance
        float sqrRemainingDistance = (transform.position - end).sqrMagnitude;
        isMoving = true;

        while (sqrRemainingDistance > float.Epsilon) // Close enough
        {
            // determine intermediate position. Use Time.deltaTime to calculate total number of 'steps' required.
            Vector3 newPosition = Vector3.MoveTowards(transform.position, end, Time.deltaTime * speed);
            // move player to intermediate position
            transform.position = newPosition;
            // compute new remaining distance;
            sqrRemainingDistance = (transform.position - end).sqrMagnitude;
            yield return null;
        }
        transform.position = end; // avoid any drift
        isMoving = false;
    }

    private void Rotate(float degrees)
    {
        isMoving = true;
      
        //transform.Rotate(new Vector3(0, degrees, 0));
        transform.Rotate(Vector3.up, degrees);

        //StartCoroutine(SmoothRotation(end));

        isMoving = false;
    }

If I eliminate the rotation component I can translate as long as I care to. But enabling the rotation code causes the issue every execution time. Any suggestions on how I can troubleshoot this issue are greatly appreciated.

After reading more thoroughly I’m pretty sure the problem is your Coroutine.

If Move is being called every frame, then you’re trying to start the routine every frame. This is an odd way to do it considering the yield statement in the routine will make the contents of the while loop run every frame, and it will also stack. Instead you should have a variable storing the target point, lerp to that, and if Move is called again, set the current position to the end point and then update the end point with the new direction. This means you should be using Input.GetKeyDown for horizontal and vertical movement instead, because the Horizontal and Vertical axis are continuous inputs whereas your movement scheme is discrete.

To get direction you could refactor your MovementDirection method to this:

    private Vector3 MovementDirection()
    {
        Vector3 direction;
        if(Input.GetKeyDown(KeyCode.W))
        {
            direction = Vector3.forward;
        }else if(Input.GetKeyDown(KeyCode.S))
        {
            direction = Vector3.back;
        }else if(Input.GetKeyDown(KeyCode.A))
        {
            direction = Vector3.left;
        }else if(Input.GetKeyDown(KeyCode.D))
        {
            direction = Vector3.right;
        }
        return direction;
    }

Setting the isMoving flag in the Rotate method is also pointless, this all runs in a single thread (even coroutines), so your application is never using isMoving when that method is called.

With this scheme you can refactor the Move method to this:

    private void Move(Vector3 direction)
    {
        direction = direction * LevelManager.scale;
        // Check if we're moving before updating the end point to stop
        // weird offsets from happening
        if(isMoving)
        {
            transform.position = end;
        }
        end = transform.position + direction;
        isMoving = true;
    }
}

Note: the Vector3 end has been changed to a global variable with this design.

Now the isMoving control flag will tell us when we should be lerping to the end point. In your Update method:

    public void Update()
    {
        if(isMoving)
        {
            transform.position += (end - transform.position) * Time.deltaTime * speed;
            if((transform.position - end).sqrMagnitude < 0.001f)
            {
                transform.position = end;
                isMoving = false;
            }
        }
    }

This will lerp to the endpoint in the same way your coroutine did. I think that’s about it, I probably missed something 'cause I wrote it in the forum editor but hopefully it’s clear enough.

Thank you for taking the time to reply.

I think the Move routine is called once per player input, at least that is the intent. The isMoving flag is supposed to turn off player input and the ability to start a co-routine until the co-routine finishes and then starts the cycle again. So input starts the move method chain once, the co-routine sets the flag when it is first called, and then Update should ignore that code block until the co-routine completes and the co-routine unsets the flag.

I have some code to make the Input.GetAxis(“Vertical”) discrete but your Input.GetKeyDown approach is a better one.

I’m not sure I understand. If I declare this variable as private for the class, does using a co-routine hide that variable in the co-routine?

Thanks.

The Rotate method is called in update at line 27 and 31 of your first posts code snippet. If you think about the flow of the program, it does this:

Line 27, call rotate
isMoving = false;
rotate
isMoving = true;
Back to line 27.

In that one line, isMoving is set to false and then true, but nothing else happens so there’s no point. The operation takes one frame.

It is possible that the coroutine is taking a while to finish, the float.Epsilon value is the smallest non-zero value you could have, hence why I cut it off at 0.001f in my example and set the position to the end after that.

You can add a print(“Coroutine finished.”); statement to the end of the SmoothMovement coroutine, so you can see when it finishes in the console log.

I think my new rule of thumb is Debug.Log everywhere all the time :slight_smile:

I spent the day refactoring the code and taking things in some of the directions you suggested and some other stuff research turned up. I think that my initial problem was due to Quaternion equality comparisons. I had a few if/else if statements where there was no code to catch when the Quaternions being tested didn’t quite line up. Things like:

quaternion1 == Quaternion.Euler(0,90,0);

I modified something I found on the forums into this:

private bool QuaternionsEqual(Quaternion q1, Quaternion q2)
    {
        return (q1.Equals(q2) || (q1 == q2) || (Quaternion.Angle(q1, q2) < .01f));
    }

In the context of my program, a value that was supposed to be assigned based on a Quaternion comparison was ending up null causing the movement code to break as the player’s positioning numbers didn’t add up.

A bit embarrassingly I thought the comparison code was airtight and edited it out of the original post for readability sake. So that’s another leaning moment for me.

There are still issues I’m working out but I think the scope of the initial question has been answered. Thanks a ton–you’ve been very helpful.

Ah yea, in general the only numeric type you should be using the == operator with are ints, bytes, chars, shorts and longs. Doubles, floats, decimals should all be tested with either a threshold like you did above or using inequality operators.

The q1.Equals(q2) || q1==q2 part is redundant, by the way. You only need the third operator using the angle between them.

Cheers. Thanks!