Map unknown gamepads and read raw gamepad packets #5967
Conversation
Also map unknown gamepads to a default gamepad config
| @@ -379,6 +379,48 @@ namespace dmGameObject | |||
| lua_setfield(L, action_table, "gamepad_name"); | |||
| } | |||
|
|
|||
| if (params.m_InputAction->m_HasGamepadPacket) | |||
There was a problem hiding this comment.
I'm tempted to put this in a action.gamepad table instead:
action.gamepad.axis
action.gamepad.hats
action.gamepad.buttons
There was a problem hiding this comment.
I think it is a good idea. Question is, what else should be put there?
Ofc we'll get some backwards compatibility issues if we move things without adding duplicate data. (e.g. device id)
| @@ -183,7 +183,8 @@ enum Gamepad | |||
| GAMEPAD_GUIDE = 24; | |||
| GAMEPAD_CONNECTED = 25; | |||
| GAMEPAD_DISCONNECTED = 26; | |||
| MAX_GAMEPAD_COUNT = 27; | |||
| GAMEPAD_RAW = 27; | |||
There was a problem hiding this comment.
The trigger to set in the gamepad section of your input bindings to get raw gamepad data from a connected gamepad. With raw data I mean the axis, hats and buttons as they are read from the OS, without applying any deadzone or other operations on the data.
| @@ -26,6 +26,8 @@ | |||
|
|
|||
| namespace dmInput | |||
| { | |||
| const uint32_t UNKNOWN_GAMEPAD_CONFIG_ID = dmHashString32("UNKNOWN_GAMEPAD_CONFIG_ID"); | |||
There was a problem hiding this comment.
Unidentified connected gamepads will be assigned to this id
| @@ -98,6 +100,16 @@ namespace dmInput | |||
| } | |||
| } | |||
|
|
|||
| static GamepadConfig* GetGamepadConfigFromDeviceName(HBinding binding, const uint32_t device_hash) | |||
There was a problem hiding this comment.
Get the gamepad config for the device if it exists. If it doesn't exist the device will be assigned to the unknown gamepad config.
| unknownGamepadConfig.m_DeadZone = 0.0; | ||
| unknownGamepadConfig.m_DeviceId = UNKNOWN_GAMEPAD_CONFIG_ID; | ||
| memset(unknownGamepadConfig.m_Inputs, 0, sizeof(unknownGamepadConfig.m_Inputs)); | ||
| for (uint32_t j = 0; j < (sizeof(unknownGamepadConfig.m_Inputs) / sizeof(struct GamepadInput)); ++j) |
There was a problem hiding this comment.
In array.hwe now have DM_ARRAY_SIZE, which is easier to write, and also handles some special cases.
engine/input/src/input.cpp
Outdated
| memset(unknownGamepadConfig.m_Inputs, 0, sizeof(unknownGamepadConfig.m_Inputs)); | ||
| for (uint32_t j = 0; j < (sizeof(unknownGamepadConfig.m_Inputs) / sizeof(struct GamepadInput)); ++j) | ||
| { | ||
| unknownGamepadConfig.m_Inputs[j].m_Index = (uint16_t)~0; |
There was a problem hiding this comment.
No. We want the change I did from the Switch-repo where I fixed the memory overwrite bug by using a constant instead (e.g. INVALID_INDEX).
I haven't merged that back to the public Defold repo, did you?
Technically, we want the fixes from the private repos back to the main repo so that we can keep the bug fixes up-to-date.
There was a problem hiding this comment.
Merged back to Defold now
| @@ -649,7 +682,7 @@ namespace dmInput | |||
| dmHID::GamepadPacket* packet = &gamepad_binding->m_Packet; | |||
|
|
|||
| dmHID::GamepadPacket* prev_packet = &gamepad_binding->m_PreviousPacket; | |||
| GamepadConfig* config = binding->m_Context->m_GamepadMaps.Get(gamepad_binding->m_DeviceId); | |||
| GamepadConfig* config = GetGamepadConfigFromDeviceName(binding, gamepad_binding->m_DeviceId); | |||
There was a problem hiding this comment.
I'm actually not entirely sure why we don't store the pointer to the config in the gamepad binding struct.
Possibly it's because if the memory of the configs would move, i.e. if the table is reallocated (e.g. after some new configs are inserted). But we we read all configs at startup, and then don't touch that table. Strange.
| @@ -379,6 +379,48 @@ namespace dmGameObject | |||
| lua_setfield(L, action_table, "gamepad_name"); | |||
| } | |||
|
|
|||
| if (params.m_InputAction->m_HasGamepadPacket) | |||
| { | |||
| dmHID::GamepadPacket gamepadPacket = params.m_InputAction->m_GamepadPacket; | |||
There was a problem hiding this comment.
I'm not sure about the performance difference here versus using a GamepadPacker*. On one hand we're copying the 144bytes, and have the data in a local cache, versus using a pointer, and it might thrash the caches.
I think this setup is probably better.
I only mention it since I don't think we normally use this pattern of copying a whole struct on the stack, but I'm thinking it's a good use case for itt.
| @@ -379,6 +379,48 @@ namespace dmGameObject | |||
| lua_setfield(L, action_table, "gamepad_name"); | |||
| } | |||
|
|
|||
| if (params.m_InputAction->m_HasGamepadPacket) | |||
There was a problem hiding this comment.
I think it is a good idea. Question is, what else should be put there?
Ofc we'll get some backwards compatibility issues if we move things without adding duplicate data. (e.g. device id)
Fixes #5895