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

Bevy 0.15.1 change to fallible systems breaks projects #17138

Open
Schmarni-Dev opened this issue Jan 4, 2025 · 4 comments
Open

Bevy 0.15.1 change to fallible systems breaks projects #17138

Schmarni-Dev opened this issue Jan 4, 2025 · 4 comments
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior S-Needs-SME Decision or review from an SME is required X-Controversial There is active debate or serious implications around merging this PR
Milestone

Comments

@Schmarni-Dev
Copy link

Bevy version

0.15.1

What went wrong

I tried to update to bevy 0.15.1 for a project and it crashed on startup, because of a missing systemparam (in this case an Event)
i get that the default in bevy 0.15.0 for missing systemparams was bad, i think panics are the better default, i support changing the default, but please don't do it in a patch release! especially without giving apps a way to change the default without forking bevy

so i tried to downgrade to 0.15.0; but the bevy crate doesn't depend on the exact version for subcrates, so while i was able to downgrade bevy, bevy_ecs and co are still 0.15.1, breaking my current code without a way back (other than cargo patching all crates to the git commit of bevy 0.15.0, maybe adding all the crates as app dependencies would work too idk)

@Schmarni-Dev Schmarni-Dev added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jan 4, 2025
@Schmarni-Dev
Copy link
Author

luckily in my case i just needed to add a singe plugin, but still bad

@alice-i-cecile alice-i-cecile modified the milestones: 0.16, 0.15.2 Jan 4, 2025
@BenjaminBrienen
Copy link
Contributor

BenjaminBrienen commented Jan 4, 2025

The suggestion to panic is a reasonable stop-gap, and should have been done in the initial PR.
originally posted by @alice-i-cecile in #16718

To me, this is a good reason to put a change which is technically breaking into a patch release. From the perspective of engine maintainers, the feature which was shipped had a fundamental problem and didn't work as intended.

@BenjaminBrienen BenjaminBrienen added S-Needs-SME Decision or review from an SME is required and removed S-Needs-Triage This issue needs to be labelled labels Jan 4, 2025
@alice-i-cecile
Copy link
Member

Agreed. From Cart's (and the community's) perspective, the existing solution was fundamentally broken, and reverting to 0.14 behavior was preferred. I don't think we should do anything further here, but I'll leave it open for a bit more discussion.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events X-Controversial There is active debate or serious implications around merging this PR labels Jan 5, 2025
@alice-i-cecile alice-i-cecile changed the title Bevy 0.15.1 is breaking projects Bevy 0.15.1 change to fallible systems breaks projects Jan 5, 2025
@fallible-algebra
Copy link

fallible-algebra commented Jan 13, 2025

The reversion to 0.14 behaviour does feel to me like a significant pain point, especially in a patch release after a release where "skip system/observer if its parameter's constraints aren't met" was lauded as a new feature that was a jump up in usability. It's disappointing to have to shift the mental model of how to work on my project in future or when cargo next decides that I should be depending on 0.15.1 because it's "only a patch change".

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-Bug An unexpected or incorrect behavior S-Needs-SME Decision or review from an SME is required X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

No branches or pull requests

4 participants