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

Rename many global enums relating to input #38054

Merged
merged 2 commits into from
Mar 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions core/core_constants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,12 @@ static Vector<_CoreConstant> _global_constants;

#endif

VARIANT_ENUM_CAST(KeyList);
VARIANT_ENUM_CAST(Key);
Copy link
Member

@akien-mga akien-mga Mar 23, 2021

Choose a reason for hiding this comment

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

Only remaining concern that I have is that some users might not be happy with the fairly generic (3 letter) Key identifier being taken in the global scope. Might lead to issues if one wants to name a class Key for a key item, or naming a public property Key in C# (which I think uses PascalCase for public properties).

Of course I agree that the name is correct for the enum and matches the constants' naming scheme. But that might be the reason why this (and other enums, for consistency) was named KeyList and not Key in the first place.

Copy link
Member Author

Choose a reason for hiding this comment

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

We also already have name conflicts between properties and non-properties in the core Godot API for C#.

One option is to make KeyList be the exception, or we could do something else like Keyboard or KeyCode. Key seems fine to me though.

Copy link
Member

Choose a reason for hiding this comment

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

KeyCode sounds good. InputEventKey has a property with this name.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I guess KeyCode might be fine. It does re-add an inconsistency, but to some extent the KeyList enumerated values are actually different from the other enums. They're internal keycodes while the rest are indices (0 to max).

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we have KEY_ENTER or KEY_CODE_ENTER?

Copy link
Member

Choose a reason for hiding this comment

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

KEY_ENTER

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, we can't use KeyCode, it conflicts with X11:

/usr/include/X11/X.h:108:23: note: 'KeyCode' has a previous declaration here
  108 | typedef unsigned char KeyCode;
      |                       ^~~~~~~

Copy link
Member

Choose a reason for hiding this comment

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

Let's give it a go with Key and see what happens. If it turns out problematic, we can just do the few renames necessary before the 4.0 release.

VARIANT_ENUM_CAST(KeyModifierMask);
VARIANT_ENUM_CAST(ButtonList);
VARIANT_ENUM_CAST(JoyButtonList);
VARIANT_ENUM_CAST(JoyAxisList);
VARIANT_ENUM_CAST(MidiMessageList);
VARIANT_ENUM_CAST(MouseButton);
VARIANT_ENUM_CAST(JoyButton);
VARIANT_ENUM_CAST(JoyAxis);
VARIANT_ENUM_CAST(MIDIMessage);

void register_global_constants() {
BIND_CORE_ENUM_CONSTANT(SIDE_LEFT);
Expand Down Expand Up @@ -397,20 +397,20 @@ void register_global_constants() {
BIND_CORE_ENUM_CONSTANT(KEY_MASK_GROUP_SWITCH);

// mouse
BIND_CORE_ENUM_CONSTANT(BUTTON_LEFT);
BIND_CORE_ENUM_CONSTANT(BUTTON_RIGHT);
BIND_CORE_ENUM_CONSTANT(BUTTON_MIDDLE);
BIND_CORE_ENUM_CONSTANT(BUTTON_XBUTTON1);
BIND_CORE_ENUM_CONSTANT(BUTTON_XBUTTON2);
BIND_CORE_ENUM_CONSTANT(BUTTON_WHEEL_UP);
BIND_CORE_ENUM_CONSTANT(BUTTON_WHEEL_DOWN);
BIND_CORE_ENUM_CONSTANT(BUTTON_WHEEL_LEFT);
BIND_CORE_ENUM_CONSTANT(BUTTON_WHEEL_RIGHT);
BIND_CORE_ENUM_CONSTANT(BUTTON_MASK_LEFT);
BIND_CORE_ENUM_CONSTANT(BUTTON_MASK_RIGHT);
BIND_CORE_ENUM_CONSTANT(BUTTON_MASK_MIDDLE);
BIND_CORE_ENUM_CONSTANT(BUTTON_MASK_XBUTTON1);
BIND_CORE_ENUM_CONSTANT(BUTTON_MASK_XBUTTON2);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_LEFT);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_RIGHT);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_MIDDLE);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_XBUTTON1);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_XBUTTON2);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_WHEEL_UP);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_WHEEL_DOWN);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_WHEEL_LEFT);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_WHEEL_RIGHT);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_MASK_LEFT);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_MASK_RIGHT);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_MASK_MIDDLE);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_MASK_XBUTTON1);
BIND_CORE_ENUM_CONSTANT(MOUSE_BUTTON_MASK_XBUTTON2);

// Joypad buttons
BIND_CORE_ENUM_CONSTANT(JOY_BUTTON_INVALID);
Expand Down
32 changes: 16 additions & 16 deletions core/input/input.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -560,11 +560,11 @@ void Input::_parse_input_event_impl(const Ref<InputEvent> &p_event, bool p_is_em
button_event->set_position(st->get_position());
button_event->set_global_position(st->get_position());
button_event->set_pressed(st->is_pressed());
button_event->set_button_index(BUTTON_LEFT);
button_event->set_button_index(MOUSE_BUTTON_LEFT);
if (st->is_pressed()) {
button_event->set_button_mask(mouse_button_mask | (1 << (BUTTON_LEFT - 1)));
button_event->set_button_mask(mouse_button_mask | (1 << (MOUSE_BUTTON_LEFT - 1)));
} else {
button_event->set_button_mask(mouse_button_mask & ~(1 << (BUTTON_LEFT - 1)));
button_event->set_button_mask(mouse_button_mask & ~(1 << (MOUSE_BUTTON_LEFT - 1)));
}

_parse_input_event_impl(button_event, true);
Expand Down Expand Up @@ -792,8 +792,8 @@ void Input::ensure_touch_mouse_raised() {
button_event->set_position(mouse_pos);
button_event->set_global_position(mouse_pos);
button_event->set_pressed(false);
button_event->set_button_index(BUTTON_LEFT);
button_event->set_button_mask(mouse_button_mask & ~(1 << (BUTTON_LEFT - 1)));
button_event->set_button_index(MOUSE_BUTTON_LEFT);
button_event->set_button_mask(mouse_button_mask & ~(1 << (MOUSE_BUTTON_LEFT - 1)));

_parse_input_event_impl(button_event, true);
}
Expand Down Expand Up @@ -907,7 +907,7 @@ void Input::joy_button(int p_device, int p_button, bool p_pressed) {
// no event?
}

void Input::joy_axis(int p_device, int p_axis, const JoyAxis &p_value) {
void Input::joy_axis(int p_device, int p_axis, const JoyAxisValue &p_value) {
_THREAD_SAFE_METHOD_;

ERR_FAIL_INDEX(p_axis, JOY_AXIS_MAX);
Expand All @@ -921,12 +921,12 @@ void Input::joy_axis(int p_device, int p_axis, const JoyAxis &p_value) {
//when changing direction quickly, insert fake event to release pending inputmap actions
float last = joy.last_axis[p_axis];
if (p_value.min == 0 && (last < 0.25 || last > 0.75) && (last - 0.5) * (p_value.value - 0.5) < 0) {
JoyAxis jx;
JoyAxisValue jx;
jx.min = p_value.min;
jx.value = p_value.value < 0.5 ? 0.6 : 0.4;
joy_axis(p_device, p_axis, jx);
} else if (ABS(last) > 0.5 && last * p_value.value <= 0) {
JoyAxis jx;
JoyAxisValue jx;
jx.min = p_value.min;
jx.value = last > 0 ? 0.1 : -0.1;
joy_axis(p_device, p_axis, jx);
Expand Down Expand Up @@ -1206,22 +1206,22 @@ void Input::_get_mapped_hat_events(const JoyDeviceMapping &mapping, int p_hat, J
}
}

JoyButtonList Input::_get_output_button(String output) {
JoyButton Input::_get_output_button(String output) {
for (int i = 0; i < JOY_BUTTON_SDL_MAX; i++) {
if (output == _joy_buttons[i]) {
return JoyButtonList(i);
return JoyButton(i);
}
}
return JoyButtonList::JOY_BUTTON_INVALID;
return JoyButton::JOY_BUTTON_INVALID;
}

JoyAxisList Input::_get_output_axis(String output) {
JoyAxis Input::_get_output_axis(String output) {
for (int i = 0; i < JOY_AXIS_SDL_MAX; i++) {
if (output == _joy_axes[i]) {
return JoyAxisList(i);
return JoyAxis(i);
}
}
return JoyAxisList::JOY_AXIS_INVALID;
return JoyAxis::JOY_AXIS_INVALID;
}

void Input::parse_mapping(String p_mapping) {
Expand Down Expand Up @@ -1279,8 +1279,8 @@ void Input::parse_mapping(String p_mapping) {
input = input.left(input.length() - 1);
}

JoyButtonList output_button = _get_output_button(output);
JoyAxisList output_axis = _get_output_axis(output);
JoyButton output_button = _get_output_button(output);
JoyAxis output_axis = _get_output_axis(output);
ERR_CONTINUE_MSG(output_button == JOY_BUTTON_INVALID && output_axis == JOY_AXIS_INVALID,
String(entry[idx] + "\nUnrecognised output string: " + output));
ERR_CONTINUE_MSG(output_button != JOY_BUTTON_INVALID && output_axis != JOY_AXIS_INVALID,
Expand Down
12 changes: 6 additions & 6 deletions core/input/input.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class Input : public Object {
JOYPADS_MAX = 16,
};

struct JoyAxis {
struct JoyAxisValue {
int min;
float value;
};
Expand Down Expand Up @@ -199,10 +199,10 @@ class Input : public Object {

JoyType outputType;
union {
JoyButtonList button;
JoyButton button;

struct {
JoyAxisList axis;
JoyAxis axis;
JoyAxisRange range;
} axis;

Expand All @@ -220,8 +220,8 @@ class Input : public Object {
JoyEvent _get_mapped_button_event(const JoyDeviceMapping &mapping, int p_button);
JoyEvent _get_mapped_axis_event(const JoyDeviceMapping &mapping, int p_axis, float p_value);
void _get_mapped_hat_events(const JoyDeviceMapping &mapping, int p_hat, JoyEvent r_events[HAT_MAX]);
JoyButtonList _get_output_button(String output);
JoyAxisList _get_output_axis(String output);
JoyButton _get_output_button(String output);
JoyAxis _get_output_axis(String output);
void _button_event(int p_device, int p_index, bool p_pressed);
void _axis_event(int p_device, int p_axis, float p_value);

Expand Down Expand Up @@ -325,7 +325,7 @@ class Input : public Object {

void parse_mapping(String p_mapping);
void joy_button(int p_device, int p_button, bool p_pressed);
void joy_axis(int p_device, int p_axis, const JoyAxis &p_value);
void joy_axis(int p_device, int p_axis, const JoyAxisValue &p_value);
void joy_hat(int p_device, int p_val);

void add_joy_mapping(String p_mapping, bool p_update_existing = false);
Expand Down
56 changes: 28 additions & 28 deletions core/input/input_event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -619,15 +619,15 @@ String InputEventMouseButton::as_text() const {
// Button
int idx = get_button_index();
switch (idx) {
case BUTTON_LEFT:
case BUTTON_RIGHT:
case BUTTON_MIDDLE:
case BUTTON_WHEEL_UP:
case BUTTON_WHEEL_DOWN:
case BUTTON_WHEEL_LEFT:
case BUTTON_WHEEL_RIGHT:
case BUTTON_XBUTTON1:
case BUTTON_XBUTTON2:
case MOUSE_BUTTON_LEFT:
case MOUSE_BUTTON_RIGHT:
case MOUSE_BUTTON_MIDDLE:
case MOUSE_BUTTON_WHEEL_UP:
case MOUSE_BUTTON_WHEEL_DOWN:
case MOUSE_BUTTON_WHEEL_LEFT:
case MOUSE_BUTTON_WHEEL_RIGHT:
case MOUSE_BUTTON_XBUTTON1:
case MOUSE_BUTTON_XBUTTON2:
full_string += RTR(_mouse_button_descriptions[idx - 1]); // button index starts from 1, array index starts from 0, so subtract 1
break;
default:
Expand All @@ -651,15 +651,15 @@ String InputEventMouseButton::to_string() {
String button_string = itos(idx);

switch (idx) {
case BUTTON_LEFT:
case BUTTON_RIGHT:
case BUTTON_MIDDLE:
case BUTTON_WHEEL_UP:
case BUTTON_WHEEL_DOWN:
case BUTTON_WHEEL_LEFT:
case BUTTON_WHEEL_RIGHT:
case BUTTON_XBUTTON1:
case BUTTON_XBUTTON2:
case MOUSE_BUTTON_LEFT:
case MOUSE_BUTTON_RIGHT:
case MOUSE_BUTTON_MIDDLE:
case MOUSE_BUTTON_WHEEL_UP:
case MOUSE_BUTTON_WHEEL_DOWN:
case MOUSE_BUTTON_WHEEL_LEFT:
case MOUSE_BUTTON_WHEEL_RIGHT:
case MOUSE_BUTTON_XBUTTON1:
case MOUSE_BUTTON_XBUTTON2:
button_string += " (" + RTR(_mouse_button_descriptions[idx - 1]) + ")"; // button index starts from 1, array index starts from 0, so subtract 1
break;
default:
Expand Down Expand Up @@ -761,20 +761,20 @@ String InputEventMouseMotion::to_string() {
int button_mask = get_button_mask();
String button_mask_string = itos(button_mask);
switch (get_button_mask()) {
case BUTTON_MASK_LEFT:
button_mask_string += " (" + RTR(_mouse_button_descriptions[BUTTON_LEFT - 1]) + ")";
case MOUSE_BUTTON_MASK_LEFT:
button_mask_string += " (" + RTR(_mouse_button_descriptions[MOUSE_BUTTON_LEFT - 1]) + ")";
break;
case BUTTON_MASK_MIDDLE:
button_mask_string += " (" + RTR(_mouse_button_descriptions[BUTTON_MIDDLE - 1]) + ")";
case MOUSE_BUTTON_MASK_MIDDLE:
button_mask_string += " (" + RTR(_mouse_button_descriptions[MOUSE_BUTTON_MIDDLE - 1]) + ")";
break;
case BUTTON_MASK_RIGHT:
button_mask_string += " (" + RTR(_mouse_button_descriptions[BUTTON_RIGHT - 1]) + ")";
case MOUSE_BUTTON_MASK_RIGHT:
button_mask_string += " (" + RTR(_mouse_button_descriptions[MOUSE_BUTTON_RIGHT - 1]) + ")";
break;
case BUTTON_MASK_XBUTTON1:
button_mask_string += " (" + RTR(_mouse_button_descriptions[BUTTON_XBUTTON1 - 1]) + ")";
case MOUSE_BUTTON_MASK_XBUTTON1:
button_mask_string += " (" + RTR(_mouse_button_descriptions[MOUSE_BUTTON_XBUTTON1 - 1]) + ")";
break;
case BUTTON_MASK_XBUTTON2:
button_mask_string += " (" + RTR(_mouse_button_descriptions[BUTTON_XBUTTON2 - 1]) + ")";
case MOUSE_BUTTON_MASK_XBUTTON2:
button_mask_string += " (" + RTR(_mouse_button_descriptions[MOUSE_BUTTON_XBUTTON2 - 1]) + ")";
break;
default:
break;
Expand Down
36 changes: 18 additions & 18 deletions core/input/input_event.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,24 +42,24 @@
* The events are pretty obvious.
*/

enum ButtonList {
BUTTON_LEFT = 1,
BUTTON_RIGHT = 2,
BUTTON_MIDDLE = 3,
BUTTON_WHEEL_UP = 4,
BUTTON_WHEEL_DOWN = 5,
BUTTON_WHEEL_LEFT = 6,
BUTTON_WHEEL_RIGHT = 7,
BUTTON_XBUTTON1 = 8,
BUTTON_XBUTTON2 = 9,
BUTTON_MASK_LEFT = (1 << (BUTTON_LEFT - 1)),
BUTTON_MASK_RIGHT = (1 << (BUTTON_RIGHT - 1)),
BUTTON_MASK_MIDDLE = (1 << (BUTTON_MIDDLE - 1)),
BUTTON_MASK_XBUTTON1 = (1 << (BUTTON_XBUTTON1 - 1)),
BUTTON_MASK_XBUTTON2 = (1 << (BUTTON_XBUTTON2 - 1))
enum MouseButton {
MOUSE_BUTTON_LEFT = 1,
MOUSE_BUTTON_RIGHT = 2,
MOUSE_BUTTON_MIDDLE = 3,
MOUSE_BUTTON_WHEEL_UP = 4,
MOUSE_BUTTON_WHEEL_DOWN = 5,
MOUSE_BUTTON_WHEEL_LEFT = 6,
MOUSE_BUTTON_WHEEL_RIGHT = 7,
MOUSE_BUTTON_XBUTTON1 = 8,
MOUSE_BUTTON_XBUTTON2 = 9,
MOUSE_BUTTON_MASK_LEFT = (1 << (MOUSE_BUTTON_LEFT - 1)),
MOUSE_BUTTON_MASK_RIGHT = (1 << (MOUSE_BUTTON_RIGHT - 1)),
MOUSE_BUTTON_MASK_MIDDLE = (1 << (MOUSE_BUTTON_MIDDLE - 1)),
MOUSE_BUTTON_MASK_XBUTTON1 = (1 << (MOUSE_BUTTON_XBUTTON1 - 1)),
MOUSE_BUTTON_MASK_XBUTTON2 = (1 << (MOUSE_BUTTON_XBUTTON2 - 1))
};

enum JoyButtonList {
enum JoyButton {
JOY_BUTTON_INVALID = -1,
JOY_BUTTON_A = 0,
JOY_BUTTON_B = 1,
Expand All @@ -86,7 +86,7 @@ enum JoyButtonList {
JOY_BUTTON_MAX = 36, // Android supports up to 36 buttons.
};

enum JoyAxisList {
enum JoyAxis {
JOY_AXIS_INVALID = -1,
JOY_AXIS_LEFT_X = 0,
JOY_AXIS_LEFT_Y = 1,
Expand All @@ -98,7 +98,7 @@ enum JoyAxisList {
JOY_AXIS_MAX = 10, // OpenVR supports up to 5 Joysticks making a total of 10 axes.
};

enum MidiMessageList {
enum MIDIMessage {
MIDI_MESSAGE_NOTE_OFF = 0x8,
MIDI_MESSAGE_NOTE_ON = 0x9,
MIDI_MESSAGE_AFTERTOUCH = 0xA,
Expand Down
2 changes: 1 addition & 1 deletion core/os/keyboard.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ enum {
SPKEY = (1 << 24)
};

enum KeyList {
enum Key {
/* CURSOR/FUNCTION/BROWSER/MULTIMEDIA/MISC KEYS */
KEY_ESCAPE = SPKEY | 0x01,
KEY_TAB = SPKEY | 0x02,
Expand Down
Loading