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

Add a way to globally disable the required components feature #16924

Open
Bcompartment opened this issue Dec 22, 2024 · 3 comments
Open

Add a way to globally disable the required components feature #16924

Bcompartment opened this issue Dec 22, 2024 · 3 comments
Labels
A-ECS Entities, components, systems, and events X-Controversial There is active debate or serious implications around merging this PR

Comments

@Bcompartment
Copy link

PROPOSAL

Add a way to globally disable the required components feature. Or make required components compile-time optional feature. IMO required components can create confusion and unexpected behavior.

ALTERNATIVE

without components automatically added
commands.spawn(...).insert(...);

with components automatically added
commands.spawn_with_required(...).insert_with_required(...);

@ItsDoot ItsDoot added A-ECS Entities, components, systems, and events X-Controversial There is active debate or serious implications around merging this PR labels Dec 22, 2024
@bushrat011899
Copy link
Contributor

Ignoring whether we should offer this option for a moment, how should we handle spawning a component which has required components when the feature is disabled?

  1. Just insert the explicit component and do nothing with the required components. This would likely cause a runtime panic immediately for basically every Bevy application, since core components like Node are defined knowing that their required components are available.
  2. Panic if the required components aren't already there. This would panic even more than option 1, but it would happen immediately when trying to spawn rather than being a ticking time bomb which may panic eventually.

For the record, I don't like the idea of feature gating something so fundamental to how the ECS is being used internally. But if we're thinking about this, we might as well think it through.

@mweatherley
Copy link
Contributor

This would likely cause a runtime panic immediately for basically every Bevy application, since core components like Node are defined knowing that their required components are available.

I believe the situation is actually worse than that, and component hooks are allowed to assume that required components exist at insertion time for the sake of avoiding undefined behavior, which makes it hard to imagine a world where this feature is implemented soundly.

Without both a path forward in that capacity and a stronger motivation for implementing this in the first place, I would recommend closing this issue as not planned.

@BenjaminBrienen
Copy link
Contributor

I'm also in favor of closing this as not planned.

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 X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

No branches or pull requests

5 participants