-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Added Xbox S Bluetooth controller support. #936
Added Xbox S Bluetooth controller support. #936
Conversation
@@ -0,0 +1,17 @@ | |||
using UnityEngine.EventSystems; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
License header missing
@@ -10,6 +11,7 @@ namespace HoloToolkit.Unity.InputModule | |||
/// TODO This should be converted to an input source. | |||
/// </summary> | |||
/// <remarks>Make sure to enable the HumanInterfaceDevice capability before using.</remarks> | |||
[Obsolete] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mention obsolete in favor of what. Could remove the TODO then aswell if you mention the input source here.
{ | ||
if (string.IsNullOrEmpty(joystickNames[i]) || gamePadInputDatas.ContainsKey((uint)i)) { continue; } | ||
|
||
if (joystickNames[i].Contains("Xbox Bluetooth Gamepad") || joystickNames[i].Contains("Xbox Wireless Controller") || joystickNames[i].Contains("Xbox Controller")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there gonna be more of these names? Maybe have a fixed list of these so this line doesn't get any longer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly a fixed list from Unity side.
The joystick list gets changed every time a new one is recognized by either the player or the editor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean a fixed list for the contains names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do.
…ething you shouldn't have)
Added rotation example.
When testing this, all the buttons work (Bumpers, ABXY, Select, Menu, Stick Click. But none of the Axis ones seem to be working (Triggers, sticks horz & vert, DPAD, etc) I did grab the input settings from the repro, so that's not the issue. |
Screen shot the input settings? |
Also what kind of xbox controller are you using? |
@jessemcculloch did you test this in the toolkit project or in your own? |
Also, this was in the player, not on device. Not sure if that matters |
@jessemcculloch I think I might have an idea about what's going on. Initially I had gotten the input from any joystick, but found when I made a build for WSA PC that the axis inputs were stuck at either 1 or -1 depending on if the axis was inverted. Come to find out the scroll and mouse was also influencing the left and shared trigger axis'. So I changed it to only get joystick axis inputs from joystick 1. Try changing the input to get input from any joystick and see if that works for you. |
That was exactly it @StephenHodgson |
@jessemcculloch Yeah I'm unsure how to go about getting joystick input from all the axis without getting mouse and scroll axis inputs that influence our controller. Unfortunately the joystick axis for the left stick and the shared triggers need to be x, y, and scroll respectively to work. The only other thing we could do is write our own input module, but I'm trying to avoid that. |
/// </summary> | ||
/// <remarks>Make sure to enable the HumanInterfaceDevice capability before using.</remarks> | ||
[Obsolete] | ||
public class GameControllerManipulator : MonoBehaviour |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a message letting people know what they should be using instead of this Obsolete class, or is this just used internally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was never really working anyway, but I think it's good practice to add the message.
After switching back to getting all joystick axis inputs, this no longer seemed to happen.
[Edit] |
… condition. Reverted input mappings to use motion from all joysticks.
@keveleigh, @NeerajW, @cre8ivepark is there any chance MSFT would provide a low poly 3d xbox controller model for the test scene? Or could we load the controller model via glTF & driver like the motion controllers? |
added position and rotation speed modifiers to test scene.
Added a customizable button via enum example.
Does this replace the GamepadInput script added in the dev branch? |
@StephenHodgson I'm not aware of an Xbox controller model, unfortunately. |
Yes. I'm working on a PR to address this soon. |
userPermission = EditorUtility.DisplayDialog("Attention!", | ||
"Hi there, we noticed that you've enabled the Xbox Controller support.\n\n" + | ||
"Do you give us permission to download the latest input mapping definitions from " + | ||
"the Mixed Reality Toolkit's GitHub page and replace your project's InputManager.asset?\n\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to append instead of replace? I'd prefer not to be destructive here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that I'm aware of. This step is optional, and I've kept this flexible where people don't have to use the provided InputManager.asset
and map their own.
I'd also rather overwrite so there are not overlapping or conflicting mappings.
##### SpeechEventData.cs | ||
Event data for an event coming from the speech keyword source. | ||
|
||
##### XboxControllerEVentData.cs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: typo: EVent
{ | ||
public interface IGamePadHandler : IEventSystemHandler | ||
{ | ||
void OnGamePadDetected(GamePadEventData eventData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it's more useful making gamepad input separate events, or to combine them with the motion controller events? They share a set of input axes, and source detected/lost could be handled similarly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept it as a singular event type to reduce complexity. But it will raise separate events for each controller.
I'm already thinking about a plan to make sure sure this plays well with the motion controllers in the dev branch. I'll be sure to request your feedback when I submit.
|
||
for (var i = 0; i < joystickNames.Length; i++) | ||
{ | ||
Debug.LogWarningFormat("Joystick \"{0}\" has not been setup with the input manager.Create a new class that inherits from \"GamePadInputSource\" and implement it.", joystickNames[i]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: missing a space between "." and "Create".
InputManger.asset
file from GitHub and stores the old one.TODO:
Add documentation.Nicelooking test scene.Figure out an easy way for people to copyInputManger.asset
into their own projects.Test Xbox Platform builds.