Button debounce/update rate

[Daft-Freak]

Think the buttons may need some debounce or something. Sometimes you get a lot of double-presses. Possibly related to the fact that if the game isn't doing much blit_process_input gets called thousands of times a second. (Then again, so does blit_update_led/vibration)

[Gadgetoid]

Yeah the debouncing is pretty horrible at the moment. I was broadly happy with the fused/debounced joystick/D-Pad input I managed to hash together for the firmware games list, but it needs finessing and merged into the system.

I guess in most cases it doesn't make sense for the input frequency to be any faster than the frame rate, since you can't perform an action "blind". Then becomes a case of feeding update() with the right state/transition information.

[Daft-Freak]

D-Pad/Joystick fusion would be a separate thing above the firmware though? (SDL also has D-Pads and joysticks...)

I think this was okay before I added the fancy buttons.pressed/.released, since it was impossible to see state changes in between calls to update. Possibly just track a last_state and calculate those right before calling update, which is roughly what SDL is doing with it's 20ms loop.

[Daft-Freak]

Basically, this:

diff --git a/32blit-sdl/System.cpp b/32blit-sdl/System.cpp
index fad10d8a..b281f49c 100644
--- a/32blit-sdl/System.cpp
+++ b/32blit-sdl/System.cpp
@@ -238,7 +238,7 @@ int System::update_thread() {
 void System::loop()
 {
    SDL_LockMutex(m_input);
-   blit::buttons = shadow_buttons;
+   blit::buttons.state = shadow_buttons;
    blit::tilt.x = shadow_tilt[0];
    blit::tilt.y = shadow_tilt[1];
    blit::tilt.z = shadow_tilt[2];
diff --git a/32blit-stm32/Src/32blit.cpp b/32blit-stm32/Src/32blit.cpp
index 43efc810..67d365bf 100644
--- a/32blit-stm32/Src/32blit.cpp
+++ b/32blit-stm32/Src/32blit.cpp
@@ -846,7 +846,7 @@ void blit_process_input() {
   bool joystick_button = false;

   // Read buttons
-  blit::buttons =
+  blit::buttons.state =
     (!HAL_GPIO_ReadPin(DPAD_UP_GPIO_Port,     DPAD_UP_Pin)      ? blit::DPAD_UP    : 0) |
     (!HAL_GPIO_ReadPin(DPAD_DOWN_GPIO_Port,   DPAD_DOWN_Pin)    ? blit::DPAD_DOWN  : 0) |
     (!HAL_GPIO_ReadPin(DPAD_LEFT_GPIO_Port,   DPAD_LEFT_Pin)    ? blit::DPAD_LEFT  : 0) |
diff --git a/32blit/engine/engine.cpp b/32blit/engine/engine.cpp
index d4173767..8883f735 100644
--- a/32blit/engine/engine.cpp
+++ b/32blit/engine/engine.cpp
@@ -77,10 +77,15 @@ namespace blit {
     // catch up on updates if any pending
     pending_update_time += (time - last_tick_time);
     while (pending_update_time >= update_rate_ms) {
+      // button state changes
+      uint32_t changed = api.buttons.state ^ api.buttons.last_state;
+
+      api.buttons.pressed = changed & api.buttons.state;
+      api.buttons.released = changed & api.buttons.last_state;
+      api.buttons.last_state = api.buttons.state;
+
       update(time - pending_update_time); // create fake timestamp that would have been accurate for the update event
       pending_update_time -= update_rate_ms;
-
-      api.buttons.pressed = api.buttons.released = 0;
     }

     last_tick_time = time;
diff --git a/32blit/engine/input.hpp b/32blit/engine/input.hpp
index b82dae5d..f64a7118 100644
--- a/32blit/engine/input.hpp
+++ b/32blit/engine/input.hpp
@@ -20,22 +20,12 @@ namespace blit {
   };

   struct ButtonState {
-    ButtonState &operator=(uint32_t v) {
-      uint32_t changed = state ^ v;
-
-      pressed |= changed & v;
-      released |= changed & state;
-
-      state = v;
-
-      return *this;
-    }

     operator uint32_t() const {
       return state;
     }

-    uint32_t state;
+    uint32_t state, last_state = 0;
     uint32_t pressed, released; // state change since last update
   };

(Technically an API break)

[Gadgetoid]

You're right re joystick/d-pad. Probably shouldn't be trying to do anything SDL doesn't offer!

Would be interesting to see how your patch affects the launcher menu if I pull out all my long-winded button handling gumph. I really felt the button skipping there, which is why I wrangled all that nonsense in anger.

[ahnlak]

Upvote for button debounce :)

[Daft-Freak]

Slightly smaller version of that patch https://github.com/Daft-Freak/32blit-beta/commit/2e5a3f462f909036b8192ed388fcfe74ee307d43

(May break less)

[Gadgetoid]

I think this is what's making Rainbow Ascent an absolute nightmare to play- aside from it being intentionally an absolute nightmare to play of course.

I should probably add some kind of difficulty setting though :laughing:

[Daft-Freak]

Kind-of related: the HOME button debounce/hold doesn't seem to be working right, sometimes the menu will immediately close and sometimes pressing the button repeatedly will be handled as holding it...

[Gadgetoid]

Ooof, I'm glad you said the latter. I've had some rare instances where I've been bailed out of a game when opening the menu and wondered what the heck just happened.

I wouldn't be surprised if there's something wrong in here: https://github.com/pimoroni/32blit-beta/blob/ee0fe08b246ae5a977831183f1db14ea71c8e997/32blit-stm32/Src/32blit.cpp#L523-L549

[mslinklater]

Apologies to resurrect this but being hit by the lack of pressed/released edge triggers on desktop. Will take a look at this and see if I can't do something useful... my preference would be for engine side to accumulate presses and releases in the input update and then for the game-side update to call a method which calculates the pressed/released once per update before polling for those values. Or some other way of decoupling the update from edge detect logic. I'll have a play.

[Daft-Freak]

blit::buttons.pressed/released? That's the mask of buttons pressed/released since the last update.

(I think the bug here was mostly fixed by #521)

[mslinklater]

Those are the values I'm polling and they're not coming through at all on my laptop (Linux). State is fine... edge detect is just zero all the time.

[Daft-Freak]

Hmm odd, that should work everywhere... (It's calculated in the common code)