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

[FEATURE] Support types with contravariance #70

Closed
DreadKyller opened this issue Oct 8, 2024 · 8 comments · Fixed by #75
Closed

[FEATURE] Support types with contravariance #70

DreadKyller opened this issue Oct 8, 2024 · 8 comments · Fixed by #75
Assignees
Labels
enhancement New feature or request

Comments

@DreadKyller
Copy link

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> and IAction<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.

@DreadKyller DreadKyller added the enhancement New feature or request label Oct 8, 2024
@Antoshidza
Copy link

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
Copy link
Author

DreadKyller commented Oct 11, 2024

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
Copy link

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

@Antoshidza
Copy link

Antoshidza commented Oct 13, 2024

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
Copy link
Author

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.

@mackysoft
Copy link
Owner

I am currently trying an experimental type search. I may be missing a case, but so far it appears to be working.

image

using System;
using System.Collections.Generic;
using UnityEngine;

public interface IActor { }
public interface IStandardActor : IActor { }
public interface INetworkActor : IActor { }
public interface IAction<in T> where T : IActor { }

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

[Serializable]
public sealed class StandardActorAction : IAction<IStandardActor> { }

[Serializable]
public sealed class ActorAction : IAction<IActor> { }

[Serializable]
public class BaseAction<T> : IAction<T> where T : IActor { }

[Serializable]
public sealed class DerivedAction1 : BaseAction<IActor> { }

[Serializable]
public sealed class DerivedAction2 : BaseAction<INetworkActor> { }

[Serializable]
public sealed class DerivedAction3 : BaseAction<IStandardActor> { }

[Serializable]
public sealed class NetworkActorAction1 : INetworkActorAction { }

[Serializable]
public sealed class NetworkActorAction2 : IAction<INetworkActor> { }

[Serializable]
public sealed class NetworkActorAction3 : IAction<IActor> { }

public class Example_Contravariance : MonoBehaviour
{

	[SerializeReference, SubclassSelector]
	public List<IAction<INetworkActor>> action = new List<IAction<INetworkActor>>();
}
public static IEnumerable<Type> GetTypes (Type baseType)
{
	var result = new List<Type>();
	Type genericInterfaceDefinition = null;
	Type[] targetTypeArguments = null;
	Type[] genericTypeParameters = null;

	if (baseType.IsGenericType)
	{
		genericInterfaceDefinition = baseType.GetGenericTypeDefinition();
		targetTypeArguments = baseType.GetGenericArguments();
		genericTypeParameters = genericInterfaceDefinition.GetGenericArguments();
	}
	else
	{
		genericInterfaceDefinition = baseType;
		targetTypeArguments = Type.EmptyTypes;
		genericTypeParameters = Type.EmptyTypes;
	}

	var assemblies = AppDomain.CurrentDomain.GetAssemblies();

	foreach (var assembly in assemblies)
	{
		foreach (var type in assembly.GetTypes())
		{
			if ((type.IsPublic || type.IsNestedPublic || type.IsNestedPrivate) &&
				!type.IsAbstract &&
				!type.IsGenericType &&
				!typeof(UnityEngine.Object).IsAssignableFrom(type) &&
				Attribute.IsDefined(type, typeof(SerializableAttribute)) &&
				!Attribute.IsDefined(type, typeof(HideInTypeMenuAttribute)))
			{
				var interfaces = type.GetInterfaces();
				foreach (var iface in interfaces)
				{
					if (iface.IsGenericType && iface.GetGenericTypeDefinition() == genericInterfaceDefinition)
					{
						var sourceTypeArguments = iface.GetGenericArguments();

						bool allParametersMatch = true;

						for (int i = 0; i < genericTypeParameters.Length; i++)
						{
							var variance = genericTypeParameters[i].GenericParameterAttributes & GenericParameterAttributes.VarianceMask;

							var sourceTypeArg = sourceTypeArguments[i];
							var targetTypeArg = targetTypeArguments[i];

							if (variance == GenericParameterAttributes.Contravariant)
							{
								if (!sourceTypeArg.IsAssignableFrom(targetTypeArg))
								{
									allParametersMatch = false;
									break;
								}
							}
							else if (variance == GenericParameterAttributes.Covariant)
							{
								if (!targetTypeArg.IsAssignableFrom(sourceTypeArg))
								{
									allParametersMatch = false;
									break;
								}
							}
							else
							{
								if (sourceTypeArg != targetTypeArg)
								{
									allParametersMatch = false;
									break;
								}
							}
						}

						if (allParametersMatch)
						{
							result.Add(type);
							break;
						}
					}
				}
			}
		}
	}

	return result;
}

@mackysoft
Copy link
Owner

mackysoft commented Oct 26, 2024

If released, it would be UNITY_2023_1_OR_NEWER. As the behaviour was not stable in 2023.1, 2023.2 may be better.
https://unity.com/releases/editor/whats-new/2023.1.0#notes

@mackysoft mackysoft linked a pull request Oct 27, 2024 that will close this issue
@mackysoft
Copy link
Owner

@DreadKyller
In 1.6.0, covariance / contravariance support was implemented. It should work.
This feature work with Unity 2023.2 and later.
https://github.com/mackysoft/Unity-SerializeReferenceExtensions/releases/tag/1.6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants