[FEATURE REQUEST] Collect AutoGenerated Cleanup Systems into single System per Context

[fahall]

Problem I use quite a few [Cleanup] components. These generate cleanup systems for me (e.g. DestroyGameEntitiesWithMyComponentSystem), which is great; however, I then have to remember to add each of them to my game.

Solution I'd prefer that an additional AutoGeneratedGameCleanupSystems system be created that compiles all of the automatically generated cleanup systems into a single system that I can add to my game.

Alternatives I can do this manually and maintain separate autogenerated cleanup features, but that still involves manually maintaining the systems contained in there. It would provide a layer of abstraction, but doesn't alleviate the maintenance hassle.

Vanilla Entitas provides only a single systems that lumps together all of the cleanups per context. I do prefer the ability to pick and choose that's supported by having the individual systems, but usually (read: programmer speak for always) I'm just going to want to use all of them.

[jeffcampbellmakesgames]

@fahall What's your thought on something like these. I have two code generators for this; one generates a feature and another generates a systems. Both are generated per context and contain all of the cleanup systems associated with that context.

//------------------------------------------------------------------------------
// <auto-generated>
//      This code was generated by a tool (Genesis v1.3.0, branch:develop).
//
//
//      Changes to this file may cause incorrect behavior and will be lost if
//      the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------
using JCMG.EntitasRedux;

public class VisualDebugCleanupFeature : Feature
{
    public VisualDebugCleanupFeature() : base()
    {
        AddSystems(Contexts.SharedInstance.VisualDebug);
    }

    public VisualDebugCleanupFeature(IContext<VisualDebugEntity> context) : base()
    {
        AddSystems(context);
    }

    private void AddSystems(IContext<VisualDebugEntity> context)
    {
        Add(new DestroyVisualDebugEntitiesWithAnCleanupDestroyEntitySystem(context));
        Add(new RemoveAnCleanupRemoveFromVisualDebugEntitiesSystem(context));
    }
}

//------------------------------------------------------------------------------
// <auto-generated>
//      This code was generated by a tool (Genesis v1.3.0, branch:develop).
//
//
//      Changes to this file may cause incorrect behavior and will be lost if
//      the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------
using JCMG.EntitasRedux;

public class TestCleanupSystems : JCMG.EntitasRedux.Systems
{
    public TestCleanupSystems() : base()
    {
        var context = Contexts.SharedInstance.Test;
        _cleanupSystems.Add(new RemoveBaseFromTestEntitiesSystem(context));
        _cleanupSystems.Add(new RemoveAnyBaseAddedListenerFromTestEntitiesSystem(context));
    }

    public TestCleanupSystems(IContext<TestEntity> context) : base()
    {
        _cleanupSystems.Add(new RemoveBaseFromTestEntitiesSystem(context));
        _cleanupSystems.Add(new RemoveAnyBaseAddedListenerFromTestEntitiesSystem(context));
    }
}

[jeffcampbellmakesgames]

This work can be previewed on the feat/aggregate_cleanup_systems branch.

[fahall]

Those look great to me!

Quick question of style: why two constructors instead of null propagation or Having one ctor call the other? I’m not suggesting either approach is better, but curious as to your thought process.

public TestCleanupSystems(IContext<TestEntity> contextOverride = null) : base() { var context = contextOverride ?? Contexts.SharedInstance.Test; _cleanupSystems.Add(new RemoveBaseFromTestEntitiesSystem(context)); _cleanupSystems.Add(new RemoveAnyBaseAddedListenerFromTestEntitiesSystem(context)); }

Or:

public TestCleanupSystems() : this(Contexts.SharedInstance.Test) { }

*wrote those two on my phone, so they may not compile as is😁

[jeffcampbellmakesgames]

Those look great to me!

Quick question of style: why two constructors instead of null propagation or Having one ctor call the other? I’m not suggesting either approach is better, but curious as to your thought process.

public TestCleanupSystems(IContext<TestEntity> contextOverride = null) : base() { var context = contextOverride ?? Contexts.SharedInstance.Test; _cleanupSystems.Add(new RemoveBaseFromTestEntitiesSystem(context)); _cleanupSystems.Add(new RemoveAnyBaseAddedListenerFromTestEntitiesSystem(context)); }

It's a clean code thing of generally trying to avoid optional parameters in favor of explicit methods with overloads. If a parameter is optional, it indicates that a method branches it's logic based on the different values of that param and if you're unit testing the more params a method has the more test cases there could be. An empty constructor implies it only has one outcome (even if it's not technically true).

I also try to avoid passing null as a parameter since it means potentially all code further down the stack might need to handle that param being null.

Or:

public TestCleanupSystems() : this(Contexts.SharedInstance.Test) { }

*wrote those two on my phone, so they may not compile as is😁

This would be reasonable, but won't compile since Contexts.SharedInstance.Test isn't a compile time constant.

[jeffcampbellmakesgames]

Merged to develop in 265d6a7eb301f120b4985ef7ba9b30b096e299df.