Skip to content

Conversation

@blunted2night
Copy link
Contributor

This patch alters the Window::set_cursor_position API to take a Vec2 in the same coordinate system as the CursorMoved event.

The change to floating point allows more precise positioning in high DPI contexts. The coordinate system change and use of Vec2 makes things consistent with the CursorMoved event.

bevy_window::WindowCommand::SetCursorPosition { x, y } => {
bevy_window::WindowCommand::SetCursorPosition { position } => {
let window = winit_windows.get_window(id).unwrap();
let inner_size = window.inner_size().to_logical::<f32>(window.scale_factor());
Copy link
Contributor

@ambeeeeee ambeeeeee Dec 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be able to get rid of the type hint here since you have subtraction below which assigns to an f32, which should hint the compiler enough to figure it out, I haven't tested that but it feels like it would be the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried but the compiler can't figure it out.

Copy link
Member

@cart cart Dec 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also very cool with explicit types in cases like this, even if they can be inferred. Helps convey intent / protects against implict behavior changes when types change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am disappointed in the compiler ):

Nathan Jeffords added 2 commits December 3, 2020 11:55
@cart
Copy link
Member

cart commented Dec 3, 2020

Looks good to me! Resolve the conflict and I think we can merge this.

@cart cart merged commit 1aff709 into bevyengine:master Dec 3, 2020
@blunted2night blunted2night deleted the set-cursor-updates branch December 3, 2020 20:39
@fopsdev fopsdev mentioned this pull request Jan 24, 2021
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.

3 participants