Unity-Technologies / InputSystem

An efficient and versatile input system for Unity.
Other
1.44k stars 312 forks source link

[API Proposal] Basic input slice of the new high level API #1559

Closed andrew-oc closed 1 year ago

andrew-oc commented 2 years ago

Background and Motivation

This API is an effort to simplify using the Input System for new users, small projects, and/or experienced users/teams starting new projects. It should be possible for all users to implement basic input functionality in a zero-configuration, everything-just-works, environment. New users should have to learn a minimum number of new concepts to get started, without having to read any documentation, but existing users of the legacy input manager should feel some level of familiarity with the interface. The intent is also that it is possible to transition with minimum effort from this high level API into the lower level APIs piecemeal as a game progresses.

This slice of the high level API includes methods for users to get basic input only (read: no actions, maps, processors, interactions, bindings, assets, control schemes, action types, disambiguation, callbacks, composites, or any other low level concepts are involved).

Proposed API

namespace UnityEngine.InputSystem
{
  public static partial class Input
  {
    public static IReadOnlyList<Gamepad> gamepads { get; }
    public static IReadOnlyList<Joystick> joysticks { get; }
    public static ReadOnlyArray<InputSlot> gamepadSlotEnums { get; }
    public static int maxGamepadSlots { get; }

    public static bool IsPressed(Inputs input, InputSlot slot = InputSlot.All);
    public static bool WasPressedThisFrame(Inputs input, InputSlot slot = InputSlot.All);
    public static bool WasReleasedThisFrame(Inputs input, InputSlot slot = InputSlot.All);

    public static bool IsPressed(GamepadButton button, InputSlot slot = InputSlot.All);
    public static bool WasPressedThisFrame(GamepadButton button, InputSlot slot = InputSlot.All);
    public static bool WasReleasedThisFrame(GamepadButton button, InputSlot slot = InputSlot.All);

    public static bool IsPressed(JoystickButton button, InputSlot slot = InputSlot.All);
    public static bool WasPressedThisFrame(JoystickButton button, InputSlot slot = InputSlot.All);
    public static bool WasReleasedThisFrame(JoystickButton button, InputSlot  slot = InputSlot.All);

    public static float GetAxis(Inputs input, InputSlot slot = InputSlot.All);
    public static float GetAxis(Inputs negativeAxis, Inputs positiveAxis);

    public static Vector2 GetAxis(Inputs left, Inputs right, Inputs up, Inputs down);
    public static Vector2 GetAxisRaw(Inputs left, Inputs right, Inputs up, Inputs down);
    public static Vector2 GetAxis(GamepadAxis stick, InputSlot inputSlot= InputSlot.All);
    public static Vector2 GetAxis(InputSlot joystickSlot, float deadzone = kDefaultJoystickDeadzone);

    public static void SetGamepadTriggerPressPoint(float pressPoint, InputSlot gamepadSlot = InputSlot.All);
    public static void SetGamepadStickDeadzone(float deadzone, InputSlot gamepadSlot = InputSlot.All);

    public static bool IsGamepadConnected(InputSlot gamepadSlot);
    public static bool DidGamepadConnectThisFrame(InputSlot gamepadSlot);
    public static bool DidGamepadDisconnectThisFrame(InputSlot gamepadSlot);

    public static Vector2 pointerPosition { get; }
    public static bool pointerPresent { get; }
    public static Vector2 scrollDelta { get; }

    public enum GamepadAxis
    {
        LeftStick,
        RightStick
    }

    public enum GamepadButton
    {
      DpadUp,
      DpadDown,
      DpadLeft,
      DpadRight,
      North,
      East,
      South,
      West,
      LeftStickButton,
      RightStickButton,
      LeftShoulder,
      RightShoulder,
      LeftTrigger,
      RightTrigger,
      Start,
      Select,
      X = West,
      Y = North,
      A = South,
      B = East,
      Cross = South,
      Square = West,
      Triangle = North,
      Circle = East
    }

    public enum JoystickButton
    {
        Trigger,
        Button2,
        Button3,
        Button4,
        Button5,
        Button6,
        Button7,
        Button8
    }

    public enum GamepadSlot
    {
        Slot1 = 0,
        Slot2,
        Slot3,
        Slot4,
        Slot5,
        Slot6,
        Slot7,
        Slot8,
        Slot9,
        Slot10,
        Slot11,
        Slot12,
        All = Int32.MaxValue,
        Any = Int32.MaxValue
    }

    public enum JoystickSlot
    {
        Slot1 = 0,
        Slot2,
        Slot3,
        Slot4,
        All = Int32.MaxValue,
        Any = Int32.MaxValue
    }

    public enum Inputs
    {
        Key_Space,
        Key_Enter,
        Key_Tab,
        Key_Backquote,
        Key_Quote,
        Key_Semicolon,
        Key_Comma,
        Key_Period,
        Key_Slash,
        Key_Backslash,
        Key_LeftBracket,
        Key_RightBracket,
        Key_Minus,
        Key_Equals,
        Key_A,
        Key_B,
        Key_C,
        Key_D,
        Key_E,
        Key_F,
        Key_G,
        Key_H,
        Key_I,
        Key_J,
        Key_K,
        Key_L,
        Key_M,
        Key_N,
        Key_O,
        Key_P,
        Key_Q,
        Key_R,
        Key_S,
        Key_T,
        Key_U,
        Key_V,
        Key_W,
        Key_X,
        Key_Y,
        Key_Z,
        Key_Digit1,
        Key_Digit2,
        Key_Digit3,
        Key_Digit4,
        Key_Digit5,
        Key_Digit6,
        Key_Digit7,
        Key_Digit8,
        Key_Digit9,
        Key_Digit0,
        Key_LeftShift,
        Key_RightShift,
        Key_LeftAlt,
        Key_RightAlt,
        Key_AltGr = Key_RightAlt,
        Key_LeftCtrl,
        Key_RightCtrl,
        Key_LeftMeta,
        Key_RightMeta,
        Key_LeftWindows = Key_LeftMeta,
        Key_RightWindows = Key_RightMeta,
        Key_LeftApple = Key_LeftMeta,
        Key_RightApple = Key_RightMeta,
        Key_LeftCommand = Key_LeftMeta,
        Key_RightCommand = Key_RightMeta,
        Key_ContextMenu,
        Key_Escape,
        Key_LeftArrow,
        Key_RightArrow,
        Key_UpArrow,
        Key_DownArrow,
        Key_Backspace,
        Key_PageDown,
        Key_PageUp,
        Key_Home,
        Key_End,
        Key_Insert,
        Key_Delete,
        Key_CapsLock,
        Key_NumLock,
        Key_PrintScreen,
        Key_ScrollLock,
        Key_Pause,
        Key_NumpadEnter,
        Key_NumpadDivide,
        Key_NumpadMultiply,
        Key_NumpadPlus,
        Key_NumpadMinus,
        Key_NumpadPeriod,
        Key_NumpadEquals,
        Key_Numpad0,
        Key_Numpad1,
        Key_Numpad2,
        Key_Numpad3,
        Key_Numpad4,
        Key_Numpad5,
        Key_Numpad6,
        Key_Numpad7,
        Key_Numpad8,
        Key_Numpad9,
        Key_F1,
        Key_F2,
        Key_F3,
        Key_F4,
        Key_F5,
        Key_F6,
        Key_F7,
        Key_F8,
        Key_F9,
        Key_F10,
        Key_F11,
        Key_F12,
        Key_OEM1,
        Key_OEM2,
        Key_OEM3,
        Key_OEM4,
        Key_OEM5,

        Mouse_Left,
        Mouse_Right,
        Mouse_Middle,
        Mouse_Forward,
        Mouse_Back,

        Gamepad_DpadUp,
        Gamepad_DpadDown,
        Gamepad_DpadLeft,
        Gamepad_DpadRight,
        Gamepad_North,
        Gamepad_East,
        Gamepad_South,
        Gamepad_West,

        Gamepad_LeftStickButton,
        Gamepad_RightStickButton, 
        Gamepad_LeftStickUp,
        Gamepad_LeftStickDown,
        Gamepad_LeftStickLeft,
        Gamepad_LeftStickRight,
        Gamepad_RightStickUp,
        Gamepad_RightStickDown,
        Gamepad_RightStickLeft,
        Gamepad_RightStickRight,
        Gamepad_LeftShoulder,
        Gamepad_RightShoulder,
        Gamepad_LeftTrigger,
        Gamepad_RightTrigger,
        Gamepad_Start,
        Gamepad_Select,
        Gamepad_X = Gamepad_West,
        Gamepad_Y = Gamepad_North,
        Gamepad_A = Gamepad_South,
        Gamepad_B = Gamepad_East,
        Gamepad_Cross = Gamepad_South,
        Gamepad_Square = Gamepad_West,
        Gamepad_Triangle = Gamepad_North,
        Gamepad_Circle = Gamepad_East,

        Joystick_Trigger,
        Joystick_Button2,
        Joystick_Button3,
        Joystick_Button4,
        Joystick_Button5,
        Joystick_Button6,
        Joystick_Button7,
        Joystick_Button8,
    }

  }
}

API Usage

Perform some gameplay action when both the left keyboard Control key and left mouse button are pressed.

  if (Input.IsPressed(Inputs.Key_LeftCtrl) && Input.IsPressed(Inputs.Mouse_Left))
  {
    DoTheThing();
  }

Start a timer when a control is pressed and perform some action on release.

  public class ChargedFire : MonoBehaviour
  {
    private float m_StartTime = 0;

    public void Update()
    {
      if (Input.WasPressedThisFrame(Inputs.Mouse_Left))
      {
        m_StartTime = Time.time;
      }

      if (Input.WasReleasedThisFrame(Inputs.Mouse_Left))
      {
        Fire(Time.time - m_StartTime);
        m_StartTime = 0;
      }
    }

    private IEnumerator Fire(float chargeTime)
    {
      ...
      yield return null;
    }
  }

Use the left and right triggers of any gamepad as a one-dimensional axis.

  public class SteerWithGamepadTriggers : MonoBehaviour
  {
    [SerializeField] private float m_MaxDegreesPerSecond;

    public void Update()
    {
      // GetAxis returns a value in the range [-1, 1]
      float steering = Input.GetAxis(Inputs.Gamepad_LeftTrigger, Inputs.Gamepad_RightTrigger);
      transform.Rotate(Vector2.up, steering * m_MaxDegreesPerSecond * Time.deltaTime);
    }
  }

Use the WASD keys to move in a plane at the same speed in any direction.

  public class Move2DWithWASD : MonoBehaviour
  {
    [SerializeField] private float m_MaxVelocity;

    public void Update()
    {
      Vector2 directionVector = Input.GetAxis(Inputs.Key_A, Inputs.Key_D, Inputs.Key_W, Inputs.Key_S);
      transform.Translate(directionVector * m_MaxVelocity * Time.deltaTime);
    }
  }

Use the right analogue stick on any gamepad to move a game object in a plane.

  public class Move2DWithGamepadRightStick : MonoBehaviour
  {
    [SerializeField] private float m_MaxVelocity;

    public void Update()
    {
      Vector2 directionVector = Input.GetAxis(GamepadAxis.RightStick);
      transform.Translate(directionVector * m_MaxVelocity * Time.deltaTime);
    }
  }

If any joysticks are connected, get the axis value of the first one and use it to move a game object.

  public class Move2DWithJoystick : MonoBehaviour
  {
    [SerializeField] private float m_MaxVelocity;

    public void Update()
    {
      if (Input.joysticks.Count > 0)
      {
        Vector2 directionVector = Input.GetAxis(0);
        transform.Translate(directionVector * m_MaxVelocity * Time.deltaTime);
      }
    }
  }

Spawn prefabs continuously while the south button on any gamepad is pressed.

  public class FireProjectiles : MonoBehaviour
  {
    [SerializeField] private float m_FireInterval;
    [SerializeField] private GameObject m_Projectile;
    private float m_LastFireTime;

    public void Update()
    {
      if (Input.IsGamepadButtonPressed(GamepadButton.South) && Time.time - m_LastFireTime > m_FireInterval)
      {
        Instantiate(m_Projectile);
        m_LastFireTime = Time.time;
      }
    }
  }

Set some state to true in the frame the gamepad Square button is pressed on any PlayStation gamepad, and false when it is released.

  public class Jump : MonoBehaviour
  {
    private bool m_IsJumping;

    public void Update()
    {
      if (Input.IsGamepadButtonDown(GamepadButton.Square))
      {
        m_IsJumping = true;
      }

      if (Input.IsGamepadButtonUp(GamepadButton.Square))
      {
        m_IsJumping = false;
      }
    }
  }

Set the gamepad trigger press point to 0.9 for the gamepad connected to slot 1.

  Input.SetGamepadTriggerPressPoint(0.9f, GamepadSlot.Slot1);

Set the gamepad stick deadzone to 0.3 for all gamepads.

  Input.SetGamepadStickDeadzone(0.3f);

Simple local multiplayer using gamepad slots

  public class SimpleLocalMultiplayer : MonoBehaviour
  {
    public struct Player
    {
      public GameObject gameObject;
      public GamepadSlot slot;
    }

    [SerializeField] private GameObject m_PlayerPrefab;
    private List<Player> m_Players;

    public void Start()
    {
      m_Players = new List<Player>();

      for (var i = 0; i < 4; i++)
      {
        if (Input.IsGamepadConnected((GamepadSlot)i))
          m_Players.Add(new Player
          {
            gameObject = Instantiate(m_PlayerPrefab),
            slot = (GamepadSlot)i
          });
      }
    }

    public void Update()
    {
      foreach (Player player in m_Players)
      {
        var movement = Input.GetAxis(GamepadAxis.LeftStick, player.slot);
        player.gameObject.GetComponent<PlayerController>().Move(movement);

        if (Input.IsGamepadButtonDown(GamepadButton.West, player.slot))
          player.gameObject.GetComponent<PlayerController>().Shoot();
      }
    }
  }

Notes

Risks

Namespace issues

The most obvious name for the static class wrapping all of this functionality is Input, but that name is already taken by legacy input. We can work around it by using

using Input = UnityEngine.InputSystem.HighLevelAPI.Input;

in our using declarations, but that's not great. I think we could include a code analyzer to detect when users are using the wrong type and display it as a compilation error or warning to help mitigate this, but maybe a different name is better. InputMgr?

The infernal 'Was pressed this frame' issue

WasPressedThisFrame/WasReleasedThisFame i.e. was pressed this frame functionality, isn't particularly robust at the device layer right now because controls don't have any concept of a frame. So a press and release within a frame won't cause WasPressedThisFrame calls to return true because they only look at the current state, not the state over the whole frame. Since this API funnels users towards the polling approach, this flaw might become more obvious and should be fixed.

jimon commented 2 years ago

Question, so let's say users use this API earlier in development of their game, satisfied with it, and then later in the development they need to support control rebinding, how would that look like?

jimon commented 2 years ago

Have you had thoughts about making enum Inputs a partial enum? (doesn't exist in C# but still)

Thinking if we could achieve stability of constants of that enum (e.g. adding new key should change value of Key_U) we could achieve better things down the line (native->managed api transition stability)

jimon commented 2 years ago

Have you tried going with method overloads? E.g.

    public static bool IsButtonPressed(Inputs input);
    public static bool IsButtonDown(Inputs input);
    public static bool IsButtonUp(Inputs input);

    public static bool IsButtonPressed(GamepadButton button, GamepadSlot gamepadSlot = GamepadSlot.Any);
    public static bool IsButtonDown(GamepadButton button, GamepadSlot gamepadSlot = GamepadSlot.Any);
    public static bool IsButtonUp(GamepadButton input, GamepadSlot gamepadSlot = GamepadSlot.Any);

    ... etc ...

Seems like if we do GamepadButton-esque enums for every sub-type we could provide overloads for each and rely on type safety for API UX

jimon commented 2 years ago

Why there is no public static Vector2 GetAxis(Inputs stick); and no public static Vector2 GetAxis(Inputs horizontal, Inputs vertical);? Feels like providing multiple enum values for same control could work given that copies will point to different type

That way in the enum we would have Up, Down, Left, Right, Horizonal, Vertical, 2D. Imho will provide better API UX if we can leverage autocomplete to do the right thing.

andrew-oc commented 2 years ago

Question, so let's say users use this API earlier in development of their game, satisfied with it, and then later in the development they need to support control rebinding, how would that look like?

This part of the API doesn't support control rebinding. The action part, coming next, does though. But the path would be to replace the Input.IsControlxxx method calls with the equivalent Input.[action].

Have you had thoughts about making enum Inputs a partial enum? (doesn't exist in C# but still)

Thinking if we could achieve stability of constants of that enum (e.g. adding new key should change value of Key_U) we could achieve better things down the line (native->managed api transition stability)

Can you elaborate? What would that look like if the concept of partial enum doesn't exist in C#? What would be the motivation to add another enum there? It should already cover all inputs on the standard devices that this API supports.

Have you tried going with method overloads? E.g.

    public static bool IsButtonPressed(Inputs input);
    public static bool IsButtonDown(Inputs input);
    public static bool IsButtonUp(Inputs input);

    public static bool IsButtonPressed(GamepadButton button, GamepadSlot gamepadSlot = GamepadSlot.Any);
    public static bool IsButtonDown(GamepadButton button, GamepadSlot gamepadSlot = GamepadSlot.Any);
    public static bool IsButtonUp(GamepadButton input, GamepadSlot gamepadSlot = GamepadSlot.Any);

    ... etc ...

Seems like if we do GamepadButton-esque enums for every sub-type we could provide overloads for each and rely on type safety for API UX

I did look at that, but I felt that it implied that you couldn't use IsControlPressed/Up/Down for gamepad buttons, which you can, and in a completely type safe way.

Why there is no public static Vector2 GetAxis(Inputs stick); and no public static Vector2 GetAxis(Inputs horizontal, Inputs vertical);? Feels like providing multiple enum values for same control could work given that copies will point to different type

That way in the enum we would have Up, Down, Left, Right, Horizonal, Vertical, 2D. Imho will provide better API UX if we can leverage autocomplete to do the right thing.

Because those wouldn't be entirely type safe. It would be possible to pass the wrong enum in both cases, and I feel like we should avoid runtime errors where possible.

jimon commented 2 years ago

Existence of both Inputs.Gamepad_DpadUp and GamepadButton.DpadUp at a same time is a bit of a deal breaker for me tbh. Can we work around so there is always only one definition for a control?

ekcoh commented 2 years ago

Regarding Pressed, Released, Down, Up. I think its key to keep consistency in these. E.g. Pressed is an event/transition so is Released right? Down/Up are absolute state evaluations right? - It feels IsPressed is confusing since nothing is constantly transitioning into pressed? IsDown/IsUp makes sense though. Noticed this in, e.g.

  public class FireProjectiles : MonoBehaviour
  {
    [SerializeField] private float m_FireInterval;
    [SerializeField] private GameObject m_Projectile;
    private float m_LastFireTime;

    public void Update()
    {
      if (Input.IsGamepadButtonPressed(GamepadButton.South) && Time.time - m_LastFireTime > m_FireInterval)
      {
        Instantiate(m_Projectile);
        m_LastFireTime = Time.time;
      }
    }
  }

Or is this implying everytime the button is pressed as in down, up sequence and one such event happened during a frame one projectile is spawned. If two such events happened during a frame one projectile is spawned?

lyndon-unity commented 2 years ago

What does this property represent? It was the only one that wasn't intuitive to me. public static Vector2 mousePresent { get; }

andrew-oc commented 2 years ago

@ekcoh The intent was that Pressed means this control is currently held down. Maybe IsHeld is better? Down and up are the actual state transitions. Those methods are intended to return true in the frame that the control is pressed or released, and they're named for familiarity with the legacy API.

@lyndon-unity It's meant to represent whether a mouse is connected or not, but should have had a bool return type.

lyndon-unity commented 2 years ago

@lyndon-unity It's meant to represent whether a mouse is connected or not, but should have had a bool return type.

It would be good to be consistent with the pad function naming which uses the phrase 'connected' public static bool IsGamepadConnected(GamepadSlot slot);

I momentarily wondered then if it should be moved to a function and take a parameter but I see is a convenience for the common case with a single mouse (or first mouse). Do we need both?

Do we need a IsJoystickConnected(int joystickIndex);

I'm also wondering why we have an enum for gamepad but just int for joystick. I see its because we have the 'all/any' cases. Do we need that for Joysticks too though?

Or am I confused with what joystickIndex is? Is this parameter a device index or for multiple sticks on a single device public static Vector2 GetJoystickAxis(int joystickIndex);

andrew-oc commented 2 years ago

It would be good to be consistent with the pad function naming which uses the phrase 'connected' public static bool IsGamepadConnected(GamepadSlot slot);

Initially it was called mouseConnected but it ended up as mousePresent for two reasons. One, familiarity with the legacy API which also calls it mousePresent, and two, a 'mouse' can be any device that allows a cursor to be moved, like a trackpad in a laptop or gamepad, so it feels wrong calling it "connected". What do you think?

I momentarily wondered then if it should be moved to a function and take a parameter but I see is a convenience for the common case with a single mouse (or first mouse). Do we need both?

Yeah, it's a toss up between keeping the API surface as small as possible (bearing in mind it will grow a bit over the next few proposals) and providing convenience methods for every device. It is still possible to check if a mouse is present even without this convenience method by checking the mice collection for a non-null device in any slot but my thoughts on this is that it's not a common enough use case to have it's own API method.

Do we need a IsJoystickConnected(int joystickIndex);

I'm also wondering why we have an enum for gamepad but just int for joystick. I see its because we have the 'all/any' cases. Do we need that for Joysticks too though?

Same as previous argument.

Or am I confused with what joystickIndex is? Is this parameter a device index or for multiple sticks on a single device public static Vector2 GetJoystickAxis(int joystickIndex);

Nope, you're correct. It's a slot index for a device. Maybe we could rename GamepadSlot to DeviceSlot and use that for all APIs that take a slot index. Would that make things better?

lyndon-unity commented 2 years ago

Initially it was called mouseConnected but it ended up as mousePresent for two reasons. One, familiarity with the legacy API which also calls it mousePresent, and two, a 'mouse' can be any device that allows a cursor to be moved, like a trackpad in a laptop or gamepad, so it feels wrong calling it "connected". What do you think?

Thank you I wasn't aware of those aspects. Your reasoning seems fair to me. Perhaps we simply need to add a comment on this property to make it a little clearer. E.g.

public static Vector2 mousePresent { get; } // True if a mouse, trackpad or similar is present/connected.

(I was then wondering if IsGamepadConnected should be renamed IsGamepadPresent for consistency, but I still prefer the phrase connected so I'm I've mixed feelings.)

Maybe we could rename GamepadSlot to DeviceSlot and use that for all APIs that take a slot index. Would that make things better?

I like that proposal and feel it would make it more consistent and allow for the All/Any case for Joysticks.

ekcoh commented 2 years ago

Regarding previous comment/feedback

@ekcoh The intent was that Pressed means this control is currently held down. Maybe IsHeld is better? Down and up are the actual state transitions. Those methods are intended to return true in the frame that the control is pressed or released, and they're named for familiarity with the legacy API.

This was also discussed in other review thread, IsHeld is better I think but also reflects assumption about push button which might be fine. IsHeld is definately a contender, other options include I guess IsActuated/IsActive/IsTriggered/IsOn. Maybe it would be helpful to try to define whether we want to model the physical action in the names or the logical. On a physical model (class) it makes more sense to use e.g. Held, Down, Up etc if it models a physical entity, for a logical entity is feels like it makes more sense with e.g. On, Off, High, Low, Active etc. Naming is hard, but I think it might be good if we avoid mixing physical and logical modeling/representation.

ekcoh commented 2 years ago

Just for clarification I assume this API is also subject for down-sampling to frame granularity?E.g. use-case example: Perform some gameplay action when both the left keyboard Control key and left mouse button are pressed.` would basically be true if left keyboard control key and left mouse button was pressed at some point during the same frame, not necessarily at the same time? (This is just for making sure interpretation is correct)

Maybe its wise to scope out higher-frequency events since it would drive complexity and those interested can use other APIs

ekcoh commented 2 years ago

I noticed we have IsControlPressed(Inputs input); should there be a IsControlReleased(Inputs input); for the inverse edge? Or is this reflecting if actuated?

ekcoh commented 2 years ago

Regarding previous discussion of "Connected" vs "Present" based off previous APIs I have worked with I would suggest to stick to defining something like:

Connected - Is a specific device connected to the system. This may not imply that its available since it might be connected and be possible to enumerate, but it might be disabled in software, for example its driver being disabled but enumerable via underlying protocols. This is applies to both peripherals and device interfacing via e.g. on-board USB or other hardware interfaces. An integrated device, e.g. touch screen is typically always connected but might no be present if disabled in software.

Present - A higher level concept indicating that the device is available and active. A device wouldn't be present if connected and disabled in driver/os or software level. A device being present indicates it can be used.

Such detail is likely not applicable to this high level API anyway but it might be good to use similar concepts between API layers. If we already use connected maybe we need to stick to it.

ekcoh commented 2 years ago

Based on previous comments it sounds like the intention with the enum is to be applicable to this API surface only? If not to me it looks like an oppertunity to map usages that could be applicable and potentially aliased for any control, including all HID controls long-term and potentially also be fitted into future lower layers. This might imply that its not an enum but rather integer constants under the hood of a type if type safety is intended which may be used to support partial enums as mentioned by Dmytro and/or HID usage aliasing and/or Spatial aliasing and/or user-extension within custom ranges.

Or what benefits do you see with enum compared to numeric constants (under a layer of type safety)? - I assume the key point here is provide compile-time type safety?

Is the slot concept intended to be applied for all device interface models?

ekcoh commented 2 years ago

It seems like the mouse accessors are different from the other accessors but there is still a concept of a collection of mice? I noticed the reply to Lyndon about pointer devices, but since some platforms, e.g. console, typically never have a pointer device what is the motivation for having it different from other device models?

ekcoh commented 2 years ago

What benefit do you see from having GamepadSlot an enum? It seems like an unsigned would do just fine for the "instance" use-case?

ekcoh commented 2 years ago

Review meeting:

Discussed alternatives to enum Inputs:

public bool Input.IsControlPressed(GamepadButton button); vs public struct bool IsControlPressed(Button button);

One angle would be implementation aspect with e.g. HID page / id. Another angle from user is using auto-complete. Less fluid with dot vs underscore. Regarding above the biggest downside might be overloads. Might be undesirable to have too abstract identifiers. Clarified that public static bool IsControlPressed(Inputs) is only intended for the first device.

Discussed Inputs enum and whether Inputs need to be explicitly assigned or not. There are pros and cons with both approaches.

Way forward: Lets proceed with enum version for usability evaluation and reevaluate struct approach if highlighted in feedback.

ekcoh commented 2 years ago

Review meeting:

Discussed why we have mousePresent, inherited from existing API. Should we consider calling this "pointer" to not let user assume this is a mouse. If user is developing for mobile and thinking mobile should we call this "pointer" which would be touch. Discussed pen and touch and whether to differentiate or not. This might also make it confusing with scroll delta. What is use case for mousePosition. From a less-work-for-developer point of view a pointer is likely more useful to provide an abstraction.

Use-case for it, motivate why do we need that compares like isKeyboardPresent.

Way forward: Drop mouse present. Pointer could be an alternative.

Discussed whether e.g. public static Vector2 GetScreenPosition(PositionSource source)

This could provide consistency with e.g. GetAxis, IsControlDown etc.

Discussed if e.g. XR use-cases might be relevant for this API or more advanced use-cases. Will we be able to build on-top of this.

There are some inconsistency in the API design, this is intentional, but consistency might be beneficial for extendability. E.g. handle system keyboard similar to a "system gamepad". Discussed device specific and device unspecific.

Clarified that IReadonlyList and friends are mapping 1:1 with existing InputSystem collections of current devices.

Suggested to join joystick and gamepad slots. Or generalize into DeviceSlots. One option would be to have ints instead to avoid casting. Clarified that slot concept is intended to be assigned and then stick and leaves a reassignable hole when disconnected.

MInor: IsGamepadButtonUp first argument should be "button".

Way forward: Consider enum or int but there is benefit for Any and All when using enum, max value could be a constant. APIs could be generalized for instance/slot specific access also for other device types if needed.

Some concerns regarding how to handle e.g. Android specific key code. Other example is e.g. pointing devices. Extendability might be problematic with fixed enums.

jimon commented 2 years ago

After some poking at this, I would like to propose a slightly tweaked version while keeping the spirit of the original proposal:

Overall this API provides potentially somewhat more future-proof API, as now users can implement their game by using only keyboard + mouse, and then add rebinding in the end of game development and by doing this allow for usage of any other device type (even future ones).

namespace UnityEngine.InputSystem.HighLevelAPI
{
    // -------------------------------------------------------------------- Type safe public enums
    // Basic idea here is to provide enums such so return type of control is figured out in compile time.
    // For example when using GamepadTwoWayAxis.LeftStickHorizontal,
    // user will know that return type is float in [-1, 1] range, and not in [0, 1] range.

    public enum KeyboardButton
    {
        Space = InputControl.KeyboardButtonSpace,
        Digit0 = InputControl.KeyboardButtonDigit0,
        Digit1 = InputControl.KeyboardButtonDigit1,
        Digit2 = InputControl.KeyboardButtonDigit2,
        Digit3 = InputControl.KeyboardButtonDigit3
    }

    public enum MouseButton
    {
        Left = InputControl.MouseButtonLeft,
        Right = InputControl.MouseButtonRight,
        Middle = InputControl.MouseButtonMiddle,
        Forward = InputControl.MouseButtonForward,
        Back = InputControl.MouseButtonBack,
        ScrollLeft = InputControl.MouseButtonScrollLeft,
        ScrollUp = InputControl.MouseButtonScrollUp,
        ScrollRight = InputControl.MouseButtonScrollRight,
        ScrollDown = InputControl.MouseButtonScrollDown,
    }

    public enum MouseTwoWayAxis
    {
        ScrollHorizontalTimeNormalized = InputControl.MouseTwoWayAxisScrollHorizontalTimeNormalized,
        ScrollVerticalTimeNormalized = InputControl.MouseTwoWayAxisScrollVerticalTimeNormalized,
    }

    // buttons
    public enum GamepadButton
    {
        LeftTrigger = InputControl.GamepadButtonLeftTrigger,
        RightTrigger = InputControl.GamepadButtonRightTrigger,
        DpadLeft = InputControl.GamepadButtonDpadLeft,
        DpadUp = InputControl.GamepadButtonDpadUp,
        DpadRight = InputControl.GamepadButtonDpadRight,
        DpadDown = InputControl.GamepadButtonDpadDown,
        LeftStickLeft = InputControl.GamepadButtonLeftStickLeft,
        LeftStickUp = InputControl.GamepadButtonLeftStickUp,
        LeftStickRight = InputControl.GamepadButtonLeftStickRight,
        LeftStickDown = InputControl.GamepadButtonLeftStickDown,
        RightStickLeft = InputControl.GamepadButtonRightStickLeft,
        RightStickUp = InputControl.GamepadButtonRightStickUp,
        RightStickRight = InputControl.GamepadButtonRightStickRight,
        RightStickDown = InputControl.GamepadButtonRightStickDown,
        West = InputControl.GamepadButtonWest,
        North = InputControl.GamepadButtonNorth,
        East = InputControl.GamepadButtonEast,
        South = InputControl.GamepadButtonSouth,
        LeftStickPress = InputControl.GamepadButtonLeftStickPress,
        RightStickPress = InputControl.GamepadButtonRightStickPress,
        LeftShoulder = InputControl.GamepadButtonLeftShoulder,
        RightShoulder = InputControl.GamepadButtonRightShoulder,
        Start = InputControl.GamepadButtonStart,
        Select = InputControl.GamepadButtonSelect
    }

    // 1D axis with values in [-1, 1] range
    public enum GamepadTwoWayAxis
    {
        DpadHorizontal = InputControl.GamepadTwoWayAxisDpadHorizontal,
        DpadVertical = InputControl.GamepadTwoWayAxisDpadVertical,
        LeftStickHorizontal = InputControl.GamepadTwoWayAxisLeftStickHorizontal,
        LeftStickVertical = InputControl.GamepadTwoWayAxisLeftStickVertical,
        RightStickHorizontal = InputControl.GamepadTwoWayAxisRightStickHorizontal,
        RightStickVertical = InputControl.GamepadTwoWayAxisRightStickVertical
    }

    // 2D normalized vector
    public enum GamepadStick
    {
        Dpad = InputControl.GamepadStickDpad,
        Left = InputControl.GamepadStickLeft,
        Right = InputControl.GamepadStickRight
    }

    // -------------------------------------------------------------------- Storing controls in generic data structures

    // Not sure if we need this one,
    // but could be beneficial for cases where users need to
    // show icon or text for a latest used control
    // or for rebinding to check if engaged control is of the same type
    public enum InputControlType
    {
        Button, // every button could be also read as one way axis
        OneWayAxis, // do we need separate one?
        TwoWayAxis,
        Stick,
        CursorPosition,
        ScrollDelta
    }

    // It is useful to allow the users to store controls in some generic data structure like a list,
    // for that we need to unify all enums back to generic InputControl, but keep the type safety.
    // This container struct tries to achieve exactly that.
    public struct InputControlReference
    {
        internal InputControl control;

        // TODO do we want to add some type checking here?
        // is it even possible?

        public bool IsValid() => control == InputControl.Invalid;

        public static implicit operator InputControlReference(KeyboardButton btn) => new InputControlReference() {control = (InputControl)btn};
        public static implicit operator InputControlReference(MouseButton btn);
        public static implicit operator InputControlReference(MouseTwoWayAxis btn);
        public static implicit operator InputControlReference(GamepadButton btn);
        public static implicit operator InputControlReference(GamepadTwoWayAxis btn);
        public static implicit operator InputControlReference(GamepadStick btn);

        public InputControlType GetControlType() => Input.InputControlToType(control);
    }

    // -------------------------------------------------------------------- Device slots

    // Device slots are similar concept to player slots in split screen games.
    // But a specific device slot can contain multiple devices, like a keyboard and a mouse.
    // Device slots could be used for player assignment management.
    // By default all new devices go to Unassigned slot.
    public enum DeviceSlot
    {
        Any = -1,
        Unassigned = 0,
        Slot1,
        Slot2,
        Slot3,
        Slot4,
        Slot5,
        Slot6,
        Slot7,
        Slot8,
        Slot9,
        Slot10,
        Slot11,
        Slot12,
        Slot13,
        Slot14,
        Slot15,
        Slot16,
        // any positive int number could be used?
    }

    public static class Input
    {
        // By default all devices go to unassigned slot,
        // this methods should be used to manage slot assignment.
        // DeviceSlot.Any is not a valid argument.
        public static void AssignDeviceToSlot(InputDevice device, DeviceSlot slot);
        public static void RemoveDeviceFromSlot(InputDevice device, DeviceSlot slot);

        // If provided with DeviceSlot.Any returns all devices.
        public static InputDevice[] GetDevices(DeviceSlot slot);

        // Returns latest used device in this frame, or null otherwise.
        // Useful for player join-in, or showing icon of latest used device.
        // If provided with DeviceSlot.Any returns latest used device in general.
        public static InputDevice GetLatestUsedDevice(DeviceSlot slot = DeviceSlot.Any);

        // Returns latest used control in this frame, or invalid reference otherwise. 
        // Useful for rebinding implementation.
        public static InputControlReference GetLatestUsedControl(InputDevice device);

        public static bool IsConnected(DeviceSlot slot) => GetDevices(slot).Length != 0;

        // --------------------------------------------------------------------

        // Sets trigger set point for analogue buttons,
        // like gamepad triggers, joystick triggers, gamepad stick buttons up/down/left/right.
        // Default is 0.5f 
        public static void SetTriggerPressPoint(float pressPoint, DeviceSlot slot = DeviceSlot.Any);
        public static void SetStickDeadzone(float deadzone, DeviceSlot slot = DeviceSlot.Any);

        // Internal helper to convert from global enum value to control type
        internal static InputControlType InputControlToType(InputControl control);

        // --------------------------------------------------------------------
        // Less type-safe API, only to be used when required to store controls of different type
        // together in some data structure like list or array.
        // Notice we can't override GetAxis here and have to revert to name methods fully,
        // due to only difference being the return type.
        public static class ByReference
        {
            public static bool IsButtonPressed(InputControlReference control, DeviceSlot slot);
            public static bool WasButtonDown(InputControlReference control, DeviceSlot slot);
            public static bool WasButtonUp(InputControlReference control, DeviceSlot slot);

            // Any button is also a one way axis.
            // Returns value in [0, 1] range
            // TODO do we have any control which is one way axis but not a button?
            public static float GetOneWayAxis(InputControlReference control, DeviceSlot slot);

            // Returns value in [-1, 1] range
            public static float GetTwoWayAxis(InputControlReference control, DeviceSlot slot);

            public static Vector2 GetStick(InputControlReference control, DeviceSlot slot);
        }

        // -------------------------------------------------------------------- Keyboard support
        public static bool IsPressed(KeyboardButton button, DeviceSlot slot = DeviceSlot.Any);
        public static bool WasDown(KeyboardButton button, DeviceSlot slot = DeviceSlot.Any);
        public static bool WasUp(KeyboardButton button, DeviceSlot slot = DeviceSlot.Any);

        // Returns value in [0, 1] range
        public static float GetAxis(KeyboardButton button, DeviceSlot slot = DeviceSlot.Any);

        // Helper for WASD like controls.
        public static Vector2 GetAxis(KeyboardButton left, KeyboardButton up, KeyboardButton right, KeyboardButton down, DeviceSlot slot = DeviceSlot.Any);

        // -------------------------------------------------------------------- Mouse support
        public static bool IsPressed(MouseButton button, DeviceSlot slot = DeviceSlot.Any);
        public static bool WasDown(MouseButton button, DeviceSlot slot = DeviceSlot.Any);
        public static bool WasUp(MouseButton button, DeviceSlot slot = DeviceSlot.Any);

        // Returns value in [0, 1] range
        public static float GetAxis(MouseButton button, DeviceSlot slot = DeviceSlot.Any);

        // Returns value in [-1, 1] range
        public static float GetAxis(MouseTwoWayAxis button, DeviceSlot slot = DeviceSlot.Any);

        // Returns current mouse cursor position
        public static Vector2 GetMousePosition(DeviceSlot slot = DeviceSlot.Any);

        // Returns current frame accumulated mouse scroll value
        public static Vector2 GetMouseScroll(DeviceSlot slot = DeviceSlot.Any);

        // -------------------------------------------------------------------- Gamepad support
        public static bool IsPressed(GamepadButton button, DeviceSlot slot = DeviceSlot.Any);
        public static bool WasDown(GamepadButton button, DeviceSlot slot = DeviceSlot.Any);
        public static bool WasUp(GamepadButton button, DeviceSlot slot = DeviceSlot.Any);

        // Returns value in [0, 1] range
        public static float GetAxis(GamepadButton button, DeviceSlot slot = DeviceSlot.Any);

        // Returns value in [-1, 1] range
        public static float GetAxis(GamepadTwoWayAxis axis, DeviceSlot slot = DeviceSlot.Any);

        public static Vector2 GetAxis(GamepadStick stick, DeviceSlot slot = DeviceSlot.Any);

        // --------------------------------------------------------------------
        // Replaces which controls drive a provided control.
        // By default every control drives itself,
        // e.g. pressing Space key on keyboard will report as space key.
        // But it's possible to override which keys will be reported as space key,
        // e.g. pressing Enter key could be driving Space key.
        // But pressing Space key then would do nothing.  
        public static void SetRebinding(
            InputControlReference driveControl,
            IEnumerable<InputControlReference> withAnyOfFollowingControls,
            DeviceSlot inSlot);
    }

    // --------------------------------------------------------------------
    // It is in principle beneficial to have a list of all controls,
    // as this is where stability of enum API values could be enforced.
    // Also we could separate controls by type based on this enum,
    // e.g. have bool IsButton(InputControl control) and similar.
    // It's also useful for rebinding activities.
    // Plus we can figure out if we need to split a control into multiple of different types,
    // e.g. mouse scroll is vector2 delta control, and also 2 single axis controls, plus 4 buttons.
    // But for the sake of type safety, let's not expose it directly to the user,
    // and provide necessary overrides instead.
    internal enum InputControl
    {
        Invalid = 0x0,

        KeyboardButtonSpace = 0x0100000,
        KeyboardButtonDigit0,
        KeyboardButtonDigit1,
        KeyboardButtonDigit2,
        KeyboardButtonDigit3,
        // ... 100 more

        MouseButtonLeft = 0x0200000,
        MouseButtonRight,
        MouseButtonMiddle,
        MouseButtonForward,
        MouseButtonBack,
        MouseButtonScrollLeft,
        MouseButtonScrollUp,
        MouseButtonScrollRight,
        MouseButtonScrollDown,

        // mouse scroll divided by delta time, and normalized to [-1, 1] range
        // useful for using scroll like gamepad stick
        MouseTwoWayAxisScrollHorizontalTimeNormalized = 0x0210000,
        MouseTwoWayAxisScrollVerticalTimeNormalized,

        // Cursor position is an absolute value control, unlike vector2 control which is normalized
        MouseCursorPosition = 0x0220000,

        // Scroll delta is a relative value control, unlike vector2 control which is normalized
        MouseScrollDelta = 0x0230000,

        GamepadButtonLeftTrigger = 0x0300000,
        GamepadButtonRightTrigger,
        GamepadButtonDpadLeft,
        GamepadButtonDpadUp,
        GamepadButtonDpadRight,
        GamepadButtonDpadDown,
        GamepadButtonLeftStickLeft,
        GamepadButtonLeftStickUp,
        GamepadButtonLeftStickRight,
        GamepadButtonLeftStickDown,
        GamepadButtonRightStickLeft,
        GamepadButtonRightStickUp,
        GamepadButtonRightStickRight,
        GamepadButtonRightStickDown,
        GamepadButtonWest,
        GamepadButtonNorth,
        GamepadButtonEast,
        GamepadButtonSouth,
        GamepadButtonLeftStickPress,
        GamepadButtonRightStickPress,
        GamepadButtonLeftShoulder,
        GamepadButtonRightShoulder,
        GamepadButtonStart,
        GamepadButtonSelect,

        GamepadTwoWayAxisDpadHorizontal = 0x0310000,
        GamepadTwoWayAxisDpadVertical,
        GamepadTwoWayAxisLeftStickHorizontal,
        GamepadTwoWayAxisLeftStickVertical,
        GamepadTwoWayAxisRightStickHorizontal,
        GamepadTwoWayAxisRightStickVertical,

        // stick is normalized vector2
        GamepadStickDpad = 0x0320000,
        GamepadStickLeft,
        GamepadStickRight
    }
}
ekcoh commented 2 years ago

Team discussion notes on last iteration of proposal (comment above):

Key concepts:

Discussion points

Way forward

jamesmcgill commented 1 year ago

This work has been discontinued.