[Editor] Collection support in abstract properties

[Eideren]

PR Details

Collection stored into properties of a non-collection type do not allow for adding and removing from said collection, this PR fixes that and ensures the feature can co-exist with the abstract setter.

Excerpt from bepu:

class MyClass
{
    public required ICollider Collider;
}

interface ICollider { }

[DataContract]
class CompoundCollider : List<ColliderBase>, ICollider { }

Related Issue

None reported afaict.

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My change requires a change to the documentation.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have built and run the editor to try this change out.

Debugging quantum is such a shore, this PR doesn't even finish support for dictionaries or sets, I just don't have time to cover those as well.
@Kryptos-FR could you take a quick look to check that this nonsense is fine - that would be great.

[Kryptos-FR]

@Eideren It's not trivial 😅. Allow me a few days to take the time to review it.

If I understand correctly, the issue is for a new type inheriting from a collection type? If so, can't composition be used instead of inheritance?

[Nicogo1705]

@Eideren It's not trivial 😅. Allow me a few days to take the time to review it.

If I understand correctly, the issue is for a new type inheriting from a collection type? If so, can't composition be used instead of inheritance?

It work with unique inheritance (eg : https://github.com/Nicogo1705/Stride.BepuPhysics/blob/master/Stride.BepuPhysics/Definitions/ListOfColliders.cs )

But as soon as you add an interface (eg : class MyClass : List< Entity >, IMyInterface ) make the collection "readonly" inside editor (we cannot add new items).

but it should be like in the screen from Eideren that fix that issue.

See https://discord.com/channels/500285081265635328/1183088711810945157/1198297089151619273 for more informations.

[Kryptos-FR]

My question is more: why are we using inheritance instead of composition.

[DataContract]
class CompoundColliders : ICollider
{
    public List<ColliderBase> Colliders { get; set; }
    public int SomeOtherProperty { get; set; }
}

Quantum is built towards composition, so instead of fighting it and possibly introducing non-critical bugs or regression, it would be better to adapt to it.

Inheriting a collection is a code smell for me, even outside Stride context.

[Nicogo1705]

Ho, that's actually how i fixed it until this PR get merged. You will have to wait for Eideren answer then.

[Eideren]

If I understand correctly, the issue is for a new type inheriting from a collection type? If so, can't composition be used instead of inheritance?

Kind of but it goes a bit further than that:

class MyClass
{
   // Users cannot add or remove items from this list
   public AnyTypeOrInterfaceThatCanHoldAList MyField = new List<float>();
   // or this one
   public Object MyField2 = new MyCustomListType();
   // but they can swap the value of those fields to one of a different type somehow
}
class MyCustomListType : IList<float>{}

Inheriting a collection is a code smell for me, even outside Stride context.

Yes, the end goal of that collection is to implement IList<> instead of inheriting from List<>, I think that was an issue that has been fixed since but haven't validated that yet.

Quantum is built towards composition, so instead of fighting it and possibly introducing non-critical bugs or regression, it would be better to adapt to it.

I do agree with you if this was for a project using the engine, we shouldn't create hacks that have the potential to break the engine. But here we do have the ability to fix those issues, granted it might not be as easy, but we really shouldn't dictate how users should build their API. I think it makes sense that the editor has the same feature set as the serialization system, and our serialization system and dotnet coverage is a big help in converting users of unity over to our engine; tons of users were relieved when they could use properties in the engine for example.