Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sokol_nuklear.h: change snk_handle_event to return a bool if handled #959

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

adamrt
Copy link
Contributor

@adamrt adamrt commented Dec 21, 2023

This method was recommended by @vurtun: vurtun/nuklear#189 (comment)

This is an older comment but the functionality works as expected.

This method was recommended by @vurtun:
vurtun/nuklear#189 (comment)

This is an older comment but the functionality works as expected.
@adamrt
Copy link
Contributor Author

adamrt commented Dec 21, 2023

Related: Why does the current code store the state in new frame, but then apply it in event handling. Why not just do it all in the event handling instead of carrying the state through.

@adamrt
Copy link
Contributor Author

adamrt commented Dec 30, 2023

If there is anything you would like adjusted or changed, please let me know @floooh

@floooh
Copy link
Owner

floooh commented Jan 3, 2024

Starting to look into the PR later today, apologies for the delay.

@adamrt
Copy link
Contributor Author

adamrt commented Jan 3, 2024

No apology necessary. Thanks for taking the time.

@floooh
Copy link
Owner

floooh commented Jan 3, 2024

...please ignore the failed Emscripten CI run, unrelated to the PR and already fixed in master.

@floooh
Copy link
Owner

floooh commented Jan 3, 2024

Related: Why does the current code store the state in new frame, but then apply it in event handling. Why not just do it all in the event handling instead of carrying the state through.

IFIR the sokol_nuklear.h header was mostly copied over from sokol_imgui.h by the original author (sokol_imgui.h has diverged in the meantime because the Dear ImGui input model has changed to input events -> to fix exactly the problems which made this per-frame code necessary in the first place).

Specifically for mouse buttons, it was necessary to delay a button-up into the next frame, otherwise on touchpads it would frequently happen that at button-down happened in the same frame as button up, which caused at least Dear ImGui to miss the event (and I think Nuklear suffered from the same problem).

@floooh floooh merged commit 1df37b1 into floooh:master Jan 3, 2024
22 of 23 checks passed
@floooh
Copy link
Owner

floooh commented Jan 3, 2024

...and merged. Thanks!

I also added a small blurb to the changelog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants