Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rudimentary Gamepad Support #1723

Merged
merged 33 commits into from Apr 28, 2020
Merged

Rudimentary Gamepad Support #1723

merged 33 commits into from Apr 28, 2020

Conversation

jefetienne
Copy link
Contributor

@jefetienne jefetienne commented Feb 25, 2020

(This is a big pull request, I'm sorry! But it had to be in order to get the functionality working. Even though it shows 20+ files changed, only two of them contain actual business logic: InputManager.cs and DaggerfallControlsWindow.cs. The rest are modified to replace Unity's "Input" static class methods with InputManager's new wrapper methods (e.g. Input.mousePosition to InputManager.Instance.MousePosition, or Input.GetKeyDown(...) to InputManager.Instance.GetKeyDown(...)).

I have spent a couple weeks working with support for controller input and it became a lot more complicated than I thought it was going to be:

  • In Unity, the Left/Right Triggers and D-Pad (on Windows/Linux) are treated as axes, rather than as buttons like ABXY, L/R2 or L/R3. This is because they return a range of values from -1.0f to 1.0f, rather than a boolean of 'pressed' and 'not pressed'. Because they are considered axes and not buttons, I had to implement functionality to treat them like buttons by creating 'fake' KeyCode enums starting at 5000, with indexable string names like "JoystickAxis7Button0", "JoystickAxis7Button1", etc. Because those numbers don't actually exist in the KeyCode enum, I created a method for parsing them into their matching string names and vice versa.

  • I also had implement my own versions of GetKey, GetKeyDown, and GetKeyUp for these "axis buttons" since there are no built-in Unity event handlers to detect a change in axis input. It was tricky at first, but essentially it uses three different dictionaries - one that holds an axis button's previous value in the last frame, one that will have it currently flagged as "up" for the current frame, and one that will have it currently flagged as "down" for the current frame. My GetKeyDown and GetKeyUp functions look at these dictionaries to see if a key at their corresponding dictionaries is flagged as true.

  • I even found out that controllers and OS drivers can have different axis mappings. For example, the left and right triggers for an Xbox controller are marked as axes 9 and 10 on Windows, but on macOS, they are axes 5 and 6. (https://answers.unity.com/storage/temp/134371-xbox-one-controller-unity-windows-macos.jpg)
    Because there can be a lot of nuance and diversity of using different controllers and platforms, I've added twelve more axes in Unity's InputManager ("Edit > Project Settings > Input", not InputManager.cs) to make a total of sixteen recognized axes ("Axis1", "Axis2", ... "Axis16").

  • Like InputManager.Actions, I also created an enum called InputManager.AxisActions:

public enum AxisActions
{
      MovementHorizontal,
      MovementVertical,
      CameraHorizontal,
      CameraVertical,
}
    • They are also serialized in Keybinds.txt and each correspond to an axis number. Eventually, the plan is to create a Joystick Controls window that will allow the user to bind these AxisActions to any of the sixteen axes, so a person with a PS4 or Logitech controller, or is on Linux/macOS, can still play.
  • I'm also incredibly delighted to say that Unity provides no built-in method to control the in-game cursor with a joystick. So to navigate through the UI with a controller, DFU's InputManager will recognize when the user moves their joystick to switch to a "controller mode" by hiding the real mouse cursor and spawning a "fake cursor" in its place via GUI.DrawTexture(...). If the user touches the mouse again, then the fake cursor disappears and the real cursor reappears (unfortunately the real cursor reappears where it last moved instead of where the joystick cursor was because Unity can't physically reposition the mouse, except when locking it to the center).
    InputManager.MousePosition is determined by what mode InputManager is in, and returns the position of either the real cursor or the joystick cursor. For now, I also hardcoded the left- and right-click buttons to KeyCode.JoystickButton0 (A button for Xbox on Windows/Linux) and KeyCode.JoystickButton1 (B button for Xbox on Windows/Linux), respectively. I also plan for these buttons to be rebindable when we create a future Joystick Controls window.

  • To use my faux KeyCode enums but still have readability in KeyBinds.txt, I changed the serialized dictionary's key type to be Strings instead of KeyCodes, since the JSON serialization library can't parse a fake KeyCode number into an equivalent ToString(). I've tested to make sure it won't cause errors when transitioning from the old .txt format to the new one.

  • I also created multiplier properties for joystick camera and mouse sensitivity. Right now, they can't be modified (set at 1.0f), but in preparation for the secondary keybind window, we can use a slider to adjust the values.

  • Accessing the pause menu must still be done via the Escape key. I've grep'd through the source code and found the Escape key hardcoded in many areas, and I figured this should be fixed in a future pull request for when we start creating the secondary keybind window.

  • It's recommended that you have "Click to attack" enabled when using a controller. Maybe in a future pull request, you can have "Click to attack" still disabled but the engine will switch to click-to-attack if the attack button is binded to a joystick control.

Right now, I've only tested this game with a wired Xbox 360 controller and a wired PDP Xbox One controller for Windows 8.1 and Linux Mint 19.3 Cinnamon (but I'd expect it to work for Windows 10). Unfortunately, macOS and possibly other Linux distros may have issues with the default Movement/Camera axes set because of the different axis mappings.

This pull request is basically the start of rudimentary controller support after rolling out the secondary keybind window. I'll also create a controller setting window, such as axis mappings, inverting the Y axis, setting the controller left/right click buttons, joystick sensitivity, deadzone, etc.

…gers. keybindings now map as integers instead of keycode tostrings. replaced some Input.GetKey..() functions with InputManager functions, since these custom 'keycodes' aren't officially in the enum list
… added additional axes in Unity's Input manager up to 'Axis16' for future rebinding support
…equently calling InputManager.GetKeyDown to an axis button. For some reason, when rebinding on the Controls window, I had no issues with a 360 controller, but my PDP Xbox One controller could not detect 'GetKeyDown' with axis buttons. As a result, I created 'InputManager.LastKeyDown' which returns the last key that was pressed down.
@jefetienne jefetienne changed the title Xbox Rudimentary Gamepad Support Feb 25, 2020
@rossipaolo rossipaolo added the enhancement A new feature or an improvement to an existing one. label Feb 25, 2020
@rossipaolo
Copy link
Collaborator

To use my faux KeyCode enums but still have readability in KeyBinds.txt, I changed the serialized dictionary's key type to be Strings instead of KeyCodes, since the JSON serialization library can't parse a fake KeyCode number into an equivalent ToString(). I've tested to make sure it won't cause errors when transitioning from the old .txt format to the new one.

The serialization tool we are using supports custom serialization; may this be useful for your needs?

@petchema
Copy link
Collaborator

(as a general remark, it would be good to harden KeyBind.txt parsing; For example the "Print Screen" feature PR added an entry in this file, but the next time I run a DFU version that doesn't have that patch, all bindings are reset to None)

@jefetienne
Copy link
Contributor Author

To use my faux KeyCode enums but still have readability in KeyBinds.txt, I changed the serialized dictionary's key type to be Strings instead of KeyCodes, since the JSON serialization library can't parse a fake KeyCode number into an equivalent ToString(). I've tested to make sure it won't cause errors when transitioning from the old .txt format to the new one.

The serialization tool we are using supports custom serialization; may this be useful for your needs?

I guess I could implement custom serialization, the only thing is that I would have to create another class and override the superclass methods, which adds more code and would require further debugging. Since I only use ParseKeyCodeString in only two places, I think the simplicity helps keep things neat and does work fine, but if you want the code to be more formal, I can add it in.

@jefetienne
Copy link
Contributor Author

(as a general remark, it would be good to harden KeyBind.txt parsing; For example the "Print Screen" feature PR added an entry in this file, but the next time I run a DFU version that doesn't have that patch, all bindings are reset to None)

Yeah, we could work on that too. Fortunately the screenshot PR should be accepted before this does, so at least that specific issue won't be a problem in the future.

… any platform can be used now, except for rebinding Movement/Camera horizontals and verticals
@jefetienne
Copy link
Contributor Author

jefetienne commented Feb 26, 2020

I just made a commit that reworked the axis bindings a bit more so that way they get generated dynamically, and now Linux and Mac users should be able to rebind the buttons. Only the AxisActions of Movement and Camera cannot be rebinded yet. However, Axis 1 and 2 usually correspond to the same joystick on the same OSes anyway.

@Interkarma
Copy link
Owner

(as a general remark, it would be good to harden KeyBind.txt parsing; For example the "Print Screen" feature PR added an entry in this file, but the next time I run a DFU version that doesn't have that patch, all bindings are reset to None)

I've hardened keybind data now using fix originally in PR #1322 - at least for future versions going backwards to 0.10.21 or later. We can't do much for older versions other than note this in release notes. We cycle through versions very quickly, so this situation will expire out naturally within a month or so. And it only affects users rolling back, not users moving forwards.

@jefetienne
Copy link
Contributor Author

jefetienne commented Mar 13, 2020

One thing that I should mention is that the secondary keybind pull request I made earlier #1748 should definitely be merged before this one.

@jefetienne
Copy link
Contributor Author

I tried this branch with my new Linux gaming machine (Linux Mint 19.3 Cinnamon) and it actually works perfectly with the default Movement/Camera axes for Xbox! If anyone has a Mac and wants to try this branch out, feel free.

@Interkarma
Copy link
Owner

One thing that I should mention is that the secondary keybind pull request I made earlier #1748 should definitely be merged before this one.

Thank you, I will ensure this one gets merged first. I haven't been able to review any code in the last week due to work, but I'm paying attention to comments. :)

@jefetienne
Copy link
Contributor Author

jefetienne commented Apr 20, 2020

Now I understand the importance of git pull --rebase.

@Interkarma, I think this is good to merge. To recap, I've tested it with an Xbox 360 Wired Controller and an Xbox One PDP Wired Controller on Windows 8.1 and Linux Mint 19.3 Tricia Cinnamon (should work for Windows 10 too). It should still be good to use with other controllers like PS3, PS4, Logitech, etc. I have not tested it with macOS, however, but I'm sure it won't be a problem in the near future when I work on the joystick controls window.

@Interkarma
Copy link
Owner

Very awesome! Excited to have this one in next builds. Will test as much as possible before merging and let you know how I go. :)

@jefetienne
Copy link
Contributor Author

Awesome, please do! I worked so much on this that it was hard for me to find a line to draw to know when I felt "finished" with it.

I left some things, like message boxes with typed input, force the player to use a keyboard to verify.

@Interkarma
Copy link
Owner

IMO you've definitely gone above and beyond on this one. Something I really appreciate is that you broke it down into many smaller pieces and we were able to roll them out into live builds one chunk at a time. Not only does that make it easier for me to digest and review, it helps us test for problems in stages rather than one big change. Very happy with how this has turned out, and you're right to be very proud of it.

@jefetienne
Copy link
Contributor Author

Thank you so much, that really means a lot to me. I'm very happy to help :)

@Interkarma
Copy link
Owner

Very happy with all this so far. I'll merge into master so we can all get some live testing in before next round of builds. :)

@Interkarma Interkarma merged commit 0a263b7 into Interkarma:master Apr 28, 2020
@jefetienne
Copy link
Contributor Author

jefetienne commented Apr 28, 2020

Great, thank you :)

I'll start working on a joystick controls window with keybinds for left/right click, sliders for sensitivities and thresholds, and I'm even considering implementing a dropdown menu to select presets for xbox and playstation controls, but I believe those don't exist yet, so I'd need to make a PR for a dropdown menu first.

@jefetienne jefetienne deleted the xbox branch July 5, 2020 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or an improvement to an existing one.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants