Skip to content

Conversation

@marstaik
Copy link

Hi, while using migrating from winit to SDL3, I noticed that some of the events strangely did not have all of the correct fields and names (In my particular case, I have multiple windows and many of the events were missing window_id).

Seeing as some of these events haven't been updates in years from their source, I went ahead and modernized the events a bit. Thankfully, as you can see in the examples, their replacement/updated usage is pretty easy to follow.

Key changes:

  • Group related raw SDL events under category enums (Window, Keyboard, Mouse, Joystick, Controller, etc.).
  • Preserve ability to convert to/from raw SDL_Event (where feasible).
  • Keep the name Event so external code continues to reference crate::event::Event.
  • Maintain custom/user event registration and pushing.
  • Provide helper methods analogous to the old API (get_timestamp, get_window_id, is_* classifiers).

@marstaik marstaik changed the title Modernize SDL3 Events sdl3::events Refactor - Modernize/Update SDL3 Events Nov 14, 2025
@revmischa
Copy link
Member

Thank you! Happy to look/merge when CI is passing 😄

@marstaik
Copy link
Author

Yep, working on it, I missed some of them because of the feature flags

@marstaik marstaik changed the title sdl3::events Refactor - Modernize/Update SDL3 Events refactor: sdl3::events - Modernize/Update SDL3 Events Nov 15, 2025
@marstaik
Copy link
Author

I am considering adding a lifetime to events so that we can avoid cloning into Strings. If we add a lifetime bound to the event pump, since SDL handles backing memory we should be gracefully be able to just have CStr, and let the user decide when to allocate. I think its a better solution than just copying payloads.

Since a vast majority of the events don't have string references, the inner types should be trivially copyable anyways if you really need a copy.

This would facilitate better to_ll/from_ll behavior, but I'm wondering if we really need to_ll still...

@revmischa
Copy link
Member

Okay, please let me know when this PR is ready to merge. Do you want to add lifetimes here or later?

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