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

Move run_if to IntoSystem #14576

Open
DasLixou opened this issue Aug 1, 2024 · 7 comments
Open

Move run_if to IntoSystem #14576

DasLixou opened this issue Aug 1, 2024 · 7 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@DasLixou
Copy link
Contributor

DasLixou commented Aug 1, 2024

What problem does this solve or what need does it fill?

I am using bevy_eventlistener to listen for events and want to have a run condition on the runned system.
However, run_if is only implemented for IntoSystemConfigs, not IntoSystem, where only pipe and map exist, which both don't really help me.

What solution would you like?

Move / implement run_if for IntoSystem

What alternative(s) have you considered?

Hand-wiring the two systems together.

@DasLixou DasLixou added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Aug 1, 2024
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Aug 1, 2024
@alice-i-cecile
Copy link
Member

Run conditions still need to work on system sets. If anyone tries their hand at this change, that's the critical requirement that I don't think will be solved by simply moving the method.

@alice-i-cecile alice-i-cecile added the D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes label Aug 1, 2024
@DasLixou
Copy link
Contributor Author

DasLixou commented Aug 1, 2024

Rust compiler would nag when both IntoSystemConfigs and IntoSystem had a method called run_if, right? :c

@alice-i-cecile
Copy link
Member

Yeah, I think it would likely end up ambiguous, which is no fun at all and would seriously degrade UX. By the way, bevy_eventlistener is largely doomed with #13991 being merged, but we still need run conditions on observers anyways.

@DasLixou
Copy link
Contributor Author

DasLixou commented Aug 1, 2024

I guess bevy_mod_picking will also not receive an update using observers because of it's upstreaming? So waiting for 0.15 release it is..?

@alice-i-cecile
Copy link
Member

Most likely yeah. Fingers crossed we can get everything in place for 0.15 though!

@DasLixou
Copy link
Contributor Author

DasLixou commented Aug 1, 2024

Good news! I could work around it pretty easy by pipeing the bool into my system as an In and returning early

@alice-i-cecile alice-i-cecile added D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished and removed D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Aug 2, 2024
@jnhyatt
Copy link
Contributor

jnhyatt commented Sep 19, 2024

we still need run conditions on observers anyways

Is this the case if we get entity disabling?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

No branches or pull requests

3 participants