Skip to content

[Merged by Bors] - Update MouseMotion and CursorMoved docs#5090

Closed
Nilirad wants to merge 3 commits intobevyengine:mainfrom
Nilirad:mouse_cursor_event_docs
Closed

[Merged by Bors] - Update MouseMotion and CursorMoved docs#5090
Nilirad wants to merge 3 commits intobevyengine:mainfrom
Nilirad:mouse_cursor_event_docs

Conversation

@Nilirad
Copy link
Copy Markdown
Contributor

@Nilirad Nilirad commented Jun 24, 2022

Objective

Solution

I looked at the implementation of those events. I noticed that they both are adaptations of winit's DeviceEvent/WindowEvent enum variants. Therefore I based the description of the items on the documentation provided by the upstream crate. I also added a link to CursorMoved, just like MouseMotion already has.

Observations

  • Looking at the implementation of MouseMotion, I noticed the DeviceId field of the winit event is discarded by bevy_input. This means that in the case a machine has multiple pointing devices, it is impossible to distinguish to which one the event is referring to. EDIT: just tested, MouseMotion events are emitted for movement of both mice.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-Input Player input via keyboard, mouse, gamepad, and more A-Windowing Platform-agnostic interface layer to run your app in labels Jun 25, 2022
Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

These are a solid improvement. I would like to see:

  1. Links between these types, with an explanation of the differences.
  2. A note in the docs recording your observation about MouseMotion and multiple pointing devices.

@Nilirad
Copy link
Copy Markdown
Contributor Author

Nilirad commented Jun 25, 2022

I only added a link to MouseMotion because a MouseMotionCursorMoved link would violate the dependency principle (bevy_input does not depend on bevy_winit, it's the other way around).

Copy link
Copy Markdown
Contributor

@Weibye Weibye left a comment

Choose a reason for hiding this comment

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

I think this looks good as is, see comments but those should not be blocking

@Weibye Weibye added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jun 26, 2022
@alice-i-cecile
Copy link
Copy Markdown
Member

bors r+

bors bot pushed a commit that referenced this pull request Jun 26, 2022
# Objective

- Fixes #5083 

## Solution

I looked at the implementation of those events. I noticed that they both are adaptations of `winit`'s `DeviceEvent`/`WindowEvent` enum variants. Therefore I based the description of the items on the documentation provided by the upstream crate. I also added a link to `CursorMoved`, just like `MouseMotion` already has.

## Observations

- Looking at the implementation of `MouseMotion`, I noticed the `DeviceId` field of the `winit` event is discarded by `bevy_input`. This means that in the case a machine has multiple pointing devices, it is impossible to distinguish to which one the event is referring to. **EDIT:** just tested, `MouseMotion` events are emitted for movement of both mice.
@bors bors bot changed the title Update MouseMotion and CursorMoved docs [Merged by Bors] - Update MouseMotion and CursorMoved docs Jun 26, 2022
@bors bors bot closed this Jun 26, 2022
@Nilirad Nilirad deleted the mouse_cursor_event_docs branch June 26, 2022 14:44
inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
# Objective

- Fixes bevyengine#5083 

## Solution

I looked at the implementation of those events. I noticed that they both are adaptations of `winit`'s `DeviceEvent`/`WindowEvent` enum variants. Therefore I based the description of the items on the documentation provided by the upstream crate. I also added a link to `CursorMoved`, just like `MouseMotion` already has.

## Observations

- Looking at the implementation of `MouseMotion`, I noticed the `DeviceId` field of the `winit` event is discarded by `bevy_input`. This means that in the case a machine has multiple pointing devices, it is impossible to distinguish to which one the event is referring to. **EDIT:** just tested, `MouseMotion` events are emitted for movement of both mice.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

- Fixes bevyengine#5083 

## Solution

I looked at the implementation of those events. I noticed that they both are adaptations of `winit`'s `DeviceEvent`/`WindowEvent` enum variants. Therefore I based the description of the items on the documentation provided by the upstream crate. I also added a link to `CursorMoved`, just like `MouseMotion` already has.

## Observations

- Looking at the implementation of `MouseMotion`, I noticed the `DeviceId` field of the `winit` event is discarded by `bevy_input`. This means that in the case a machine has multiple pointing devices, it is impossible to distinguish to which one the event is referring to. **EDIT:** just tested, `MouseMotion` events are emitted for movement of both mice.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Fixes bevyengine#5083 

## Solution

I looked at the implementation of those events. I noticed that they both are adaptations of `winit`'s `DeviceEvent`/`WindowEvent` enum variants. Therefore I based the description of the items on the documentation provided by the upstream crate. I also added a link to `CursorMoved`, just like `MouseMotion` already has.

## Observations

- Looking at the implementation of `MouseMotion`, I noticed the `DeviceId` field of the `winit` event is discarded by `bevy_input`. This means that in the case a machine has multiple pointing devices, it is impossible to distinguish to which one the event is referring to. **EDIT:** just tested, `MouseMotion` events are emitted for movement of both mice.
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-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Distinction between MouseMotion and CursorMoved events is unclear in docs

3 participants