ComponentSystemGroup needs a design review

I just finished updating all the DOTS packages, which for me meant finally updating my Unity version since there is a new minimum. While I am excited to see what the new ISystemBase has to offer performance-wise, I am also afraid of some of the compromises it is making that don’t play well with the common use cases. My hope is that if I raise these concerns now, they can be addressed before the old working functionality disappears and I’m left with tedius tasks and dead ends.

Concern 1: Manual system registration is not evil
I get it. It is super annoying when you write a system, enter play mode, and nothing happens because the system was never registered. So having systems automatically be discovered seems to be a reasonable solution and we should discourage if not prevent manual system registration, right?

Wrong!

So besides the modding use case, I often find myself in the middle of writing a system when I want to enter play mode to observe pre-existing behavior. The problem is my half-written system corrupts data, has memory leak issues, and causes the safety system to freak out. The code isn’t ready. And naturally, I don’t just want to comment out the entire file just to enter play mode. Manual system registration solves this. I don’t care whether the registration happens in the code or in the editor, I just want to have my manual system registration workflow. It is not like it is a new concept either. When you wrote a MonoBehaviour, you still had to attach it to a GameObject for it to do anything. Same idea here.

Concern 2: Manual system ordering is a necessary option
Not everyone likes to fully understand their dependencies or care what all is going on. And for that, the attribute workflow is for them. That’s fine. But for those who do understand and care, forcing all systems to be sorted by Unity’s automatic system sorter is an absolute nightmare.

  1. Automatic system order is trial and error.
    Did you get your attributes correct? Well enter playmode, look at the system order, and check the console. Not quite right? Figure out what system you were missing in your dependency ordering, modify the code, recompile, and then enter play mode again. Is the system order correct this time? No? Do it again. Eventually you will get it right, but it is a tedious process.

  2. Automatic system order couples systems which don’t need to be coupled.
    Yes. It is exactly that. SystemB shouldn’t have to know anything about SystemA or SystemC, but because the project requires SystemB to run after SystemA and before SystemC, that logic goes into SystemB. And now the code in SystemB cannot be dropped into another project without SystemA and SystemC or modifying SystemB’s code, all because of ordering attributes that aren’t really SystemB’s responsibility. Rather, some higher-level logic - whether generated in the editor or as a separate class - should be responsible for this project-specific ordering logic so that SystemB’s logic can be resusable.

  3. Unity will never know the optimal order of my systems. Only I know from the magic profiler!
    And consequently, I need to order the systems appropriately. Let’s say I have systems A, B, C, and D inside a ComponentSystemGroup. They all read the same data, but they all write to different memory. A, B, and C schedule lots of smaller jobs that are easily parallelized and can be spread across the different threads. However, D is responsible for generating a complex hierarchy using statistical analysis procedures. This one job has to be single-threaded, and it takes 7 milliseconds to complete. With automatic system ordering, D might be scheduled last. Consequently, the Job System will schedule the jobs of A, B, and C, and D will get stuck in the queue. Specifying ordering attributes is deceptive, because A, B, and C don’t require D to run first. Plus adding the attributes introduces another round of trial and error that ruins that optimization groove. It is a heck of a lot easier to cut and paste the system in a manually ordered list. On top of that, I can keep a couple different orders on hand and switch between them to find out what is the most optimal. With attributes I would have to do some fancy git branching to do that. I don’t like that.

Concern 3: The Add/Remove mechanism is too fragile
I believe there is only one use case where you should ever be manually adding and removing systems at runtime, and that is when loading and unloading mods. The only problem is, if you were to unload a mod, and then re-add it (perhaps due to some mod refresh mechanism), what ends up happening is that the systems when removed get put on a queue, then the systems get added and realize they already exist and so don’t do anything. Then when the ComponentSystemGroup updates, it goes through the queue and removes the systems. And now the refreshed mod is broken because its systems aren’t being updated. What you really need is a SystemRegistrationCommandBuffer of some sort.

Those are my concerns. I have ideas as to how to solve all these problems if anyone is curious. Please don’t leave me and everyone else who cares about this hanging!

7 Likes

Concern 1 I agree with, except unless something has changed it’s very easy to implement in a custom bootstrap and I think all projects past concept will probably be implementing their own bootstrap logic anyway so I’m not sure it’s a big deal that automatic is default.

Concern 2 - I agree with this. You could previous do this very easily overriding SortSystemUpdateList but it’s been deprecated for some reason.

Concern 3 - don’t know enough the use case

For our own systems we stopped using built in ordering over a year ago. You want to reason about ordering in a single place not scattered all over. So explicit registration I’m all for.

Our biggest pain point with ordering is internal hard coded ordering. Custom bootstraps can’t solve that. Hardcoded player loop entries is an issue also. I should not have to use a custom bootstrap for the default worlds. Unity has a good deal of internal logic involving the default worlds that I shouldn’t need to be concerned with. Additional worlds fine.

The default player loop entries and built in system placement is problematic generally. I’ll go as far as to say it’s untenable for almost any real game that uses a lot of DOTS features. I wish they would deep dive on this because I think what does work best might not be what they are thinking. Like what we do is we basically run two loops and render in the second which is always most of a frame behind gamelogic stuff. It works far better then I had imagined and let us work around all the hardcoded sync points in Unity feature packages.

  1. It seems like you have all the controls to do this today via disable auto creation or manual bootstrap.
    See below for an RFC, that might make this a bit more powerful.

  2. So you want full control. Would just having this API solve your problem:
    bool ComponentSystemGroup.EnableAutomaticSorting;
    void ComponentSystemGroup.Sort();

  3. Supporting add / remove / add seems like simply a bug in the
    AddSystemToUpdateList logic, that we should fix.

This is a new API we are currently looking at implementing, introducing the concept of features. It makes it easier to group functionality together and filter it out based on your own rules.

7 Likes

So how would you move the transform system, rendering, physics, to completely different system groups? I can disable the entire system group like simulation and then manually tick systems from my own system. There has to be a better way that doesn’t bypass the ordering system entirely.

If you want to control ordering of systems, shouldn’t you just completely control every single system order and essentially override the bootstrap and explicitly write out both which systems end in which group and which order.

It sounds like you want to mix things, automatic ordering & filtering with manual ordering. I am not sure how that would work.

Just from a quick glance I like the idea of features. Seems like it’d make writing libraries a bit easier.

1 Like

Alright. I think the best way to expose my use case is to detail out exactly what I am doing and what is working for me right now that I am worried is going to be completely shut down in the near future. Keep in mind, I do expect my solution to evolve and grow in complexity as I challenge it to more use cases.

So first off, I do use an ICustomBootstrap. Right now it looks like this:

using System;
using System.Collections.Generic;
using Latios;
using Unity.Collections;
using Unity.Entities;

public class LatiosBootstrap : ICustomBootstrap
{
    public bool Initialize(string defaultWorldName)
    {
        var world                             = new LatiosWorld(defaultWorldName);
        World.DefaultGameObjectInjectionWorld = world;

        var initializationSystemGroup = world.GetExistingSystem<InitializationSystemGroup>();
        var simulationSystemGroup     = world.GetExistingSystem<SimulationSystemGroup>();
        var presentationSystemGroup   = world.GetExistingSystem<PresentationSystemGroup>();
        var systems                   = new List<Type>(DefaultWorldInitialization.GetAllSystems(WorldSystemFilterFlags.Default));

        systems.RemoveSwapBack(typeof(InitializationSystemGroup));
        systems.RemoveSwapBack(typeof(SimulationSystemGroup));
        systems.RemoveSwapBack(typeof(PresentationSystemGroup));

        BootstrapTools.InjectUnitySystems(systems, world, simulationSystemGroup);
        BootstrapTools.InjectRootSuperSystems(systems, world, simulationSystemGroup);

        initializationSystemGroup.SortSystemUpdateList();
        simulationSystemGroup.SortSystems();
        presentationSystemGroup.SortSystems();

        ScriptBehaviourUpdateOrder.UpdatePlayerLoop(world);
        return true;
    }
}

The first thing I am doing is creating a custom subclass instance of World that has a few additional features. This World’s constructor creates LatiosWorldInitializationSystemGroup, and the default SimulationSystemGroup and PresentationSystemGroup.

LatiosWorldInitializationSystemGroup has been broken and patched with the last two Entities releases, so the code has deteriorated a bit.

using System.Collections.Generic;
using System.Linq;
using Unity.Entities;

namespace Latios.Systems
{
    public class LatiosWorldInitializationSystemGroup : InitializationSystemGroup
    {
        private SceneManagerSystem                   m_sceneManager;
        private MergeGlobalsSystem                   m_mergeGlobals;
        private DestroyEntitiesOnSceneChangeSystem   m_destroySystem;
        private ManagedComponentsReactiveSystemGroup m_cleanupGroup;
        private LatiosSyncPointGroup                 m_syncGroup;

        private BeginInitializationEntityCommandBufferSystem m_beginECB;
        private EndInitializationEntityCommandBufferSystem   m_endECB;

        protected override void OnCreate()
        {
            UseLegacySortOrder = true;

            base.OnCreate();
            m_sceneManager  = World.CreateSystem<SceneManagerSystem>();
            m_mergeGlobals  = World.CreateSystem<MergeGlobalsSystem>();
            m_destroySystem = World.CreateSystem<DestroyEntitiesOnSceneChangeSystem>();
            m_cleanupGroup  = World.CreateSystem<ManagedComponentsReactiveSystemGroup>();
            m_syncGroup     = World.GetOrCreateSystem<LatiosSyncPointGroup>();
        }

        public override void SortSystemUpdateList()
        {
            //m_destroySystem does not have a normal update, so don't add it to the list.

            //Remove from list to add back later.
            m_beginECB = World.GetExistingSystem<BeginInitializationEntityCommandBufferSystem>();
            m_endECB   = World.GetExistingSystem<EndInitializationEntityCommandBufferSystem>();
            RemoveSystemFromUpdateList(m_beginECB);
            RemoveSystemFromUpdateList(m_endECB);
            RemoveSystemFromUpdateList(m_sceneManager);
            RemoveSystemFromUpdateList(m_mergeGlobals);
            RemoveSystemFromUpdateList(m_cleanupGroup);
            RemoveSystemFromUpdateList(m_syncGroup);
            base.SortSystemUpdateList();
            var systems = Systems.ToList();
            systems.Remove(m_beginECB);
            systems.Remove(m_endECB);
            // Re-insert built-in systems to construct the final list
            var finalSystemList = new List<ComponentSystemBase>(5 + systems.Count);
            finalSystemList.Add(m_beginECB);
            finalSystemList.Add(m_sceneManager);

            //Todo: MergeGlobals and CleanupGroup need to happen after scene loads and patches but before user code.
            //However, there has to be a cleaner way to do this that doesn't make so many assumptions
            int index;
            for (index = systems.Count - 1; index >= 0; index--)
            {
                if (systems[index].GetType().Namespace.Contains("Unity"))
                    break;
            }

            foreach (var s in systems)
            {
                finalSystemList.Add(s);

                if (index == 0)
                {
                    finalSystemList.Add(m_mergeGlobals);
                    finalSystemList.Add(m_cleanupGroup);
                    finalSystemList.Add(m_syncGroup);
                }
                index--;
            }
            finalSystemList.Add(m_endECB);
            var systemsToUpdate = m_systemsToUpdate;
            systemsToUpdate.Clear();
            /*foreach (var s in Systems)
               {
                RemoveSystemFromUpdateList(s);
               }
               base.SortSystemUpdateList();*/
            foreach (var s in finalSystemList)
            {
                AddSystemToUpdateList(s);
            }
        }

        protected override void OnUpdate()
        {
            foreach (var sys in m_systemsToUpdate)
            {
                sys.Update();
            }
        }
    }

    [UpdateInGroup(typeof(LatiosWorldInitializationSystemGroup))]
    public class LatiosSyncPointGroup : ComponentSystemGroup
    {
    }
}

There are a couple of specific rules:

  1. BeginInitializationEntityCommandBufferSystem must always run first. This is my preferred ECB system for pretty much everything.
  2. SceneManagerSystem must run immediately afterwards, before any SubScene logic. This system is responsible for changing the actual scene (not the subscene).
  3. DestroyEntitiesOnSceneChangeSystem must not be run in a ComponentSystemGroup. It reacts to scene changes, and usually ends up running inside the SceneManagerSystem’s callstack.
  4. ManagedComponentsReactiveSystemGroup contains a bunch of concrete instances of reactive generic systems which maintain an Entity-NativeContainer mapping. It must run after all entities are populated from subscenes but before any other custom user code.
  5. LatiosSyncPointGroup was a group I had to add to get things to work in 0.10.0. Aside from the first frame of a scene, all sync points happen inside the InitializationSystemGroup.
    The resulting system order looks like this:
    5905388--630428--upload_2020-5-27_8-11-50.png

After creating the world, back in the bootstrap, I get all systems and remove the Initialization, Simulation, and Presentation systems from the list. Then I make these two calls to a custom static class:

BootstrapTools.InjectUnitySystems(systems, world, simulationSystemGroup);
BootstrapTools.InjectRootSuperSystems(systems, world, simulationSystemGroup);

The first call injects all Unity systems into the systems groups based on attributes. It also reports that CompanionGameObjectUpdateTransformSystem, EditorCompanionGameObjectUpdateSystem, and CompanionGameObjectUpdateSystem are missing namespaces.
The second call injects top level user ComponentSystemGroups. These groups also use attribute-based ordering, and their sole purpose is to position user code points relative to existing Unity systems. I only do this because Unity’s systems use attribute ordering, but the complexity hasn’t gotten out of hand yet. I have a few other injection algorithms based on different criteria, though I haven’t needed them personally yet.

Now what happens in the RootSuperSystem is the interesting part. Here’s what that looks like:

using System;
using System.ComponentModel;
using Unity.Entities;

namespace Latios
{
    public abstract class RootSuperSystem : SuperSystem
    {
        protected override void OnUpdate()
        {
            if (ShouldUpdateSystem())
                base.OnUpdate();
        }
    }

    public abstract class SuperSystem : ComponentSystemGroup, ILatiosSystem
    {
        public LatiosWorld latiosWorld { get; private set; }

        public ManagedEntity sceneGlobalEntity => latiosWorld.sceneGlobalEntity;
        public ManagedEntity worldGlobalEntity => latiosWorld.worldGlobalEntity;

        public FluentQuery Fluent => this.Fluent();

        public virtual bool ShouldUpdateSystem()
        {
            return Enabled;
        }

        [EditorBrowsable(EditorBrowsableState.Never)]
        protected sealed override void OnCreate()
        {
            base.OnCreate();
            if (World is LatiosWorld lWorld)
            {
                latiosWorld = lWorld;
            }
            else
            {
                throw new InvalidOperationException("The current world is not of type LatiosWorld required for Latios framework functionality.");
            }
            CreateSystems();
        }

        [EditorBrowsable(EditorBrowsableState.Never)]
        protected override void OnUpdate()
        {
            foreach (var sys in Systems)
            {
                try
                {
                    if (sys is ILatiosSystem latiosSys)
                    {
                        if (latiosSys.ShouldUpdateSystem())
                        {
                            sys.Enabled = true;
                            sys.Update();
                        }
                        else
                        {
                            sys.Enabled = false;
                        }
                    }
                    else
                    {
                        sys.Update();
                    }
                }
                catch (Exception e)
                {
                    UnityEngine.Debug.LogException(e);
                }

                if (World.QuitUpdate)
                    break;
            }
        }

        [EditorBrowsable(EditorBrowsableState.Never)]
        public sealed override void SortSystemUpdateList()
        {
            // Do nothing.
        }

        public EntityQuery GetEntityQuery(EntityQueryDesc desc) => GetEntityQuery(new EntityQueryDesc[] { desc });

        #region API

        protected abstract void CreateSystems();

        public ComponentSystemBase GetOrCreateAndAddSystem(Type type)
        {
            var system = World.GetOrCreateSystem(type);
            AddSystemToUpdateList(system);
            return system;
        }

        public T GetOrCreateAndAddSystem<T>() where T : ComponentSystemBase
        {
            var system = World.GetOrCreateSystem<T>();
            AddSystemToUpdateList(system);
            return system;
        }

        public void SortSystemsUsingAttributes()
        {
            base.SortSystemUpdateList();
        }

        #endregion API
    }
}

A SuperSystem defines an abstract class method called CreateSystems, and a utility method called GetOrCreateAndAddSystem which is usually called from inside CreateSystems implementations. The order of the systems in the group is the order this method gets called, so I manually register and order a new system with a single line of code. I can also inject an existing system into several different groups. If the system was already created, it gets reused rather than create a new instance. This all works for me because I heavily decouple my systems such that the only thing they depend on are ECB systems which have already been created by this point.

So RootSuperSystems create instances of SuperSystems, which in turn create instances of more SuperSystems and SubSystems (custom SystemBase). This leads to a top-down hierarchical creation of all my systems.
Some example snippets from my game:

[UpdateInGroup(typeof(Latios.Systems.LatiosSyncPointGroup))]
    public class LsssInitializationRootSuperSystem : RootSuperSystem
    {
        protected override void CreateSystems()
        {
            GetOrCreateAndAddSystem<GameplaySyncPointSuperSystem>();

            GetOrCreateAndAddSystem<TransformSystemGroup>();
            GetOrCreateAndAddSystem<CompanionGameObjectUpdateTransformSystem>();  //Todo: Namespace
        }
    }

    [UpdateInGroup(typeof(SimulationSystemGroup))]
    [UpdateBefore(typeof(TransformSystemGroup))]
    public class LsssPreTransformRootSuperSystem : RootSuperSystem
    {
        protected override void CreateSystems()
        {
            GetOrCreateAndAddSystem<PlayerInGameSuperSystem>();
            GetOrCreateAndAddSystem<AdvanceGameplayMotionSuperSystem>();
        }
    }

    [UpdateInGroup(typeof(SimulationSystemGroup))]
    [UpdateAfter(typeof(TransformSystemGroup))]
    public class LsssPostTransformRootSuperSystem : RootSuperSystem
    {
        protected override void CreateSystems()
        {
            GetOrCreateAndAddSystem<UpdateTransformSpatialQueriesSuperSystem>();
            GetOrCreateAndAddSystem<AiSuperSystem>();
            GetOrCreateAndAddSystem<ProcessGameplayEventsSuperSystem>();
            GetOrCreateAndAddSystem<GraphicsTransformsSuperSystem>();
        }
    }
public class ProcessGameplayEventsSuperSystem : SuperSystem
    {
        protected override void CreateSystems()
        {
            GetOrCreateAndAddSystem<ShipVsBulletDamageSystem>();
            GetOrCreateAndAddSystem<ShipVsShipDamageSystem>();
            GetOrCreateAndAddSystem<ShipVsExplosionDamageSystem>();
            GetOrCreateAndAddSystem<ShipVsWallDamageSystem>();
            GetOrCreateAndAddSystem<BulletVsWallSystem>();
            GetOrCreateAndAddSystem<CheckSpawnPointIsSafeSystem>();
            GetOrCreateAndAddSystem<FireGunsSystem>();
            GetOrCreateAndAddSystem<TravelThroughWormholeSystem>();
            GetOrCreateAndAddSystem<UpdateTimeToLiveSystem>();
            GetOrCreateAndAddSystem<DestroyShipsWithNoHealthSystem>();
            GetOrCreateAndAddSystem<SpawnShipsDequeueSystem>();
        }
    }

Now the other aspect that I don’t have implemented is mod support. Mods would most likely be inserted into a few select ComponentSystemGroups (some customized subclass of SuperSystem in my case). Now unlike my core game code, I don’t have any knowledge of what order mods need to run in at compile time. Some mods will have optional compatibility support for other mods, and will provide a set of rules for their system order when those other mods are present. So I would need to read the rules from all the mod files, compute a new system order, and be able to set that to the ComponentSystemGroup. It is not a big deal if that really just becomes adding each system one-by-one from the pre-generated order, as long as some API exists that allows me to completely clear out all systems in a group.

Kudos if you made it through all that!

1 Like

Like new Features Idea, we have something like this in our project.

One thing that is need to be supported is ability to insert additional root system groups or may be just add default ones so we dont need to create our own.

Our project use DOTS for logic and old GameObject approach for rendering game. This means than MonoBehaviour.Update is presentation for us but it invokes before SimulationSystemGroup!!!

Because of this we end up creating EarlySimulationSystemGroup and put all our systems in it just to have actual simulation updated before presentation.

May be Checkbox to move existing SimulationSystemGroup before MonoBehaviour.Update will be enough in our case :slight_smile:

I’m not asking for manual ordering per say. I want to leverage the existing ordering/placement logic not wholesale replace it.

I’m asking for a better implementation. To start with decouple attributes. That doesn’t solve all problems but it does solve a pretty big surface area and provides a better foundation I think to build on.

Off the top of my head:

Abstract out the data into it’s own api. Use the attribute derived classes I can’t think of a reason to ditch those. But have an api that gets/sets those as the core api. This abstraction decouples attributes so they are just one possible source. It also separates concerns better, ECS logic isn’t aware of attributes per say.

ECS logic doesn’t change much. It just gets it’s data from a central source.

Arbitrary placement is sort of a different problem. But I think one the core api should also handle.

Big picture I think the custom bootstraps are fine as an interim solution. More elegant solutions I think should be recognized as obviously doable, even if they take some time to evolve.

I think we need 2 complementary ways for sorting systems.

Current attribute based for direct code dependency on systems order and another manual (editor) one (like for mono behaviors) to order systems that dont have any actual dependencies but project have some dependencies between systems that even dont know about each other. Second sorting way can not hurt dependencies set from attributes.

This way package authors create logical dependencies in code and package customers can additionally order systems for exact game requirements.

I spent some extra time thinking about this. And to answer your question on (2), no it would not quite solve my problem.

  1. I need to be able to see what systems are in a ComponentSystemGroup so that I can remove all of them and replace them with a new list.
  2. I need to be able to manually iterate through all the systems in a custom OnUpdate to call ShouldUpdateSystem on systems which support that functionality. (Built-in support for this functionality in the form of a virtual method or an ICustomUpdateCriteria interface would be equally sufficient.)
  3. I need to build the system hierarchy in a top-down fashion which currently uses GetOrCreateSystem.

For (1) and (3), I can somewhat solve by keeping an extra copy of the system list in memory and pairing each system with a builder that can split system discovery and group ordering in two phases. It is ugly, but I’ve worked around worse.

However, (2) does not have a workaround other than to use the legacy system ordering. That’s unfortunate, as it is a critical optimization for game state control and UI management.