enhancement: make the Component.add, Component.addToParent and Component.remove methods generic

[wolfenrain]

Problem to solve

As a developer I want to have generic information of component being added or removed, so that I can add extra validation steps if I do so require.

While this feature won't be used in most games, it can be quite crucial for third-party Flame packages developers as it gives them the extra control to enforce certain rules that their package might require, for instance a custom ECS build on top of the existing FCS.

Proposal

My proposal is to add generic parameters to the following methods:

• Component.add

  Future<void>? add<T extends Component>(T component) {
     // add logic stays the same.
  }

• Component.remove

  void remove<T extends Component>(T component) {
     // remove logic stays the same.
  }

• Component.addToParent

  Future<void>? addToParent<T extends Component>(T parent) {
     // addToParent logic stays the same.
  }

• Component.changeParent

  void changeParent<T extends Component>(T parent) {
     // changeParent logic stays the same.
  }

This would be a breaking change as the method definition will change, and anyone who was already overwriting these methods will have to make changes to reflect the new definition.

[spydon]

Do we need it on addAll and changeParent too?
I don't think we need it on remove, since all added components would already be verified to be of the correct type.

[wolfenrain]

Do we need it on addAll and changeParent too?

The addAll uses a List we could add it there but eventually that would only be "single" type in that sense, and it calls the add method anyway, so any logic you need should be done there.

The changeParent can be made generic as well, but it will call the removeFromParent or addToParent anyway, but for concisentency sake it might be worth doing so yes.

I don't think we need it on remove, since all added components would already be verified to be of the correct type.

You still might want to have some other type of logic there, it wouldnt do any harm adding it on that method as well.

[spydon]

The addAll uses a List we could add it there but eventually that would only be "single" type in that sense, and it calls the add method anyway, so any logic you need should be done there.

By adding it to addAll too you would avoid a runtime error right? Maybe it even has to be added to add all... Otherwise the type checking wont succeed in the default addAll?

The changeParent can be made generic as well, but it will call the removeFromParent or addToParent anyway, but for concisentency sake it might be worth doing so yes.

Same here.

You still might want to have some other type of logic there, it wouldnt do any harm adding it on that method as well.

When you say logic, do you mean restrictions? Logic I feel would be type checking that is done directly on the object and not checks on T?

Can you put up some example usecases? I think maybe the whole Component class should have the generic, instead of each separate method?

[erickzanardo]

I am failing to understand how adding a generics would to these methods would give more control or make it easier to enforce rules. Virtually adding the generics <T extends Component> is pretty much the same as accepting a Component on the method right as you can check the type of the parameter with is.

But maybe I am missing something so a use case here would be very important to continue the discussion.

[wolfenrain]

I am failing to understand how adding a generics would to these methods would give more control or make it easier to enforce rules. Virtually adding the generics <T extends Component> is pretty much the same as accepting a Component on the method right as you can check the type of the parameter with is.

But maybe I am missing something so a use case here would be very important to continue the discussion.

Let's say you have a use case where a certain component can only hold one instance of any other component as it's child. To be able to do that you would need to be able to specifically filter down using it's type.

A small example of the above use case could, with generics, look like this:

@override
Future<void>? add<T extends Component>(T component) {
  assert(children.whereType<T>().isEmpty, 'The parent already has a component of type $T');
  return super.add(component);
}

This wouldn't be possible without generics unless you provide a new method. Which in my case would not be preferred.

[wolfenrain]

By adding it to addAll too you would avoid a runtime error right? Maybe it even has to be added to add all... Otherwise the type checking wont succeed in the default addAll?

There is nothing stopping us from also having addAll generic.

When you say logic, do you mean restrictions? Logic I feel would be type checking that is done directly on the object and not checks on T?

Logic is whatever the user wants to do, it can be constraints, lookups or whatever. The fact that you have that extra data is just the added addition.

Can you put up some example usecases? I think maybe the whole Component class should have the generic, instead of each separate method?

I just posted one above! But I think each method should still have their own generic. On the class itself makes no sense imho as that would imply that only that Type can be added, while you often have multiple different children.

[spydon]

Let's say you have a use case where a certain component can only hold one instance of any other component as it's child. To be able to do that you would need to be able to specifically filter down using it's type.

A small example of the above use case could, with generics, look like this:

@override
Future<void>? add<T extends Component>(T component) {
  assert(children.whereType<T>().isEmpty, 'The parent already has a component of type $T');
  return super.add(component);
}

This wouldn't be possible without generics unless you provide a new method. Which in my case would not be preferred.

That is not at all how I imagined that it would be used, since that would be a runtime error, and that runtime error can just as well be done by type checking today:

@override
Future<void>? add(Component c) {
  assert(children.where((c2) => c.runtimeType == c2.runtimeType).isEmpty, 'The parent already has a component of type ${c.runtimeType}');
  return super.add(component);
}

What I imagined was using T to restrict what types that can be added to a component by setting a stricter generics when overriding (which is not your usecase), so your usecase is what meant by "logic" in my other comment.

I'm not sure I like this way of using generics.

[erickzanardo]

This wouldn't be possible without generics unless you provide a new method. Which in my case would not be preferred.

I am not sure if that is true, I haven't tested but I think this would work:

@override
Future<void>? add(Component component) {
  assert(
    children.where((child) => child.runtimeType == component.runtimeType).isEmpty,
    'The parent already has a component of type ${child.runtimeType}',
  );
  return super.add(component);
}

[wolfenrain]

That is not at all how I imagined that it would be used, since that would be a runtime error, and that runtime error can just as well be done by type checking today:

@override
Future<void>? add(Component c) {
  assert(children.where((c2) => c.runtimeType == c2.runtimeType).isEmpty, 'The parent already has a component of type ${c.runtimeType}');
  return super.add(component);
}

What I imagined was using T to restrict what types that can be added to a component by setting a stricter generics when overriding (which is not your usecase), so your usecase is what meant by "logic" in my other comment.

I'm not sure I like this way of using generics.

Using runtimeType to test for types is not stable (https://dart.dev/guides/language/language-tour#getting-an-objects-type) and also more expensive (can't find the sources about that at the moment, will keep looking dart-lang/sdk#48896 is a quick example related to the expensiveness).

But even tho this example could be rewritten like this I don't think that should be a reason to not add these generics typings. As they come with zero overhead and would provide much more valuable data to anyone needing that on the methods (or component).

T can be used as you mentioned to restrict what can be added but for instance in flame_behaviors we could use it to enforce rules specifically for that pattern without the developer needing to learn new APIs. Generics can also be used as cost efficient way to auto register child queries or call any other method that requires generics. Something that won't be possible if the starting method (add, addToParent and such) don't have a generic type.

[st-pasha]

Generics have costs though, don't they? When you have a method like Future<void>? add<T extends Component>(T component);, then a new method will be generated for every component type T that the user has, which could be quite a lot.

Looking at dart-lang issue that you linked, there seem to be some comments that indicate that accessing .runtimeType is not that expensive after all.

Lastly, I believe @erickzanardo's solution for the problem posed (i.e. how to ensure that a component can have only children of distinct types) is more correct. For example, suppose I have a PositionComponent, and a SpriteComponent. These two have different types, but the solution with generics will make it so that adding PositionComponent then SpriteComponent would succeed, whereas SpriteComponent then PositionComponent would fail. The Erick's solution doesn't suffer from this bug.

Of course, there is another bug with both solutions: the add is a delayed method, meaning if you add() something then it doesn't appear in the children list right away. So, the way to fix this would be to move the logic to the onMount() handler -- which, again, would work with the Erick's solution but not with the generics.