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

Require components that can't be defaulted #16194

Closed
andriyDev opened this issue Oct 31, 2024 · 6 comments
Closed

Require components that can't be defaulted #16194

andriyDev opened this issue Oct 31, 2024 · 6 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Milestone

Comments

@andriyDev
Copy link
Contributor

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

Today, a required component either needs to impl Default or needs to have an explicit constructor function passed in. This is problematic for components which require some initial state. Consider the following setup:

#[derive(Component)]
#[require(EnemyTarget)]
struct Enemy;

#[derive(Component)]
struct EnemyTarget(Entity);

I want to say that an enemy needs to be inserted with a target, but a defaulted target doesn't really make sense and Entity doesn't even impl Default.

What solution would you like?

require should be able to not use a constructor at all and just require that the bundle inserts the component. Perhaps this could be done with a keyword like #[require(EnemyTarget(explicit))] to say that a component must be explicitly inserted and will fail (at compile time) otherwise.

This would severely decrease the remaining need for bundles.

One downside is that you can still break this invariant by explicitly removing one of the components after the fact, but that is already true.

What alternative(s) have you considered?

Wait for archetype invariants. This would unfortunately mean problems would be detected at runtime.

@andriyDev andriyDev added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Oct 31, 2024
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events S-Needs-Design This issue requires design work to think about how it would best be accomplished and removed S-Needs-Triage This issue needs to be labelled labels Oct 31, 2024
@alice-i-cecile
Copy link
Member

I much prefer non-defaulted required components over using archetype invariants to resolve this problem.

@alice-i-cecile alice-i-cecile added S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes and removed S-Needs-Design This issue requires design work to think about how it would best be accomplished labels Oct 31, 2024
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Oct 31, 2024
@SkiFire13
Copy link
Contributor

and will fail (at compile time) otherwise

As much as this would be nice to have, I don't think it's something possible to do without some kind of const-comparable TypeId.

@andriyDev
Copy link
Contributor Author

andriyDev commented Nov 5, 2024

After implementing #16209, I feel dissatisfied with my own solution. Because of how dynamic we are allowed to be (you can register required components at runtime), the only valid solution AFAIK is just panicking at runtime. This basically seems like worse archetype invariants to me.

@alice-i-cecile
Copy link
Member

Yeah, the transitive and dynamic nature of required components makes this quite hard to use effectively.

@andriyDev
Copy link
Contributor Author

@alice-i-cecile, thoughts on just closing this and the PR and just waiting for archetype invariants?

@alice-i-cecile
Copy link
Member

I think I'm in favor of closing this, yeah. I don't see any good way to handle the non-trivial / ecosystem implications, and any full, thoroughly designed solutions feels more in line with a complete archetype invariants design (#1481, for those who are out of the loop).

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-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants