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.

Sign up for GitHub

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

Jump to bottom

Unify picking backends #17348

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Unify picking backends #17348

wants to merge 7 commits into from

Conversation

chompaa
Copy link
Member

@chompaa chompaa commented Jan 13, 2025

Objective

Currently, our picking backends are inconsistent:

  • Mesh picking and sprite picking both have configurable opt in/out behavior. UI picking does not.
    • Sprite picking uses SpritePickingCamera and Pickable for control, but mesh picking uses RayCastPickable.
  • MeshPickingPlugin is not a part of DefaultPlugins. SpritePickingPlugin and UiPickingPlugin are.

Solution

  • Add configurable opt in/out behavior to UI picking (defaults to opt out).
  • Replace RayCastPickable with MeshPickingCamera and Pickable.
  • Remove SpritePickingPlugin and UiPickingPlugin from DefaultPlugins.

Testing

Ran some examples.

Migration Guide

UiPickingPlugin and SpritePickingPlugin are no longer included in DefaultPlugins. They must be explicitly added.

RayCastPickable has been replaced in favor of the MeshPickingCamera and Pickable components. You should add them to cameras and entities, respectively, if you have MeshPickingSettings::require_markers set to true.

@chompaa chompaa added X-Contentious There are nontrivial implications that should be thought through D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-Picking Pointing at and selecting objects of all sorts labels Jan 13, 2025
@alice-i-cecile alice-i-cecile added M-Needs-Release-Note Work that should be called out in the blog due to impact M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jan 13, 2025
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aevyrie aevyrie Awaiting requested review from aevyrie

@NthTensor NthTensor Awaiting requested review from NthTensor

Assignees
No one assigned
Labels
A-Picking Pointing at and selecting objects of all sorts D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide M-Needs-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Milestone
0.16
Development

Successfully merging this pull request may close these issues.
2 participants
@chompaa @alice-i-cecile