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

Replace converters functions in favor of the From trait #8660

Closed
wants to merge 2 commits into from
Closed

Replace converters functions in favor of the From trait #8660

wants to merge 2 commits into from

Conversation

hate
Copy link
Contributor

@hate hate commented May 23, 2023

Objective

  • The bevy_winit/converters.rs file contains many converter functions who's functionality could be achieved by implementing the From trait on the respective enums/structs.
  • With this change, we can just call the .into() method for the aforementioned enums/structs.
  • I made this PR to see if this change would be feasible, if the converter functions are preferred over the From implementations, then this PR should be closed.

Solution

Added the following trait implementations:

  • impl From<winit::event::KeyboardInput> for KeyboardInput
  • impl From<winit::event::VirtualKeyCode> for KeyCode
  • impl From<winit::event::ElementState> for ButtonState
  • impl From<winit::event::MouseButton> for MouseButton
  • impl From<CursorIcon> for winit::window::CursorIcon
  • impl From<WindowLevel> for winit::window::WindowLevel

To support this change, the winit crate must be added as a dependency in bevy_input and bevy_window crates.

@alice-i-cecile alice-i-cecile added A-Input Player input via keyboard, mouse, gamepad, and more A-Windowing Platform-agnostic interface layer to run your app in C-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use labels May 23, 2023
@alice-i-cecile
Copy link
Member

I like this better, but I remember trying this before and @mockersf complaining at me for some reason ;)

The added dependency is a bit annoying: in theory bevy_input could be agnostic to the windowing backend. It compiles though, so no immediate circular dependency. We could also feature flag this, which I think is fairly sensible.

@hate
Copy link
Contributor Author

hate commented May 24, 2023

Right, I wasn't too sure how to feel about the added dependency either, I just figured to go through with the implementation, since it would have to get added either way if the From trait were to be used instead of a converter function here: #8593 (comment)

@hate hate closed this by deleting the head repository May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Input Player input via keyboard, mouse, gamepad, and more A-Windowing Platform-agnostic interface layer to run your app in C-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants