raylib: simpler callbacks #35488
raylib: simpler callbacks #35488
Conversation
|
@deanlee what do you think? |
|
No, there's a reason I wrote it this way. This is a temporary workaround. Without this handling, when clicking the sidebar's settings button, the Settings view opens—but immediately detects the same mouse click as a press on the "Close" button and closes itself right away. |
|
I'm aware, but we already have this problem with the sidebar. If you click onroad where the settings button will be, when the sidebar pops open it brings you to settings. This PR simplifies something confusing, we'll have to fix this touch handling bug eventually |
|
Btw #35489 fixes the settings / close button issue. It only handles cases cross-widgets, we need something nice and simple to handle intra-widget interactions, such as this one: Peek.2025-06-06.22-22.mp4 |
| self._sidebar.set_callbacks(on_settings=self._on_settings_clicked, | ||
| on_flag=self._on_flag_clicked) | ||
| self._layouts[MainState.SETTINGS].set_callbacks(on_close=self._set_mode_for_state) | ||
| self._layouts[MainState.ONROAD].on_click = self._on_onrad_clicked |
There was a problem hiding this comment.
@deanlee let's decide on if we should use set functions or not soon, this is two different patterns for no reason
There was a problem hiding this comment.
think using a set method for is clearer than direct assignment
There was a problem hiding this comment.
Some modules, like ui_state with offroad transition, may need multiple callbacks. Using a consistent set_ / add_ pattern makes it easier to manage them than assigning callbacks directly.
It seems simpler to me if the children widgets call the callback functions directly rather than telling the parent to run them via dynamic setattr.
This exposes some more touch/mouse press detection issues. Now mouse up when clicking settings will be handled by sidebar and settings, opening and closing it immediately.
We should handle this by ensuring the widget also gets the down event, not just released (a bug with other widgets too)