GNOME Bugzilla – Bug 782611
gamepad: Simplify the gamepad system
Last modified: 2017-08-14 07:32:35 UTC
The gamepad system is quite complex for the moment: - it has its own custom types, requiring conversion from the native code type - gamepad events are poorly defined - the gamepad's GUID can't be accessed from the Gamepad class - the hardware events can't be accessed from the Gamepad class - the mapping of a gamepad can't be directly edited - the code of the Mapping class is somewhat hard to read - the distinction between Gamepad and RawGamepad is somewhat artificial\ Because of all that I suggest to refactor the gamepad system to make it simpler.
Created attachment 351811 [details] [review] event: Add Event This will be used to propagate gamepad events and to replace the current gamepad event system by a simpler one.
Created attachment 351812 [details] [review] gamepad: Add the 'event' signal to RawGamepad This will be used to propagate gamepad events in a simpler way.
Created attachment 351813 [details] [review] gamepad: Make LinuxrawGamepad emit the 'event' signal This will be used to propagate gamepad events in a simpler way.
Created attachment 351814 [details] [review] gamepad: Add the 'event' signal to Gamepad This will be used to propagate gamepad events from the raw gamepad.
Created attachment 351815 [details] [review] gamepad: Handle 'event' signal from RawGamepad Make the Gamepad class handle the 'event' signal from RawGamepad instead of any other event signal from it. This will be used to propagate gamepad events in a simpler way.
Created attachment 351816 [details] [review] gamepad: Stop emitting specific gamepad events Don't emit any '*-event' signal so 'event' is the only signal emitting gamepad events as they are not listend to anymore.
Created attachment 351817 [details] [review] gamepad: Remove specific gamepad event signals Drop the '*-event' signals as they are neither listened nor emitted.
Created attachment 351818 [details] [review] event: Add linux/input-event-codes.h This will allow to use the Linux input event codes in non-Linux dependent parts of the code to standardize on the Linux input codes instead of our custom values.
Created attachment 351819 [details] [review] gamepad: Map the input types to Linux event codes This will help transitionning from the custom input types to Linux event codes.
Created attachment 351820 [details] [review] gamepad: Drop usage of StandardGamepadButton in Vala
Created attachment 351821 [details] [review] gamepad: Drop usage of StandardGamepadAxis in Vala
Created attachment 351822 [details] [review] gamepad: Remove custom types Remove StandardGamepadButton, StandardGamepadAxis and GamepadInputType and directly use the corresponding Linux event types and codes.
Created attachment 351823 [details] [review] gamepad: Rename GamepadMappedEvent into GamepadInput This better reflect the goal of the type and make it shorter.
Created attachment 351824 [details] [review] gamepad: Merge GamepadInput and input_t As event codes use generic types now there is no need to have two fields to distinguish their types in GamepadInput. Merging the various type specific values make GamepadInput similar to input_t, the later can then be replaced.
Created attachment 351825 [details] [review] gamepad: Simplify mapping destination parsing Replace parse_input_type(), parse_axis() and parse_button() by the new parse_destination() function, making parsing the destination of the mapping simpler.
Created attachment 351826 [details] [review] gamepad: Make names clear in GamepadMapping Rename some variables and functions in the GamepadMapping class to make a better distinction between the input source and the destionation it maps to.
Created attachment 351827 [details] [review] gamepad: Make gamepad emit Event objects Replace the 'button-event' and 'axis-event' signals from Gamepad by the 'button-press-event', 'button-release-event', 'axis-event' and 'hat-event' ones sending Event objects after mapping them is the Gamepad object has a mapping, or directly if it doesn't.
Review of attachment 351811 [details] [review]: Tried my best :P ::: src/event/event.c @@ +22,3 @@ + g_return_val_if_fail (self, NULL); + + copy = games_event_new (); copy is probably missing a memset?
Review of attachment 351811 [details] [review]: ::: src/event/event.c @@ +22,3 @@ + g_return_val_if_fail (self, NULL); + + copy = games_event_new (); Probably more a memcpy, good catch. :)
Attachment 351811 [details] pushed as f602e51 - event: Add Event Attachment 351812 [details] pushed as 49659f1 - gamepad: Add the 'event' signal to RawGamepad Attachment 351813 [details] pushed as fb85071 - gamepad: Make LinuxrawGamepad emit the 'event' signal Attachment 351814 [details] pushed as ae3f974 - gamepad: Add the 'event' signal to Gamepad Attachment 351815 [details] pushed as 4df760b - gamepad: Handle 'event' signal from RawGamepad Attachment 351816 [details] pushed as 718d7aa - gamepad: Stop emitting specific gamepad events Attachment 351817 [details] pushed as df3ff58 - gamepad: Remove specific gamepad event signals Attachment 351818 [details] pushed as 7f7c0df - event: Add linux/input-event-codes.h Attachment 351819 [details] pushed as 8af72d7 - gamepad: Map the input types to Linux event codes Attachment 351820 [details] pushed as 03b5f73 - gamepad: Drop usage of StandardGamepadButton in Vala Attachment 351821 [details] pushed as 30104a2 - gamepad: Drop usage of StandardGamepadAxis in Vala Attachment 351822 [details] pushed as 3ef5bcc - gamepad: Remove custom types Attachment 351823 [details] pushed as 804fa2a - gamepad: Rename GamepadMappedEvent into GamepadInput Attachment 351824 [details] pushed as fea45fa - gamepad: Merge GamepadInput and input_t Attachment 351825 [details] pushed as 8553ebc - gamepad: Simplify mapping destination parsing Attachment 351826 [details] pushed as 29d41cf - gamepad: Make names clear in GamepadMapping Attachment 351827 [details] pushed as d942798 - gamepad: Make gamepad emit Event objects
Closed by accident: reopening. The following points haven't been fixed: - the gamepad's GUID can't be accessed from the Gamepad class - the mapping of a gamepad can't be directly edited - the distinction between Gamepad and RawGamepad is somewhat artificial
Created attachment 351870 [details] [review] event: Allow to keep hardware info Turn the 'index' field into 'hardware_index' and add fields containing the code of the input related to the event. Also remove the barely useful 'send_event' field. This avoid loosing information related to the original hardware event, whether the event is mapped or not.
Created attachment 351875 [details] [review] event: Allow to keep hardware info Turn the 'index' field into 'hardware_index' and add fields containing the code of the input related to the event. Also remove the barely useful 'send_event' field. This avoid loosing information related to the original hardware event, whether the event is mapped or not.
Created attachment 352516 [details] [review] event: Allow to keep hardware info Turn the 'index' field into 'hardware_index' and add fields containing the code of the input related to the event. Also remove the barely useful 'send_event' field. This avoid loosing information related to the original hardware event, whether the event is mapped or not.
Created attachment 352522 [details] [review] gamepad: Use GamepadInput in GamepadDPad This makes the code simpler.
Attachment 352516 [details] pushed as 488180f - event: Allow to keep hardware info Attachment 352522 [details] pushed as 2dc64b7 - gamepad: Use GamepadInput in GamepadDPad