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

[Joypads] Add SDL config re-mapping tool. #575

Merged
merged 1 commit into from
Feb 7, 2021

Conversation

Faless
Copy link
Contributor

@Faless Faless commented Jan 18, 2021

Most of the code is in the remap folder, but it depends on the gamepad
diagram scene.
It allows remapping of pads to values that godot can understand.
It also comes with some default mapping for the HTML5 platform.

See godotengine/godot#45078

remap

@aaronfranke aaronfranke self-requested a review January 19, 2021 10:45
Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ensure that PRs don't have warnings and follow all style guides.

misc/joypads/remap/remap_wizard.gd Outdated Show resolved Hide resolved
misc/joypads/remap/joy_mapping.gd Outdated Show resolved Hide resolved
misc/joypads/remap/joy_mapping.gd Show resolved Hide resolved
misc/joypads/remap/remap_wizard.gd Outdated Show resolved Hide resolved
Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work correctly in my testing, tested with both 3.2.3 and 3.2.4 beta 6. I tried remapping a Stadia controller on Windows. Firstly, I can't map the A button unless I click on a UI element to change the focused button, because the A button immediately closes the wizard (this needs to be fixed, you can probably just focus some other UI element to prevent this). Anyway, I clicked elsewhere and mapped it and this is what I got on 3.2.3:

03000000d11800000094000000000000,Stadia Controller,a:b0,b:b1,y:b3,x:b2,start:b9,back:b8,leftstick:b6,rightstick:b7,leftshoulder:b4,rightshoulder:b5,dpup:b12,dpleft:b14,dpdown:b13,dpright:b15,leftx:a0,lefty:a2,rightx:a3,righty:a5,lefttrigger:b12,righttrigger:b11,platform:Javascript

There are several problems with this. For one, why does it say "platform:Javascript" when this is Windows? For another, the triggers don't seem to map correctly. What was really confusing is that "Clear Mapping" doesn't actually set it to the default mapping, and clicking "Clear Mapping" messes up the whole mapping (while the default mapping when starting the program is correct).

For the left trigger, the default mapping has it as Button 6 (interpreted as left trigger), and Steam detects it as Button 12 (interpreted as left trigger), and the remapper detects it as Button 12 (interpreted as D-pad up), then clicking "Clear Mapping" sets it to Button 12 (interpreted as D-pad up).

For the right trigger, the default mapping has it as Button 7 (interpreted as right trigger), and Steam detects it as Button 11 (interpreted as right trigger), and the remapper detects it as Button 7 (interpreted as left trigger), then clicking "Clear Mapping" sets it to Button 11 (interpreted as the Start button).

For the D-pad, Godot detects it as buttons 12-15, but Steam detects it as "Hat 0.1" for up, "Hat 0.8" for left, "Hat 0.4" for down, and "Hat 0.2" for right. The remapper works for the D-pad, but this behavior seems strange.

When clicking "Clear Mapping", the triggers are interpreted as D-pad up and Start, and Select/Start are interpreted as the joystick clicks, and the joystick clicks are interpreted as the triggers. Nothing maps to Select, two things map to D-pad up.

Then I tried on 3.2.4 beta 6 and I got the exact same behavior, same map string, same bad behavior, same broken "Clear Mapping" button.

There is also the popup with "Try a common mapping" that I can see in the Godot editor, it doesn't show up when clicking any of the buttons on the bottom (Remap starts the wizard immediately, Show brings up text, and Clear Mapping doesn't open anything), so it seems that this menu is impossible to get to (and should either become accessible, or deleted).

@Faless
Copy link
Contributor Author

Faless commented Jan 21, 2021

I can't map the A button unless I click on a UI element to change the focused button

That is indeed a bug, we need to ignore the gamepad UI actions, I did that in my test project but forgot about it.
I will try to find a workaround for it.

What was really confusing is that "Clear Mapping" doesn't actually set it to the default mapping, and clicking "Clear Mapping" messes up the whole mapping (while the default mapping when starting the program is correct).

As far as I understand resetting to the godot builtin mapping is not possible via scripting API, "clear mapping" removes the current mapping, and shows "raw" inputs.

For the D-pad, Godot detects it as buttons 12-15, but Steam detects it as "Hat 0.1" for up, "Hat 0.8" for left, "Hat 0.4" for down, and "Hat 0.2" for right. The remapper works for the D-pad, but this behavior seems strange.

Godot exposes Hats as buttons. So, there isn't much we can do about it.

There is also the popup with "Try a common mapping" that I can see in the Godot editor, it doesn't show up when clicking any of the buttons on the bottom (Remap starts the wizard immediately, Show brings up text, and Clear Mapping doesn't open anything), so it seems that this menu is impossible to get to (and should either become accessible, or deleted).

That's only for platforms that have known mappings (for now, HTML5 only).

@Faless
Copy link
Contributor Author

Faless commented Feb 4, 2021

@aaronfranke
Thanks a lot for the review.
I've fixed both the issue with the platform name, and the one about input not being properly consumed.

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now this is what I get when mapping a Stadia controller on Windows:

03000000d11800000094000000000000,Stadia Controller,a:b0,b:b1,y:b3,x:b2,start:b9,back:b8,leftstick:b6,rightstick:b7,leftshoulder:b4,rightshoulder:b5,dpup:b12,dpleft:b14,dpdown:b13,dpright:b15,leftx:a0,lefty:a2,rightx:a3,righty:a5,lefttrigger:b12,righttrigger:b11,platform:Windows

I am able to map all inputs without the menu closing when pressing A, and the platform is reported correctly. Using the remapper tool, the only button bound incorrectly is the left trigger which is detected as D-pad up. In my last comment I reported that the right trigger was bound incorrectly, but it seems to be working now.

As far as I understand resetting to the godot builtin mapping is not possible via scripting API, "clear mapping" removes the current mapping, and shows "raw" inputs.

I see, but we still need to make it clear what's happening, since "Clear Mapping" does not convey this. What about "Raw Mapping", "Set to Raw Mapping", or "Set to Raw"?

misc/joypads/joypads.tscn Show resolved Hide resolved
misc/joypads/remap/remap_wizard.tscn Outdated Show resolved Hide resolved
misc/joypads/remap/remap_wizard.tscn Outdated Show resolved Hide resolved
misc/joypads/remap/remap_wizard.tscn Outdated Show resolved Hide resolved
misc/joypads/remap/remap_wizard.gd Outdated Show resolved Hide resolved
misc/joypads/remap/remap_wizard.gd Outdated Show resolved Hide resolved
misc/joypads/remap/remap_wizard.gd Outdated Show resolved Hide resolved
misc/joypads/remap/remap_wizard.gd Outdated Show resolved Hide resolved
misc/joypads/remap/remap_wizard.gd Outdated Show resolved Hide resolved
Most of the code is in the remap folder, but it depends on the gamepad
diagram scene.
It allows remapping of pads to values that godot can understand.
It also comes with some default mapping for the HTML5 platform.
@Faless
Copy link
Contributor Author

Faless commented Feb 7, 2021

Okay, I think I addressed all the remaining issues.

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants