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

Finalise names for joypad buttons and document them #43520

Closed
clarfonthey opened this issue Nov 14, 2020 · 4 comments · Fixed by #43591
Closed

Finalise names for joypad buttons and document them #43520

clarfonthey opened this issue Nov 14, 2020 · 4 comments · Fixed by #43591

Comments

@clarfonthey
Copy link

clarfonthey commented Nov 14, 2020

(Originally: naming differences from PR #38151)

Basic summary: Input.get_joy_button_string and Input.get_joy_button_index_from_string serialize the face buttons for controllers as "Face Button (Dir)" on 3.2 whereas on master they serialize them as "Face (Dir)". I traced this change down to PR #38151.

I found this because the actual strings weren't in the docs, and I was looking through the source code to see what they were. I then noticed that master is different from the stable 3.2 that I'm using.

Even though Godot 4 can have breaking changes from Godot 3, I think that this change was probably just an honest mistake, seeing how it will affect the primary way people store and load button mappings in config files. I also personally think the name "Face Button" is clearer, as "Face" doesn't make as much sense to me.

If this is properly documented somewhere as an intended change, this can be closed -- I'm mostly saying that it seems like it wasn't intended, and that it probably should be reverted if that's the case.

@Calinou
Copy link
Member

Calinou commented Nov 14, 2020

cc @madmiraal

@madmiraal
Copy link
Contributor

The change was deliberate, and the intention was to not only make the names consistent, but to actually provide names for all the buttons and axes. The word "Button" was dropped from the "Face x" names, because, first, they were the only buttons with the word "Button" in the name, and, second, the get_joy_button_string() and get_joy_button_index_from_string() functions are specific to buttons, so there is no need to add "Button" to all the button names.

The previous names were:

static const char *_buttons[JOY_BUTTON_MAX] = { 
 	"Face Button Bottom", 
 	"Face Button Right", 
 	"Face Button Left", 
 	"Face Button Top", 
 	"L", 
 	"R", 
 	"L2", 
 	"R2", 
 	"L3", 
 	"R3", 
 	"Select", 
 	"Start", 
 	"DPAD Up", 
 	"DPAD Down", 
 	"DPAD Left", 
 	"DPAD Right" 
 }; 
  
 static const char *_axes[JOY_AXIS_MAX] = { 
 	"Left Stick X", 
 	"Left Stick Y", 
 	"Right Stick X", 
 	"Right Stick Y", 
 	"", 
 	"", 
 	"L2", 
 	"R2", 
 	"", 
 	"" 
 }; 

The new names are:

static const char *_joy_button_names[JOY_BUTTON_MAX] = {
	"Face Bottom",
	"Face Right",
	"Face Left",
	"Face Top",
	"Select",
	"Guide",
	"Start",
	"Left Stick",
	"Right Stick",
	"Left Shoulder",
	"Right Shoulder",
	"D-Pad Up",
	"D-Pad Down",
	"D-Pad Left",
	"D-Pad Right",
	"Button 15",
	"Button 16",
	"Button 17",
	"Button 18",
	"Button 19",
	"Button 20",
	"Button 21",
	"Button 22",
	"Button 23",
	"Button 24",
	"Button 25",
	"Button 26",
	"Button 27",
	"Button 28",
	"Button 29",
	"Button 30",
	"Button 31",
	"Button 32",
	"Button 33",
	"Button 34",
	"Button 35"
};

static const char *_joy_axis_names[JOY_AXIS_MAX] = {
	"Left Stick X",
	"Left Stick Y",
	"Right Stick X",
	"Right Stick Y",
	"Left Trigger",
	"Right Trigger",
	"Joystick 3 Stick X",
	"Joystick 3 Stick Y",
	"Joystick 4 Stick X",
	"Joystick 4 Stick Y"
};

There was a question about whether "Face" was the right name for these buttons, but for lack of an alternative, I kept the name "Face". Is there a better alternative?

Looking at the new names, it would also be better if the Joystick 3 and 4 names were changed to just "Joystick 3 X", "Joystick 3 Y", "Joystick 4 X" and "Joystick 4 Y".

That all been said, what is a good point is that these names should be documented.

@clarfonthey
Copy link
Author

Thanks for the clarification! I'm in favour of making the names consistent, and I've personally been grappling with how I should use these strings in my own way of storing bindings. I do think that "Face" is a bit weird, and maybe it might be better to label the D-Pad as "Direction Button (Dir)" and the face buttons as "Action Button (Dir)". Additionally, the distinction between face buttons using "top" and "bottom" while the D-pad uses "up" and "down" is a bit weird, and it might be better to just say "up" and "down" everywhere.

@clarfonthey clarfonthey changed the title PR #38151 changed "Face Button" to just "Face" Finalise names for joypad buttons and document them Nov 14, 2020
@madmiraal
Copy link
Contributor

madmiraal commented Nov 16, 2020

I do think that "Face" is a bit weird, and maybe it might be better to label the D-Pad as "Direction Button (Dir)" and the face buttons as "Action Button (Dir)". Additionally, the distinction between face buttons using "top" and "bottom" while the D-pad uses "up" and "down" is a bit weird, and it might be better to just say "up" and "down" everywhere.

  • "D-pad" is the right name for those buttons see Wikipedia.
  • I agree "Action" may be a better word than "Face".
  • I think "up" and "down", "left" and "right" are correct for the "D-pad" buttons, because they are about direction, but I think, "top" and "bottom", "left" and "right" are correct for the "Action" buttons, because it's about identifying their location on the game controller.

In summary, I think we should use the following strings:

static const char *_joy_button_names[JOY_BUTTON_MAX] = {
	"Bottom Action",
	"Right Action",
	"Left Action",
	"Top Action",
	"Select",
	"Guide",
	"Start",
	"Left Stick",
	"Right Stick",
	"Left Shoulder",
	"Right Shoulder",
	"D-pad Up",
	"D-pad Down",
	"D-pad Left",
	"D-pad Right",
	"Button 15",
	"Button 16",
	"Button 17",
	"Button 18",
	"Button 19",
	"Button 20",
	"Button 21",
	"Button 22",
	"Button 23",
	"Button 24",
	"Button 25",
	"Button 26",
	"Button 27",
	"Button 28",
	"Button 29",
	"Button 30",
	"Button 31",
	"Button 32",
	"Button 33",
	"Button 34",
	"Button 35"

static const char *_joy_axis_names[JOY_AXIS_MAX] = {
	"Left Stick X",
	"Left Stick Y",
	"Right Stick X",
	"Right Stick Y",
	"Left Trigger",
	"Right Trigger",
	"Joystick 3 X",
	"Joystick 3 Y",
	"Joystick 4 X",
	"Joystick 4 Y"
};

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

Successfully merging a pull request may close this issue.

4 participants