Search Unity

Clear Input.GetKeyDown

Discussion in 'Scripting' started by Arkyris, Jan 25, 2021.

  1. Arkyris

    Arkyris

    Joined:
    Dec 5, 2018
    Posts:
    11
    I'm working on a menu system and running into what I find to be a funky problem. Right now I have three menus. Pause, Build and Units. I have it set up so the menus open when I push P/ESC, B and X respectively. And I want them to close when I push the button again or the ESC key. If I only have the Build and Pause menu it works fine. Ill try to make this as clear as possible.
    (Working scenario, only using Pause and Build menus)
    1) Start game
    2) Push ESC or P. Pause menu opens.
    3) Push B. Build menu won't open. (as intended)
    4) Push ESC or P again. Pause menu closes.
    5) Push B. Build menu opens.
    6) Push B or ESC. Build menu closes. (if I did use ESC to close Build menu the Pause menu won't pop up unless I push ESC again. As intended. And this is where the problem arises if I throw the Units menu into the mix)

    (Problem scenario, throw Units into the mix)
    1) Push X. Units menu comes up.
    2) Push ESC. Units menu closes but Pause also immediately opens instead of needing the second press of the ESC key.

    What I believe is happening is that Input.GetKeyDown is holding onto my key press and the Pause menu happens to run after the Units menu but before the Build menu and that is why this is happening. Is there anyway to clear Input.GetKeyDown after it is used or is there a different method I should be using.

    Build Menu
    Code (CSharp):
    1.     private void Update()
    2.     {
    3.         if (Input.GetKeyDown(KeyCode.B) && !GameManager.OtherMenuOpen && active == false)
    4.         {
    5.             Toggle();
    6.         }
    7.         else if ((active == true && Input.GetKeyDown(KeyCode.Escape)) || (active == true && Input.GetKeyDown(KeyCode.B)))
    8.         {
    9.             Toggle();
    10.         }
    11.     }
    Units Menu
    Code (CSharp):
    1.     private void Update()
    2.     {
    3.         if (Input.GetKeyDown(KeyCode.X) && !GameManager.OtherMenuOpen && active == false)
    4.         {
    5.             Toggle();
    6.         }
    7.         else if ((active == true && Input.GetKeyDown(KeyCode.Escape)) || (active == true && Input.GetKeyDown(KeyCode.X)))
    8.         {
    9.             Toggle();
    10.         }
    11.     }
    Pause Menu
    Code (CSharp):
    1.     private void Update()
    2.     {
    3.         if ((Input.GetKeyDown(KeyCode.Escape) || Input.GetKeyDown(KeyCode.P)) && !GameManager.OtherMenuOpen && active == false)
    4.         {
    5.             Toggle();
    6.         }
    7.         else if ((Input.GetKeyDown(KeyCode.Escape) && active == true) || (Input.GetKeyDown(KeyCode.P) && active == true))
    8.         {
    9.             Toggle();
    10.         }
    11.     }
    Toggle is the same for all
    Code (CSharp):
    1.     public void Toggle()
    2.     {
    3.         ui.SetActive(!ui.activeSelf);
    4.         active = !active;
    5.         GameManager.OtherMenuOpen = !GameManager.OtherMenuOpen;
    6.     }
    active is a private bool in each script. I know I should be able to use ui.activeself but in my couple hours of trying to get this to work I changed that just to see and haven't changed it back yet.
     
    Last edited: Jan 25, 2021
  2. seejayjames

    seejayjames

    Joined:
    Jan 28, 2013
    Posts:
    691
    Not quite sure of all the ins and outs of how you have it set up, but if you eliminate having ESC open the Pause menu, I think that would be simpler. Because it's used for closing other menus as well, that could be throwing things off.

    That said, what you want to do is certainly possible, but something's awry with your bools...somewhere.

    There's no "clear" for GetKeyDown. It's cleared (set to false) every frame, then if there's a key pressed, it's set to true. So that's not the issue (I don't think so anyways...)

    Also, you might want to set all 3 to inactive when you want to open one, then just set that one to active. You may be running into cases where the bool flipping gets off rhythm, if that makes sense.

    The same thing applies to making a row of buttons where only one can be active at once---you *can* store a variable that tracks which is currently active (so you know which one to deactivate when another one is pressed), but you can also just set all of them inactive every time, then activate the last-pressed one.
     
  3. Arkyris

    Arkyris

    Joined:
    Dec 5, 2018
    Posts:
    11
    Thank you for the reply. Yeah I had the thought while trying to fall asleep to put all the handling into its own script called UI switching. This would also clean up the game manager because all menus are referenced by it right now and this would put them all in one place.
    I do believe it is the fact that the key press remains active for the whole frame. I think that when I push escape on the Units menu it handles that and switches the bools then runs the Pause menu script and since the bools were switched it then triggers that.

    I checked that the bools were switching properly with debugs.
     
  4. Bunny83

    Bunny83

    Joined:
    Oct 18, 2010
    Posts:
    3,993
    You have way too much duplicated logic spreaded across too many scripts. You should have a central place to look for the menu closing logic when ESC is pressed. Each menu could handle its own opening and closing though it would make more sense to do this from the central script. The central script should also take care about keeping track of what menu is open. Your boolean "OtherMenuOpen" seems weird ^^. You should simply keep a reference to the currently open menu and when all menus are closed you set this variable to null. That way you don't need those seperate "active" booleans.

    You also do not need seperate scripts for controlling the opening / closing of the menus. Just use the same script on all menus, just make the button configurable. Keep in mind that currently every menu script is checking if it should close itself when esc is pressed. Those scripts will execute in a particular order. If the first script that executes is the currenly open script it will close itself. When the others are processed none will be open so if the pause menu script runs after the others it will open itself since at this point, no other menu is active.

    For example you could do:

    Code (CSharp):
    1. public MenuPanel : MonoBehaviour
    2. {
    3.     public static MenuPanel activeMenu {get; private set} = null;
    4.     public KeyCode openKeyCode;
    5.     void Start()
    6.     {
    7.         MenuHandler.Instance.RegisterPanel(this);
    8.     }
    9.     public void OpenMenu()
    10.     {
    11.         if (activeMenu != null)
    12.             activeMenu.CloseMenu();
    13.         activeMenu = this;
    14.         // your opening logic here
    15.     }
    16.     public void CloseMenu()
    17.     {
    18.         if (activeMenu != this)
    19.             return;
    20.         // your closing logic here
    21.         activeMenu = null;
    22.     }
    23.     public static void CloseAnyMenu()
    24.     {
    25.         if (activeMenu == null)
    26.             return;
    27.         activeMenu.CloseMenu();
    28.     }}
    This script would be attached to each menu panel. It just holds the information about the opening key and contains the logic to open / close a menu. This is generally useful when you need to open a certain menu through other means (interactive tutorials or other ui buttons).

    In addition you would have a central script that does the opening / closing based on key input.

    Code (CSharp):
    1. public class MenuHandler : MonoBehaviour
    2. {
    3.     private static MenuHandler m_Instance = null;
    4.     public MenuHandler Instance { get {
    5.         if (m_Instance == null)
    6.         {
    7.             m_Instance = FindObjectOfType<MenuHandler>();
    8.             if (m_Instance == null)
    9.                 m_Instance = (new GameObject("MenuHandler")).AddComponent<MenuHandler>();
    10.         }
    11.         return m_Instance;
    12.     }}
    13.     private List<MenuPanel> menus = new List<MenuPanel>();
    14.     public void RegisterPanel(MenuPanel aPanel)
    15.     {
    16.         if (!menus.Contains(aPanel))
    17.             menus.Add(aPanel);
    18.     }
    19.  
    20.     void Update()
    21.     {
    22.         if (MenuPanel.activeMenu != null)
    23.         {
    24.             if (Input.GetKeyDown(KeyCode.Escape) || Input.GetKeyDown(MenuPanel.activeMenu.openKeyCode))
    25.                 MenuPanel.CloseAnyMenu();
    26.         }
    27.         else
    28.         {
    29.             foreach(var menu in menus)
    30.             {
    31.                 if (Input.GetKeyDown(menu.openKeyCode))
    32.                 {
    33.                     menu.OpenMenu();
    34.                     break;
    35.                 }
    36.             }
    37.         }
    38.     }
    39. }
    Keep in mind that this is just an example how you can manage several menus with a consistent behaviour.

    Note that you can call OpenMenu at any time on any menu. It will automatically close any open menu before opening that menu. Since activeMenu is a property that can only be set from inside the MenuPanel class it prevents accidental manipulations of the variable. It can only be changed through the OpenMenu / CloseMenu methods. So this setup should be quite robust.
     
    Arkyris likes this.
  5. Arkyris

    Arkyris

    Joined:
    Dec 5, 2018
    Posts:
    11
    Bunny thank you so much! I was going to switch it all to a central MenuHandler but nothing this robust. I really appreciate the help.