Unity 2D. Why both possible actions are triggered by pressing button?

Question

I'm trying to make it possible to close and open door while player is colliding with door opener. But it just do both - opens and closes by pressing E key. What seems to be the problem?

{
    
    bool DoorTriggered = false;
    bool DoorClosed = true;
    [SerializeField] private DoorOpener door;
    private void Update()
    {
        if (Input.GetKeyDown(KeyCode.E) && DoorTriggered && DoorClosed)
        {
            door.OpenDoor();
            DoorClosed = false;
        }
    
        if (Input.GetKeyDown(KeyCode.E) && DoorTriggered && !DoorClosed)
        {
            door.CloseDoor();
            DoorClosed = true;
        }

    }

    private void OnTriggerEnter2D(Collider2D collision)
    {
        if (collision.TryGetComponent(out PlayerScript _player))
        {
            DoorTriggered = true;
        }
    }
    private void OnTriggerExit2D(Collider2D collision)
    {
        if (collision.TryGetComponent(out PlayerScript _player))
        {
            DoorTriggered = false;
        }
    }
}

You check if the door is closed and the E key pressed then open it. Without taking into that account you then check if the door is open and the E pressed so close it. So it will faithfully do both — BugFinder, Mar 23 '22 at 12:56

score 0 · Accepted Answer · 2022-03-23T16:50:57.440

Your code is running from top to bottom. It starts by running the first line, to the second, and so on. You are checking if the door is closed and then opening it, and then on the next line you are checking if it is open, and then closing it. This causes the door to open and then close.

Solution: You could easily swap the two if statements' places, although it would probably cause some errors. Rather than simply swapping the two, we can use an if-else statement to ensure that you get the solution you need.

{
    
    bool DoorTriggered = false;
    bool DoorClosed = true;
    [SerializeField] private DoorOpener door;
    private void Update()
    {
        if (Input.GetKeyDown(KeyCode.E) && DoorTriggered && DoorClosed)
        {
            door.OpenDoor();
            DoorClosed = false;
        }
    
        else
        {
            door.CloseDoor();
            DoorClosed = true;
        }

    }

    private void OnTriggerEnter2D(Collider2D collision)
    {
        if (collision.TryGetComponent(out PlayerScript _player))
        {
            DoorTriggered = true;
        }
    }
    private void OnTriggerExit2D(Collider2D collision)
    {
        if (collision.TryGetComponent(out PlayerScript _player))
        {
            DoorTriggered = false;
        }
    }
}

That is just as error prone in the other direction now ... what you really should do is simply use [`if - else`](https://stackoverflow.com/a/71590704/7111561)! — derHugo, Mar 23 '22 at 16:24
@derHugo You're right! That would surely make it a lot better for the situation. — , Mar 23 '22 at 16:46

score 0 · Answer 2 · answered Mar 23 '22 at 16:26

This answer explains the issue - your code runs from top to bottom so after the first code block is executed you enter the second condition as well.

However, it is still error prone.

What you actually should rather do is using if - else to call your code blocks exclusively like

private void Update()
{
    if (Input.GetKeyDown(KeyCode.E) && DoorTriggered)
    {
        if(DoorClosed)
        {
            door.OpenDoor();
            DoorClosed = false;
        }
        else
        {
            door.CloseDoor();
            DoorClosed = true;
        }
    }
}

or also

private void Update()
{
    if (Input.GetKeyDown(KeyCode.E) && DoorTriggered)
    {
        if(DoorClosed)
        {
            door.OpenDoor();
        }
        else
        {
            door.CloseDoor();
        }

        DoorClosed = !DoorClosed;
    }
}

Unity 2D. Why both possible actions are triggered by pressing button?

2 Answers2