Making Required Components force existence as a follow-up of #18514

[inact1v1ty]

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

The #18514 proposes a way to enforce the invariant of existence of required components.
While the following opinion can be controversial, I believe that this invariant should be held by default - as even the wording of Required Components means that the specified component is required for correct behaviour of the component it is specified for.

As for justification of having the API to enforce such a variant - see #18514, it is described there wonderfully that this both improves DX and can improve performance by removing unnecessary checks.

I propose that the behaviour of

#[removal(stops(B)]
struct A;

(states that when A is present, B can not be removed) should be the default behaviour of required components, and that

#[removal(when(B)]
struct A;

(states that when B is removed, A is also removed) and the current behaviour of allowing to remove the required component afterwards should be options on the required component.

What solution would you like?

• #[require(B)] and world.register_required_components now also apply the logic of #[removal(stops(B)] by default.
• Add a parameter to world.register_required_components for specifying the behaviour of removal tracking.
• If you want the current behaviour of allowing removals of required component on #[require(B)], you should write

  #[require(B, allow_removal)]
  struct A;

• If you want the behaviour of #[removal(when(B)] on #[require(B)], you should write

  #[require(B, removed_alongside)]
  struct A;

What alternative(s) have you considered?

Do not change the behaviour, so in most cases the developer would write either (most of the time)

#[required(B)]
#[removal(stops(B)]
struct A;

or like now (second place of commonness) to allow removal

#[required(B)]
struct A;

or (in I think very rare cases)

#[required(B)]
#[removal(when(B)]
struct A;

I personally think this is a worse DX than changing the behaviour of required components.

Additional context

Unity has the notion of required components in its EC architecture, and it prevents the removal of the required component both in Editor and in Playmode both from Editor UI and from scripts with the error Can't remove *B* because *A* depends on it.

[alice-i-cecile]

@cart is on the fence about this, but I think that this makes sense as one of a few related variants on the concept.

[ElliottjPierce]

Coming from #18514, I actually like this solution generally better from a DX perspective. But here's a few things to think about.

First, I'd like to see inverses. Not just stops but stopped_by, too. That will make this more reusable besides required components. This would be trivial to layer on top of this, though I agree that it's not needed ATM.

Second, I'm not sure about baking this into the syntax of #[require()] There's lots of patterns you can use in there that might be incompatible. But I do like that require adds this invariance by default. Maybe we add #[removal(allow)] to overwrite the default?

Third, I don't think a silent fail with stops is enough. What if I need the component to be removed? I think replacing the component with a default or something (maybe reusing the required component constructor) is a better default. What if the component has a Handle we're trying to unload? What if it has some kind of concurrent structure we need to drop its end of? I like stops as a default, but I think it could be dangerous. I think we need a removal(replace) or something. It's situational, but I think remove should (by default) always do the removal (even if a brand new component of the same type just replaces it).

Finally, I think we should implement this very generally. Just like spawning has affects (and more planned), I think we should make this similarly extendable (even though it's not planned yet). And I think we can implement some of this at the BundleRemover level (#18521) because that will be faster than removing it with hooks, etc.

[inact1v1ty]

I think it can be a good idea not to bake removal into required syntax and add #[removal(allow)]. This can result in a better DX.

What if I need the component to be removed?

In this (I think a pretty rare) case, for example, unloading a Handle, my opinion is that you can manually just do an .insert with some value you like (Default, custom constructor or whatever), as it will overwrite the component. Stopping removal won't stop this.

I personally think that doing this by default is a strange and misleading behaviour, when something tries to remove a component and it suddenly has somewhat corrupted data.

Additionally, maybe trying to remove a required component should produce a warning or error.

I think we should implement this very generally.

I think I agree with this.

[ElliottjPierce]

In this (I think a pretty rare) case, for example, unloading a Handle, my opinion is that you can manually just do an .insert with some value you like (Default, custom constructor or whatever), as it will overwrite the component. Stopping removal won't stop this.

I agree that it's rare. But having the user do insert themselves won't work everywhere. What the user wants is not "I want to replace this" but "I want/need to destroy this". insert doesn't do that. We also can't let the user have separate queries for "remove these cuz they're normal" and "insert on these because I can't remove them". That's complex and, the user can't know ahead of time what will require the component in advance. Unless we make remove return some kind of result, I still think (for these admittedly few situations) we need something more than stops or insert.

I personally think that doing this by default is a strange and misleading behavior, when something tries to remove a component and it suddenly has somewhat corrupted data.

I agree here too, but all of this is "strange and misleading". It is misleading that remove doesn't always remove. It would be misleading that remove is actually insert but only sometimes. And it's misleading that, depending on what downstream crates register, removing A could also remove B as a result, even though as far as the crate with the removal knows, A and B are not related by requirement.

Additionally, maybe trying to remove a required component should produce a warning or error.

100% agree here. This should tap into the new error handling somehow.

---

I think making remove return an error is the simplest solution, but it's worth discussion.

[inact1v1ty]

I agree with your points, with them considered my opinion is strongly on making remove return an error if it is trying to remove a required component.

Also, you brought up an interesting thing:

And it's misleading that, depending on what downstream crates register, removing A could also remove B as a result, even though as far as the crate with the removal knows, A and B are not related by requirement.

This is a source of misleading behaviour and even bugs, maybe. It should be thought through carefully.

It is somewhat like Rust's orphans rule prevents, by the way.

[ElliottjPierce]

Yeah, I agree. We're on the same page; it's just tricky lol.

[alice-i-cecile]

Before we return an error here, I want to solve #17273.

[ElliottjPierce]

I'm thinking about this more, and I think this is really just a special case of #1481 (archetype invariants). It's just that we want to register these invariants when we register required components. I think we should probably tackle (incrementally) #1481 and then come back to make required components trigger this behavior.

I do still think remove, take, insert, spawn (maybe others) should return results, where it errors when the request would produce an invalid archetype.