Skip to content

Create chainable run_if_dyn#18585

Open
hukasu wants to merge 1 commit intobevyengine:mainfrom
hukasu:run-if-dyn
Open

Create chainable run_if_dyn#18585
hukasu wants to merge 1 commit intobevyengine:mainfrom
hukasu:run-if-dyn

Conversation

@hukasu
Copy link
Contributor

@hukasu hukasu commented Mar 28, 2025

Objective

Most methods from IntoScheduleConfigs are chainable, bun not run_if_dyn

Solution

Make run_if_dyn chainable

Testing

cargo run -p ci

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events labels Mar 28, 2025
alice-i-cecile
alice-i-cecile reviewed Mar 28, 2025
View reviewed changes
crates/bevy_ecs/src/schedule/config.rs
/// This is useful if you have a run condition whose concrete type is unknown.
/// Prefer `run_if` for run conditions whose type is known at compile time.
pub fn run_if_dyn(&mut self, condition: BoxedCondition) {
pub fn run_if_dyn_inner(&mut self, condition: BoxedCondition) {
Copy link
Member

@alice-i-cecile alice-i-cecile Mar 28, 2025

Choose a reason for hiding this comment

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

Should this be pub?

Copy link
Contributor

@urben1680 urben1680 Mar 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't use this but in_set_inner and I prefer more than less of these methods being public because they enable more complex configuration building where you don't want to play tennis with reassigning owned values.

alice-i-cecile
alice-i-cecile reviewed Mar 28, 2025
View reviewed changes
crates/bevy_ecs/src/schedule/config.rs
/// This is useful if you have a run condition whose concrete type is unknown.
/// Prefer `run_if` for run conditions whose type is known at compile time.
pub fn run_if_dyn(&mut self, condition: BoxedCondition) {
pub fn run_if_dyn_inner(&mut self, condition: BoxedCondition) {
Copy link
Member

@alice-i-cecile alice-i-cecile Mar 28, 2025

Choose a reason for hiding this comment

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

I don't understand why this needs to exist. Can't you just discard the unused ScheduleConfigs?

@hukasu
Copy link
Contributor Author

hukasu commented Mar 28, 2025

don't know if this is really a Bug, more Usability

@alice-i-cecile alice-i-cecile 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-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 and removed C-Bug An unexpected or incorrect behavior labels Mar 28, 2025
@ItsDoot ItsDoot mentioned this pull request Nov 30, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile left review comments

+1 more reviewer

@urben1680 urben1680 urben1680 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-ECS Entities, components, systems, and events 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-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

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hukasu @alice-i-cecile @urben1680