Skip to content

Refactor and bug fixes

Vanadiae requested to merge Vanadiae/libmanette:refactor_and_bug_fixes into master

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 than g_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, use g_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

Merge request reports