Refactor and bug fixes
Don't bother yet to review, this is WIP. Not much will be pushed until everything is done, to group same changes from different files in only one commit.
Things to check (minimum, if bad things come out then I'll fix them too):
-
use g_signal_emit_by_name
rather thang_signal_emit
as it's clearer - use extensively g_auto/g_autoptr/g_autofree (while making sure those are initialized to NULL)
- give good names to callbacks (
on_*_cb
basically) -
use directly the correct type for user_data (callbacks) rather than gpointer then casting to the type - stop using
g_return_(val_)if_fail()
for static functions (when there's args to check, useg_assert*()
.) -
use full name for class vfuncs (finalize, etc.) => namespace_class_*
Files:
-
manette-device.c -
manette-event.c -
manette-event-mapping.c -
manette-mapping.c -
manette-mapping-error.c -
manette-mapping-manager.c -
manette-monitor.c -
manette-monitor-iter.c -
libmanette.h -
manette-device.h -
manette-device-private.h -
manette-event.h -
manette-event-mapping-private.h -
manette-event-private.h -
manette-mapping-error-private.h -
manette-mapping-manager-private.h -
manette-mapping-private.h -
manette-monitor.h -
manette-monitor-iter.h -
manette-monitor-iter-private.h -
manette-version.h.in -
demos/manette-test/manette-test.c -
tests/test-event-mapping.c -
tests/test-mapping.c -
tests/test-mapping-manager.c
Things I'd like feedback on
/cc @aplazas @exalm
- does
G_UNLIKELY (error==NULL)
actually save that much vs readibility ? - is the thing with g_return_if_fail in static func @exalm mentionned only about type checking ? (
g_return_if_fail (MANETTE_IS_MAPPING_MANAGER (self));
) ? - it doesn't make much sense to expose the "hat" in ManetteEvent, as it is only relevant for the mappings part (which is private anyway), as the SDL mappings threat absolute axis and hat axis separately, even for linux where they are all part of ABS_*. Remove this "hat" distinction ?
- I feel like ManetteEvent's
hardware_*
(type/code/value/index) is quite unclear on what it is actually. Needs either (or both) renaming or better documentation. I think the type is Evdev's event type (EV_*
), code is evdev's event code (ABS_*
,BTN_*
,KEY_*
, whatever), the rest I'm unsure. - going with the two previous elements, I feel like it isn't very pertinent to expose evdev's internal and probably the unmapped/uncalibrated values too. I see only two use cases. The Games mapping UI, but even there libmanette could be providing something like a ManetteSDLMappingStringBuilder thing where you'd give the axis/button you want to map to (e.g. Dpad-up, right-stick X axis) and the ManetteEvent you consider valid (like when there's a button press/release, or if an axis moved more than 0.2 from the center 0.0 (so you're sure the user actually moved the stick, not just jitter/deadzone)) and this ManetteSDLMappingStringBuilder would handle all the underlying mapping string complexity for the event and axis/button you give him. There's also probably calibration but this would anyway need an API in libmanette since we don't expose the evdev's device file descriptor (which would be needed to do that out of libmanette). Even if we really want to expose the unmapped event type+code and uncalibrated/normalized/deadzoned value, it could be done with an extra signal that would pass those.
- As such, libmanette API reflects too much the underlying evdev API and how SDL's mapping string works rather than exposing a correct gamepad event API, IMO.
Fixes #13 (closed)
Edited by Vanadiae