Unity Custom InputManger for Cross-Platform Input

Question

I have been working on my game for quite a while. one of its key features is cross-platform local multiplayer for example 2 players on 1 keyboard and some people on the controller.

here is the 4th Iteration

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.UI;
using UnityEngine.SceneManagement;

public class GameManager : MonoBehaviour
{
    public GameObject playerPrefab;
    public GameObject[] Players;
    public List<string> playerOrderString = new List<string>() { };
    public bool resetPlayerSelect;
    public Image UIImage;
    public float fadeTime = 1.0f;
    public GameObject MainCamera;
    public bool isGameRunning = false;
    #region  Spawnner
    public GameObject spawnnerGameObject;
    public GameObject[] objectsToSpawn;
    public GameObject[] ObjectsToSpawnOnFloor;
    public float[] SpawnRateOfObjectsPerFloor;
    #endregion

    void Start()
    {
        MainCamera = GameObject.FindWithTag("MainCamera");
        isGameRunning = false;
        Time.timeScale = 0.0f;
    }
    void Update()
    {
        if (isGameRunning == false)
        {
            InputCheck();
            AssignPlayerInput();
            if (playerOrderString.Count != 0)
                if (Input.GetButtonDown("Select" + playerOrderString[0]))
                {
                    SceneManager.LoadScene(0, LoadSceneMode.Single);
                }
        }
        else if (isGameRunning == true)
        {
            if (Input.GetButtonDown("Start" + playerOrderString[0]))
            {
                PauseOrResumeGame();
            }
        }
    }
    public bool RestartGame = false;
    void PauseOrResumeGame()
    {
        if (isGameRunning == true)//this pauses the game
        {
            Time.timeScale = 0.0f;
            isGameRunning = false;
        }
        else if (isGameRunning == false)
        {
            Time.timeScale = 1.0f;//this resumes the game
            isGameRunning = true;
        }
        else
            Debug.Log(Time.timeScale);
    }


    void InputCheck()
    {
        if (playerOrderString.Count == 0)
            return;
        if (resetPlayerSelect || Input.GetButtonDown("resetInput"))
            ResetInput();
        else if (Input.GetButtonDown("Start" + playerOrderString[0]) && GameObject.FindWithTag("MazeGenerator") == null)
            StartGame();
        else if (Input.GetButtonDown("Start" + playerOrderString[0]))
            PauseOrResumeGame();
    }
    public GameObject floorSet;
    void StartGame()
    {

        isGameRunning = true;
        Time.timeScale = 1.0f;
        Instantiate(floorSet, new Vector3(0, -0.45f, 0), Quaternion.identity);
        playerOrderString.RemoveAll(item => item == null);
        PlayersHasBeenSelected();
        StartCoroutine(FadeOutStartScreen(fadeTime, 0.0f));
        Players = new GameObject[playerOrderString.Count];
        InstantiatePlayers();


    }
    void InstantiatePlayers()
    {
        transform.position -= new Vector3((Players.Length - 1) * 2, 0, 0);
        for (int i = 0; i < Players.Length; i++)
        {
            Players[i] = Instantiate(playerPrefab, transform.position, Quaternion.identity) as GameObject;
            Instantiate(spawnnerGameObject, transform.position, Quaternion.identity);
            transform.position += new Vector3(4, 0, 0);
            Players[i].GetComponent<PlayerMovement>().playerInput = playerOrderString[i];
            Players[i].GetComponent<PlayerMovement>().playerText = PlayersText[i];
            Players[i].GetComponent<PlayerMovement>().PlayerNumber = i + 1;
            StartCoroutine(WaitUntilScreenFades(fadeTime, i));


        }
        for (int i = 0; i < objectsToSpawn.Length; i++)
        {
            Instantiate(objectsToSpawn[i]);
        }

        MainCamera.GetComponent<CameraController>().enabled = true;
    }
    IEnumerator WaitUntilScreenFades(float fadeTime, int i)
    {
        for (float t = 0.0f; t < 1.0f; t += Time.deltaTime / fadeTime)
        {
            yield return null;
        }
        Players[i].GetComponent<PlayerMovement>().enabled = true;
    }

    IEnumerator FadeOutStartScreen(float fadeTime, float fadeToValue)
    {
        float alpha = UIImage.color.a;
        for (float t = 0.0f; t < 1.0f; t += Time.deltaTime / fadeTime)
        {
            Color newColor = new Color(1, 1, 1, Mathf.Lerp(alpha, fadeToValue, t));
            UIImage.color = newColor;
            yield return null;
        }
    }
    void ResetInput()
    {

        playerOrderString = new List<string>() { };
        resetPlayerSelect = false;
        {
            for (int i = 0; i < 4; i++)
                PlayersText[i].text = "Player Not Selected";
        }
        StartCoroutine(FadeOutStartScreen(fadeTime, 1.0f));
    }
    void AssignPlayerInput()
    {
        for (int unAssignedPlayer = playerOrderString.Count; unAssignedPlayer <= playerOrderString.Count; unAssignedPlayer++)
        {
            if (playerOrderString.Count >= 0)
            {

                CheckInputFor("K", 2);
                CheckInputFor("C", 4);


            }
        }
    }
    void CheckInputFor(string repFor, int times)
    {

        for (int input = 0; input < times; input++)
        {
            string playerOrderValue = repFor + (input + 1);
            string playerThisTime = "Fire1" + playerOrderValue;

            if ((Input.GetButtonDown(playerThisTime)))
            {
                if (CheckForDuplicateInput(playerOrderValue))
                {
                    playerOrderString.Add(playerOrderValue);
                    PlayersHasBeenSelected();
                }
                else
                    return;

            }
        }
    }
    bool CheckForDuplicateInput(string playerOrderValue)
    {
        for (int i = 0; i < playerOrderString.Count; i++)
        {

            if (playerOrderString[i] == playerOrderValue)

            {
                return false;//isDupplicate
            }
        }
        return true;//not dubliclate
    }

    public Text[] PlayersText;
    void PlayersHasBeenSelected()
    {
        for (int i = 0; i < playerOrderString.Count; i++)
        {
            PlayersText[i].text = "Player " + (i + 1) + " is " + playerOrderString[i];
        }

    }
}

As of now in my 4ths Iteration, the code works fine but I would like to optimize it further since it still seems a bit inefficient to me

I will explain a bit more about how my code works

In unity input manager I have set up controls for 6 types of controllers which follow the pattern of thisInputK1, thisInputK2, thisInputC1... and so on where K denotes keyboard input and C denotes Controller Input.

The numbers after that denote the different sets of keys the input requests, for example, VecticalK1 is the same as VecticalK2 but K1 uses WASD while K2 uses arrow keys same is for the controller but they use separate controllers.

after a player 1 is assigned if player one presses their start button the input manager feeds controls to game manager and the game begins

Answer 1

Firstly, this bit of advice is not correct:

the reason why you might be missing the Input during some frames, might have to do with the fact that the IEnumerator is taking longer to yield

I see this a lot, where folks think that if an update method or coroutine is slow, it will somehow "fall behind" the rest of the game loop, as stuff happens in the game faster than the slow code checks for it.

Unity doesn't work that way. Update and coroutine scripts run sequentially on the main thread as part of the game loop. If one of these scripts runs slow, the game loop can't continue on, read new inputs, and render new frames. It stalls until the script finishes. So the symptom of a slow script is low framerate - but all the inputs that happen in the duration of that long frame will still be picked up on the next frame.

Rather, I think your missed inputs came from your first version of your code, where you changed the value of repfor on alternating frames. Effectively this meant you looked only at the keyboard on odd frames, and only at the controller on even frames. So if the player presses a keyboard key on an even frame, or a controller button on an odd frame, you ignore it. This doesn't appear to be an issue in your updated code.

---

Now, as for what to do with the updated code: I would throw this out wholesale and start again. The flow of this class is a confusing, it does gobs of redundant work, and it allocates garbage with reckless abandon with all its temporary string construction.

We can do much better.

Some strategies I'm going to use here:

1. Single Responsibility Principle: If this is the InputManager, then let's make it responsible only for input. The GameManager can be our state machine for determining when to show/hide UI (or it can delegate this to a MenuManager), when it's valid to accept players joining, and when it's valid to accept a start game request.

   You'll likely have other screens that want to transition in & out (like a victory/end-of-match screen), so centralizing that responsibility elsewhere also helps us stay DRY.

2. Make GameManager and InputManager singletons: that way we can find them in constant time with GameManager.GetInstance() instead of searching every time we want to use them. (Or you could hold a reference to the managers you need as a member variable, if you feel so inclined)

3. No string construction in the game loop: instead, let's define a clear data structure that holds all the strings we need, and just re-use them each frame. You can still populate this structure in code at start-up if you want.

4. No redundant work: we won't check the same input again and again for each player slot. We won't check an already-assigned input again for assigning a new player. And so we won't have to do extra work to check for duplicate mappings.

   Instead we're going to partition our possible buttons into two sets:

   • those that are still unassigned: we'll check each unassigned fire button once, and assign it to the next player in line if pressed this frame.

   • those that have been assigned: we'll check the assigned start buttons once, to decide whether it's time to start the game.

Here's what this code can look like.

using UnityEngine;
using UnityEngine.Events;

public class InputManager : MonoBehaviour {

    // Define a data structure for a set of input keys
    // that belong together as a logical unit.
    [System.Serializable]
    public struct InputIdMap {
        public string Fire;
        public string Start;
        public string Horizontal;
        public string Vertical;
        // ... etc,
    }

    // Singleton implementation so it's easy for other scripts to find us.
    static InputManager _instance;
    public static InputManager GetInstance() {
        if(_instance == null) {
            _instance = FindObjectOfType<InputManager>();
        }
        return _instance;
    }

    public int AssignedPlayersCount { get; private set; }

    // Wire up listeners in the Inspector or scripts to trigger stuff to happen
    // when a player joins and is assigned an input slot.
    public UnityEvent<int> onPlayerAssigned;

    // Populate this in the inspector to include all the logical control
    // groupings you use, or use a script to stamp them out in Awake().
    [SerializeField]
    private InputIdMap[] _inputSets;    

    // Other scripts can ask us for the set of input strings a given player is using.
    public InputIdMap GetInputsAssignedToPlayer(int playerId) {
        // Trap any attempts to get input for players we haven't assigned yet.
        Debug.AssertFormat(playerId >= 0 && playerId < AssignedPlayersCount,
            "Tried to get input for player {0} when only {1} player(s) have been assigned",
            playerId, AssignedPlayersCount);

        return _inputSets[playerId];
    }

    // GameManager can call this when it's time to reset the assigned players.
    public void ResetPlayers() {
        AssignedPlayersCount = 0;
    }

    void Update() {
        // We'll let the GameManager be the authority about game state.
        // It will dictate when we scan for joining inputs / start game inputs.
        var game = GameManager.GetInstance();
        if (game.allowJoining) {
            // We'll maintain the invariant that the bottom entries of
            // the _inputSets array represent inputs that have been
            // assigned to a player, in order by the player assigned.

            // Iterate over only the unassigned inputs, listening to
            // the "Fire" input to activate them and assign them to a player.
            for (int i = AssignedPlayersCount; i < _inputSets.Length; i++) {
                var inputToCheck = _inputSets[i];
                // If Fire is pressed this frame, swap this to our next
                // unassigned player slot, and increment our assigned players.
                if (Input.GetButtonDown(inputToCheck.Fire)) {
                    _inputSets[i] = _inputSets[AssignedPlayersCount];
                    _inputSets[AssignedPlayersCount] = inputToCheck;
                    AssignedPlayersCount++;

                    // Let listeners know we assigned a player at this ID.
                    // Now they can spawn the player, play sound/VFX...
                    onPlayerAssigned.Invoke(AssignedPlayersCount - 1);
                }
            }
        }

        if (game.readyToStart) {
            // We can allow any player to start the game, by listening
            // for Start presses from any input in the assigned group.
            for (int i = 0; i < AssignedPlayersCount; i++) {
                if (Input.GetButtonDown(_inputSets[i].Start)) {
                    game.StartGame();
                    break;
                }
            }

            // Or, if you want only Player 1 to be able to start the game:
            //if (_playersAssigned > 0 && Input.GetButtonDown(_inputSets[0].Start))
            //    game.StartGame();
        }
    }
}

Answer 2

I see a lot of inefficient code in here.

Firstly, the StartGame method gets called every frame. So the GameObject.FindWithTag and the GetComponent method are being called unnecessarily on every frame. I get that it might not go inside the if condition in every frame, but why not cache the GameObject and the Component in some variables?

Secondly, you are recreating the playerOrderString list every time in the ResetInput method. But as far as I can see, all you’re doing is removing null items from the list, or if it is “not selected”. So then why not use a simpler query like placeOrderString.FindAll(x => x != null && x != “not selected”);? Then in this case, you can just declare the list once, and be done with it.

Thirdly, the reason why you might be missing the Input during some frames, might have to do with the fact that the IEnumerator is taking longer to yield. I don’t recommend using the IEnumerator like that in Update.

Thirdly, it’ll be hard to say why there may be lag (point out the cause specifically). I would suggest using the Unity Profiler to figure out what is happening exactly on the frames where you notice lag.