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.

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

Rename PickingBehavior to Pickable #17266

Merged
merged 1 commit into from
Jan 12, 2025

Conversation

chompaa
Copy link
Member

@chompaa chompaa commented Jan 9, 2025

Objective

PR #17225 allowed for sprite picking to be opt-in. After some discussion, it was agreed that PickingBehavior should be used to opt-in to sprite picking behavior for entities. This leads to PickingBehavior having two purposes: mark an entity for use in a backend, and describe how it should be picked. Discussion led to the name Pickablemaking more sense (also: this is what the component was named before upstreaming).

A follow-up pass will be made after this PR to unify backends.

Solution

Replace all instances of PickingBehavior and picking_behavior with Pickable and pickable, respectively.

Testing

CI

Migration Guide

Change all instances of PickingBehavior to Pickable.

@chompaa chompaa added C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-Picking Pointing at and selecting objects of all sorts labels Jan 9, 2025
@alice-i-cecile alice-i-cecile added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Jan 9, 2025
Copy link
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.

I agree with this. Once the backends are unified in behavior, we should add docs to explain that Pickable is always required.

@alice-i-cecile alice-i-cecile added the X-Contentious There are nontrivial implications that should be thought through label Jan 9, 2025
@BenjaminBrienen BenjaminBrienen added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 11, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 12, 2025
Merged via the queue into bevyengine:main with commit 02bb151 Jan 12, 2025
36 checks passed
@aevyrie
Copy link
Member

aevyrie commented Jan 13, 2025

I agree with this. Once the backends are unified in behavior, we should add docs to explain that Pickable is always required

@alice-i-cecile is that correct? I thought we wanted both opt in and opt out? E.g. in opt-out, Pickable is not required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Picking Pointing at and selecting objects of all sorts C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants