Skip to content

Replace MouseScrollUnit::SCROLL_UNIT_CONVERSION_FACTOR with a Resource#24512

Open
stevehello166 wants to merge 15 commits into
bevyengine:mainfrom
stevehello166:fix_MouseScrollUnit
Open

Replace MouseScrollUnit::SCROLL_UNIT_CONVERSION_FACTOR with a Resource#24512
stevehello166 wants to merge 15 commits into
bevyengine:mainfrom
stevehello166:fix_MouseScrollUnit

Conversation

@stevehello166
Copy link
Copy Markdown
Contributor

Objective

Fixes #24508

Solution

Replace MouseScrollUnit with a Resource of the same name and remove it from the structs and events related to scrolling. More work still needs to be done to make sure that the value in MouseScrollUnit is actually respected

Testing

  • Running standard test suite/ci as well as related examples to make sure that they haven't broken

@alice-i-cecile
Copy link
Copy Markdown
Member

@MyCodingSpace5, take a look at this PR and how it evolves :) It should make clear what I meant by a resource here, and how work is done in open source.

@alice-i-cecile alice-i-cecile added A-Input Player input via keyboard, mouse, gamepad, and more C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jun 2, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Input Jun 2, 2026
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible X-Uncontroversial This work is generally agreed upon S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jun 2, 2026
Comment thread crates/bevy_input/src/mouse.rs
Comment thread crates/bevy_input/src/lib.rs Outdated
.add_message::<MouseWheel>()
.init_resource::<AccumulatedMouseMotion>()
.init_resource::<AccumulatedMouseScroll>()
.insert_resource(MouseScrollUnit::Pixel)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the idea is to insert a conversion factor as a resource, one that users can look up instead of the constant SCROLL_UNIT_CONVERSION_FACTOR.

It can start out equal to that constant; adapting it to the platform is for later work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright, I reverted the previous changes and added MouseScrollPixelsPerLine resource. I'm not entirely sure about the name of the resource given it's length. However, it is more descriptive than SCROLL_UNIT_CONVERSION_FACTOR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's a totally fine name: I don't think this will come up often.

@stevehello166 stevehello166 force-pushed the fix_MouseScrollUnit branch from d9618e6 to cd0964b Compare June 2, 2026 22:39
@stevehello166 stevehello166 marked this pull request as ready for review June 3, 2026 00:13
@stevehello166 stevehello166 changed the title Replace MouseScrollUnit with a Resource Replace MouseScrollUnit::SCROLL_UNIT_CONVERSION_FACTOR with a Resource Jun 3, 2026
@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 3, 2026
Comment thread crates/bevy_input/src/mouse.rs Outdated
Comment thread crates/bevy_camera_controller/src/pan_camera.rs Outdated
Comment thread crates/bevy_camera_controller/src/pan_camera.rs Outdated
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.

Looking good, but I think we can add a couple helper methods and make this much nicer :)

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 3, 2026
Copy link
Copy Markdown
Contributor

@amtep amtep left a comment

Choose a reason for hiding this comment

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

Some confusion around to_lines and to_pixels, otherwise looks good :)

Comment thread crates/bevy_input/src/mouse.rs Outdated
Comment thread crates/bevy_input/src/mouse.rs Outdated
Comment thread crates/bevy_input/src/mouse.rs Outdated
Comment thread crates/bevy_camera_controller/src/free_camera.rs Outdated
Comment thread crates/bevy_camera_controller/src/pan_camera.rs Outdated
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 C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Uncontroversial This work is generally agreed upon

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

MouseScrollUnit handling is unprincipled and inconsistent

3 participants