GetKeyUp fires on too many frames

In my game I have several player characters switchable using the tab key. During the update on the Player class I will check (if the current Player class is the active one) if the Tab key is released. If it is, then the game should switch to the next player character
Unfortunately this does not work as expected as sometimes the tab key is released several times

Here’s the interesting code:

Player:

public void Update()
    {
        if (IsActivePlayerInstance)
        {
            HandleInput();
        }
    }

private void HandleInput()
    {
            if (Input.GetKeyUp(KeyCode.Tab))
            {
                GameObject manager = GameObject.FindGameObjectWithTag("Manager"); 
                LevelManager lev = manager.GetComponent<LevelManager>();
                lev.SetActivePlayerNext();
            }
    }

LevelManager:

public void SetActivePlayerNext()
    {
        int nextPlayerId = -1;

        foreach (Player player in PlayerInstances)
        {
            if (player.IsActivePlayerInstance)
            {
                nextPlayerId = player.PlayerId + 1;
            }
        }

        if (nextPlayerId > PlayerInstances.Count)
        {
            nextPlayerId = 1;
        }

        SetActivePlayerInstance(nextPlayerId);
    }

   public void SetActivePlayerInstance(int playerId)
    {
        foreach (Player player in PlayerInstances)
        {
            if (player.PlayerId == playerId)
            {
                player.IsActivePlayerInstance = true;
            }
            else
            {
                player.IsActivePlayerInstance = false;
            }
        }
    }

In the player class there is a property IsActivePlayerInstance, which is only changed in the above LevelManager methods

Under correct circumstances it should switch between the player characters like this:
Player1 → Player2 → Player3 → Player1…

Unfortunately sometimes it will do like this:
Player1 → Player2 → Player3 → Player2 → Player3…
Or when I just added a few more players:
Player1 → Player3 → Player5 → Player1 → Player3…

I can replicate the same situation if I don’t change playerId on the player characters in the scene. But if I e.g. change the id of player2 to player3 and player3 to player2 it will a completely random result that will be repeated until I change the playerids again.

What I mean is this:

I had 5 characters in the scene, played 5 games and the tab result was this:

Player1 → Player2 → Player3 → Player4 → Player5 → Player2 → Player3 (always skipping player1)

Then I changed the id of Player1 to 5 and the id of player5 to 1 and played 5 games. All games gave this result now:

Player1 → Player3 → Player5 → Player1 → Player3 → Player5 → Player1 (always skipping player2 and player4)

Anybody who has any idea on what the problem could be here?
All player characters are from the same prefab, only changing the playerid

“sometimes the tab key is released several times”

Are you 100% sure you get the key up event several times? I don’t get that behaviour.

if (Input.GetKeyUp(KeyCode.Tab))
    print("Tab released at frame " + Time.frameCount);

Could be that the events work as intended but the problem is in your code that is executed when it happens:

GameObject manager = GameObject.FindGameObjectWithTag("Manager"); 
LevelManager lev = manager.GetComponent<LevelManager>();
lev.SetActivePlayerNext(); // Well, most likely in here, right?

Anyway, if for some reason your keyboard or operating system is sending multiple events, I guess you can add a time guard.

bool canUseTab = Time.time > safeToTabAgain;
if (canUseTab && Input.GetKeyUp(KeyCode.Tab))
{
    safeToTabAgain = Time.time + 0.2f; // It's ok to release tab again in 0.2 seconds.
    // Rest of your code.
}

Here’s my guess on what’s happening:

  • You’re on Player1 and release Tab.
  • Player1 sees the Tab released and that it’s the active instance, so it tells the level manager to switch to the next one.
  • Level manager switches active instance to Player2
  • Player2 gets it’s Update() cycle, it’s on the same game frame so it sees Tab has been released and that it’s the active instance, so it tells the level manager to switch to the next one.

My suggestion is to have a bool in the player code, something like ‘hasJustSwitched’. When the level manager sets the active instance, it also sets that bool on it. The instance then ignores any Tab if that bool is set, and puts it false at the end of the HandleInput().

public void Update()
     {
         if (IsActivePlayerInstance)
         {
             HandleInput();
         }
     }
 
 private void HandleInput()
     {
             if (Input.GetKeyUp(KeyCode.Tab) && !hasJustSwitched)
             {
                 GameObject manager = GameObject.FindGameObjectWithTag("Manager"); 
                 LevelManager lev = manager.GetComponent<LevelManager>();
                 lev.SetActivePlayerNext();
             }
             hasJustSwitched = false;
     }

Then in level manager:

public void SetActivePlayerInstance(int playerId)
     {
         foreach (Player player in PlayerInstances)
         {
             if (player.PlayerId == playerId)
             {
                 player.IsActivePlayerInstance = true;
                 player.hasJustSwitched = true;
             }
             else
             {
                 player.IsActivePlayerInstance = false;
             }
         }
     }

Thanks, that fixed the problem. The problem was probably like OncaLupe say, But looking at the time I could make sure that a certain amount of time has passed between each press.