[Merged by Bors] - Fix set_cursor_grab_mode to try an alternative mode before giving an error #6599
Conversation
IceSentry
added
A-Windowing
|
Yeah we should rename this to reflect the bool nature |
|
Ugh, this is technically a breaking change :( I'm probably fine violating semver here: the provided API basically doesn't work. |
alice-i-cecile
added
C-Bug
|
One way to keep semver would be to keep the api and add a new one with bools. Maybe even tag the broken one as deprecated, but I'm not sure if deprecation warning break semver. |
|
this was |
|
Yes, but I decided to keep the grab terminology to stay closer to winit. Would you say it's better to keep it named |
|
Yeah I like the idea of keeping the old API with a deprecation so we can put this in the point release |
|
I reintroduced the api, but I modified the implementation a bit to avoid the error. This means that we technically don't need the boolean api, and this gives a tiny bit more control to the user. I left both in the PR for now, but I'm not sure what to do here. |
IceSentry
force-pushed
the
grab_mode_bool
branch
2 times, most recently
from
8ea185e
to
bab52f2
Compare
IceSentry
force-pushed
the
grab_mode_bool
branch
from
bab52f2
to
bcf9e45
Compare
|
Updated to only use the fixed api and don't introduce a new api or remove anything. |
|
I'm very happy with this now. Can you update your PR description? |
IceSentry
changed the title
|
I updated the title and changed the docs a little bit to reflect that it's possible that the getter will return a value that is different from the one sent to winit. |
There was a problem hiding this comment.
Fwiw, this works on Windows and the changes look good to me.
Note that I had to remove the commit with the breaking change from main that's in this branch to test this with my project. I assume there's plan to avoid including that commit in the 0.9.1 release but just a heads up.
|
Which commit is that? This PR shouldn't have any breaking commit. |
|
Oh, is the issue that I made this PR from main? If that's the case then yeah the 0.9.1 release will only cherry pick this PR. |
alice-i-cecile
added
the
S-Ready-For-Final-Review
|
bors r+ |
…error (#6599) # Objective - Closes #6590 - The grab mode is platform dependent, this is problematic for bevy users since we can't easily use the recommended way to detect if the feature works like the winit docs recommend https://docs.rs/winit/0.27.5/winit/window/struct.Window.html#method.set_cursor_grab ## Solution Try to use the grab mode that was requested, if it fails use the other one. Only then log an error if it fails after this step.
bors
bot
changed the title
…error (#6599) # Objective - Closes #6590 - The grab mode is platform dependent, this is problematic for bevy users since we can't easily use the recommended way to detect if the feature works like the winit docs recommend https://docs.rs/winit/0.27.5/winit/window/struct.Window.html#method.set_cursor_grab ## Solution Try to use the grab mode that was requested, if it fails use the other one. Only then log an error if it fails after this step.
…error (bevyengine#6599) # Objective - Closes bevyengine#6590 - The grab mode is platform dependent, this is problematic for bevy users since we can't easily use the recommended way to detect if the feature works like the winit docs recommend https://docs.rs/winit/0.27.5/winit/window/struct.Window.html#method.set_cursor_grab ## Solution Try to use the grab mode that was requested, if it fails use the other one. Only then log an error if it fails after this step.
Objective
Solution
Try to use the grab mode that was requested, if it fails use the other one. Only then log an error if it fails after this step.