mackysoft / Unity-SerializeReferenceExtensions

Provide popup to specify the type of the field serialized by the [SerializeReference] attribute in the inspector.
https://qiita.com/makihiro_dev/items/26daeb3e5f176934bf0a
MIT License
754 stars 56 forks source link

[FEATURE] Support types with contravariance #70

Open DreadKyller opened 1 week ago

DreadKyller commented 1 week ago

Feature destription

Given a structure like the following:

interface IAction<in T> where T : IActor { ... }

interface IActorAction : IAction<IActor> { }
interface IStandardActorAction: IAction<IStandardActor> { }
interface INetworkActorAction: IAction<INetworkActor> { }

Where IStandardActor and INetworkActor have a shared super interface of IActor.

We can imagine the situation where IActor is used for actions that are compatible with both standard and networked actors, where IStandardAction and INetworkAction are only compatible with IStandardActor and INetworkActor respectively, where IStandardActor may be implemented on a MonoBehaviour and INetworkActor may be implemented on a NetworkBehaviour.

Under such conditions, attempting to store the list of actions as such:

[SerializeReference, SubclassSelector]
private List<IAction<INetworkActor>> actions;

That list of actions should allow selection of both IAction<INetworkActor> andIAction<IActor>, as IAction<IActor> is contravariant to IAction<INetworkActor> and IAction::T is marked as supporting contravariance. In fact if I manually add an implementation of both to the list it is fully supported, but only IAction<INetworkActor> specifically are found with the subclass selector.

This scenario is what we find ourselves dealing with when working on a modular controller for our games. Supporting contravariant types like this would, I feel, be quite useful.

Antoshidza commented 1 week ago

In my project I have tones of cases where I need such a feature, because as @DreadKyller I need to build modular simulation. So I vote for this feature :)

Though as I understand the case of example with actions and actors, next code explicitly declare that it needs at least IAction<INetworkActor> while IAction<IActor> could be IAction<IStandardActor> which as I understand would be selectable but won't be casted properly. Am I wrong?

[SerializeReference, SubclassSelector]
private List<IAction<INetworkActor>> actions;
DreadKyller commented 6 days ago

In my project I have tones of cases where I need such a feature, because as @DreadKyller I need to build modular simulation. So I vote for this feature :)

Though as I understand the case of example with actions and actors, next code explicitly declare that it needs at least IAction<INetworkActor> while IAction<IActor> could be IAction<IStandardActor> which as I understand would be selectable but won't be casted properly. Am I wrong?

[SerializeReference, SubclassSelector]
private List<IAction<INetworkActor>> actions;

You are correct that this requests INetworkActor, however as the T parameter in IAction is marked as in this marks the parameter as contravariant, it specifies that type parameter can only be used as input, so T can't be used as output, due to this, any function that recieves a value of IActor can accept a value of INetworkActor.

private void Example1(INetworkActor) {}
private void Example2(IActor actor) {}
---
Example1(new INetworkActor());
Example2(new INetworkActor());

In the end using contravariance means that it can accept T being any class that is of a less defined type. It basically specifies that a specific type will be provided as input. If I pass an INetworkActor into something, it can be casted down to an IActor without issue, but casting an IActor to an INetworkActor is going to fail under most circumstances.

// Only accepts IActor, INetworkActor and IStandardActor aren't valid since they are more-defined types
IAction<IActor>

// Only accepts IActor and INetworkActor as IStandardActor isn't part of INetworkActor's type hierarchy
IAction<INetworkActor>

// Only accepts IActor and IStandardActor as INetworkActor isn't part of IStandardActor's type hierarchy
IAction<IStandardActor>

The opposite of this would be covariant, which you do by marking the parameter as out instead of in, this specifies that the type can only be used as output types, and not as input (though if an input type takes generic parameters and some of those parameters are used for output the parameter can be used in those types as well). Marking the parameter as out communicates that they who fulfil the contract of the interface/class must provide at least that type. The actual implementation can specify a more-defined type, because the output type then can be guaranteed to be castable down to the requested type.

For more information: https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/covariance-contravariance/

Unity supports this just fine, adding an item to the list unity will serialize it properly and it works without issue, the problem comes from the selection. Do to an optimization that was made to SubclassSelector they moved from scanning through the assembly's manually, to using Unity's TypeCache, and unfortunately Unity's TypeCache doesn't support lookup for covariant and contravariant types. So I have no clue immediately how to implement this given it's current implementation, at least not without invalidating the optimizations. even in the older versions that searched the assembly's manually this wasn't a feature, but it would have been relatively simple to implement.

For my purposes I only have a few types so I just modified the subclass selector to allow my specifying other types to search in addition to the one specified in the field type. Still I think that finding a solution to this would be very useful.

Antoshidza commented 6 days ago

Yep, seems that I misunderstood you. Fully agree. I have tons of cases where I need such a feature.

Antoshidza commented 5 days ago

If we can check that type is covariance/contravariance generic interface and get it's generic T type we could find it's TypeCache.GetTypesDerivedFrom then construct same interface with each of these types and recursively resolve derived types for result interface type. That would allow us still use TypeCache and find all more/less derived types (for less derived types it is simple as just find all base classes so in that case we don't even need TypeCache).

DreadKyller commented 4 days ago

Indeed might be an option to look into. I don't know much about the internals of Unity's TypeCache, if we construct a Type based on the type of the field and parameters I'm not entirely certain whether it'll match the entry in the type cache. Also when dealing with multiple parameters this becomes more difficult, if you have 2 parameters with 3 options each that now becomes 9 combinations you have to check derived types from.