0

I have next structure:

TestFacade is a GameObject that gets pooled.

    public class TestFacade : MonoBehaviour, IPoolable<IMemoryPool>, IDisposable
    {
        public class Factory : PlaceholderFactory<TestFacade> { }

        public class Pool : MonoPoolableMemoryPool<IMemoryPool, TestFacade> { }

        public Guid ID;

        private TestComponent _component;
        private TestRegistry _registry;
        private IMemoryPool _pool;

        [Inject]
        public void Construct(TestRegistry registry)
        {
            _component = new TestComponent(this);
            _registry = registry;
            ID = Guid.NewGuid();
        }

        public void OnSpawned(IMemoryPool pool)
        {
            _component.TestMethod();
            _pool = pool;
            _registry.Add(ID, this);
        }

        public void OnDespawned()
        {
            _pool = null;
            _registry.Remove(ID);
        }

        public void Dispose()
        {
            _pool.Despawn(this);
        }
    }

Logicaly, TestComponent doesn't exist without TestFacade, but I couldn't "bind" it (don't know how), so I create it myself in TestFacade making it coupled to TestFacade.

    public class TestComponent
    {
        private readonly TestFacade _testFacade;

        public TestComponent(TestFacade testFacade)
        {
            _testFacade = testFacade;
        }

        public void TestMethod()
        {
            Debug.Log(_testFacade.ID);
            // Do something
        }
    }

TestFacade fill this registry to keep a record of spawned TestFacade's.

    public class TestRegistry
    {
        private readonly Dictionary<Guid, TestFacade> _objects;

        public IReadOnlyDictionary<Guid, TestFacade> Objects => _objects;

        public TestRegistry()
        {
            _objects = new Dictionary<Guid, TestFacade>();
        }

        public void Add(in Guid id, TestFacade test)
        {
            _objects.Add(id, test);
        }

        public void Remove(in Guid id)
        {
            _objects.Remove(id);
        }
    }

TestSpawner handles spawning/despawning of TestFacade using TestRegistry and TestFacade.Factory (it seems logical, right?).

    public class TestSpawner : IInitializable, IDisposable
    {
        private readonly TestFacade.Factory _testFactory;
        private readonly TestRegistry _registry;

        public TestSpawner(TestFacade.Factory testFactory, TestRegistry registry)
        {
            _testFactory = testFactory;
            _registry = registry;
        }

        public ref Guid Spawn()
        {
            var testFacade = _testFactory.Create();

            return ref testFacade.ID;
        }

        public void Despawn(in Guid id)
        {
            _registry.Objects[id].Dispose();
        }

        public void DespawnAll()
        {
            foreach (var testFacade in _registry.Objects.Values)
            {
                testFacade.Dispose();
            }
        }

        public void Initialize()
        {
            // 1
        }

        public void Dispose()
        {
            // 2
        }

And this is just a test script. All it does is requests to spawn new TestFacade every second using injected TestSpawner. Is this right usage of IoC principle and DI Container?

    public class TestScript : MonoBehaviour
    {
        [Inject]
        private TestSpawner _testSpawner;

        private Guid _id;

        private bool set;

        private void Start()
        {
            StartCoroutine(RemoveEachTime());
        }

        private IEnumerator RemoveEachTime()
        {
            yield return new WaitForSeconds(1f);

            while (true)
            {
                if (set)
                {
                    _testSpawner.Despawn(in _id);
                    set = false;
                }
                else
                {
                    _id = _testSpawner.Spawn();
                    set = true;
                }

                yield return new WaitForSeconds(1f);
            }
        }
    }

I have two scenes: Scene1 and Scene2.

Scene1 simply loads Scene2.

Scene2 has SceneContext where I install all this logic.

    public class TestSceneInstaller : MonoInstaller
    {
        [Inject] private Settings _settings;

        public override void InstallBindings()
        {
            Container.Bind<TestRegistry>().AsSingle();
            Container.BindInterfacesAndSelfTo<TestSpawner>().AsSingle();

            Container.BindFactory<TestFacade, TestFacade.Factory>()
                .FromPoolableMemoryPool<TestFacade, TestFacade.Pool>(poolBinder => poolBinder
                    .WithInitialSize(2)
                    .FromComponentInNewPrefab(_settings.TestFacadePrefab)
                    .UnderTransformGroup("TestObjects"));
        }

        [Serializable]
        public class Settings
        {
            public GameObject TestFacadePrefab;
        }
    }

I've decided to use Memory Profiler to check what happens in memory.

My logic is that:

  1. Scene1 loads Scene2 and unloads itself
  2. Scene2 installs all this logic
  3. TestScript runs some logic
  4. Scene2 loads Scene1 and unloads itself
  5. All TestFacade's GameObjects got destroyed, which means
    TestComponent gets destroyed too because it is not referenced by TestFacade anymore (right?). And basically whole structure should be destroyed? Nobody references TestSpawner so it should also go. Same goes for TestRegistry.

But, when I load Scene2 again I notice that now I have 2 of everything.

Same happens 3rd time...

Did I messed something up? Is the way I'm thinking wrong?

Unity leak analysis

Memory Profiler tells me that TestFacade is a Leaked Managed Shell.

enter image description here

P.S. Also, I wouldn't mind any feedback regarding this structure, how I handle everything, e.g., usage of Guid struct as ID and passing it by ref to not copy it over and over again, etc.

stroibot
  • 808
  • 1
  • 11
  • 25
  • Don't forget you are coding in Unity which like Unreal Engine is for making _games_ where performance is paramount; the last thing you want to do is follow _white-collar-business-practices_ where people spend more time worrying about dependency injection and writing unit tests than working on the problem at hand. Both of these technologies use excessive reflection and/or dynamic proxies both of which will hurt. Check out [this](https://forum.unity.com/threads/design-patterns-singleton-and-dependency-injection.547033/) thread over at the Unity forums. The comments by **ippdev** are _gold!_ –  Nov 10 '22 at 21:49
  • Why the need for `ref` in the method `public ref Guid Spawn()`? Why would you want to allow callers to modify `testFacade.ID`? –  Nov 10 '22 at 22:00
  • As for the leak, do you need a `MonoBehaviour.OnDisable()` somewhere? Also, as for the _"leak",_ Unity is a **CLR Host** whereby it can and will **zap** out the **Primary App Domain** along with all your objects **including singletons** not just during runtime but also whilst using the **Editor**. See also _[Details of disabling Domain and Scene Reload](https://docs.unity3d.com/Manual/ConfigurableEnterPlayModeDetails.html)_. What evidence that you have for a leak over the long-term? Is it cumulative? –  Nov 10 '22 at 22:09
  • @MickyD, I checked out that thread, I was 50/50 use DI or not before and 50/50 after About `ref`, no, I don't want anyone to modify `ID`, buuuut I also don't want to copy it every time – stroibot Nov 10 '22 at 22:29
  • Also good point on Domain Reload, let me check it in the build – stroibot Nov 10 '22 at 22:29
  • _"copy it every time"_ - `Guid` is a struct, thus a _value type_ and therefor much faster than reference types due to their ability to move about _on the stack_. :) –  Nov 10 '22 at 22:31
  • 1
    Well, I just checked it on the actual build and I was right. Memory Profiler tells me that `TestFacade` is a Leaked Managed Shell. [Here's a screenshot of snapshot](https://i.imgur.com/OoZU7TR.png) – stroibot Nov 10 '22 at 23:32
  • _"Memory Profiler tells me that TestFacade is a Leaked Managed Shell"_ - oh no!! I hope you don't mind but I have edited your question to include that little nugget. –  Nov 11 '22 at 01:09
  • Oh I wonder if its because the type is being whacked in a memory pool via `FromPoolableMemoryPool<>`. What happens if you don't use a pool? –  Nov 11 '22 at 01:11
  • Well the thing is I need it to be a pool lol – stroibot Nov 11 '22 at 11:33
  • The thing about pools is (and this applies to the ones in .NET), is that when you request an object instance, the pool looks for a pre-existing one and returns that, otherwise it creates a new one. When you no longer need it, you hand it back. However, and this depends on the type being whacked in the pool, care must be performed to ensure any extra data is cleared on your part. In this case `TestFacade` is a `MonoBehaviour` so technically you need to tell Unity that you "destroyed" this object and want Unity to clean-up its part. It's like calling `Font.Dispose` but you attempt to use it –  Nov 11 '22 at 12:16
  • ...also the notion of a _recyclable_ `MonoBehaviour` might be problematic. I notice Zeninject hasn't been updated since the last major 2018 update. Project looks pretty dead. :( –  Nov 11 '22 at 12:18
  • Ok, just checked through Zenject's Memory Pool Monitor (didn't know it existed) and indeed the pool persists. Ok, now I guess I need to rephrase the question to "How to destroy/clear/release pool in Zenject". I tried searching this info but got nothing. I really don't get it why it doesn't go with a scene and also I noticed that `TestFacade.Poo.MonoPoolableMemoryPool.OnDestroyed` doesn't get called when object is destroyed! – stroibot Nov 11 '22 at 14:56
  • That’s a shame. This is not the first buggy and abandoned “Unity compatible” GitHub project I’ve seen referenced here on SO. As for DI, as mentioned they have no place in games and there are better ways to do things. Also, Unity dislikes anything that creates objects without it knowing (outside of typical script flow). –  Nov 11 '22 at 15:02

0 Answers0