Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.
  2. We have updated the language to the Editor Terms based on feedback from our employees and community. Learn more.
    Dismiss Notice

Question What is better in terms of performance?

Discussion in 'Scripting' started by MartinMa_, Apr 6, 2023.

  1. MartinMa_

    MartinMa_

    Joined:
    Jan 3, 2021
    Posts:
    455
    Hello guys I am thinking about 2 options, both work but I am not sure what is better approach.

    1 . Create instance of class and then call it from different class.

    Code (CSharp):
    1. public class ExampleInstanceProvider : MonoBehaviour
    2. {
    3.     public static ExampleInstanceProvider instance { get; set; }
    4.     void Start()
    5.     {
    6.         instance = this;
    7.     }
    8. }
    9. public class ExampleInstanceConsumer : MonoBehaviour
    10. {
    11.     private ExampleInstanceProvider instance;
    12.     void Start()
    13.     {
    14.         instance= ExampleInstanceProvider.instance;
    15.     }
    16. }
    2. Create some kind of Manager what take care of all Managers and just inherit it in each new manager

    Code (CSharp):
    1. public class CoreManager : MonoBehaviour
    2. {
    3.     [SerializeField] public ControlsManager controlsManager;
    4.     [SerializeField] public UiManager uiManager;
    5.     [SerializeField] public ResourcesManager resourcesManager;
    6.     [SerializeField] public CameraManager cameraManager;
    7.     [SerializeField] public ActionsManager actionsManager;
    8.     [SerializeField] public TimeManager timeManager;
    9.     [SerializeField] public EventsManager eventsManager;
    10.     [SerializeField] public GridManager3D gridManager3d;
    11. }
    Code (CSharp):
    1. public abstract class BaseAbstractBaseManager : MonoBehaviour
    2. {
    3.     private CoreManager _coreManager;
    4.     protected ControlsManager ControlsManager;
    5.     protected UiManager UIManager;
    6.     protected ResourcesManager ResourcesManager;
    7.     protected ActionsManager ActionsManager;
    8.     protected TimeManager TimeManager;
    9.     protected EventsManager EventsManager;
    10.     protected CameraManager CameraManager;
    11.     protected GridManager3D GridManager3D;
    12.  
    13.     private void Awake()
    14.     {
    15.         InitComponents();
    16.     }
    17.  
    18.     private void InitComponents()
    19.     {
    20.         _coreManager = GetComponentInParent<CoreManager>();
    21.         ControlsManager = _coreManager.controlsManager;
    22.         UIManager = _coreManager.uiManager;
    23.         ResourcesManager = _coreManager.resourcesManager;
    24.         ActionsManager = _coreManager.actionsManager;
    25.         TimeManager = _coreManager.timeManager;
    26.         EventsManager = _coreManager.eventsManager;
    27.         CameraManager = _coreManager.cameraManager;
    28.         GridManager3D = _coreManager.gridManager3d;
    29.     }
    30. }
    I really like second option because in all classes what inherit from BaseAbstractManager i have available every Managers what I have in game, but I am not sure if it is a good way in terms of performance.
     
  2. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    36,963
    DO NOT OPTIMIZE "JUST BECAUSE..." If you don't have a problem, DO NOT OPTIMIZE!

    If you DO have a problem, there is only ONE way to find out. Always start by using the profiler:

    Window -> Analysis -> Profiler

    Failure to use the profiler first means you're just guessing, making a mess of your code for no good reason.

    Not only that but performance on platform A will likely be completely different than platform B. Test on the platform(s) that you care about, and test to the extent that it is worth your effort, and no more.

    https://forum.unity.com/threads/is-...ng-square-roots-in-2021.1111063/#post-7148770

    Remember that optimized code is ALWAYS harder to work with and more brittle, making subsequent feature development difficult or impossible, or incurring massive technical debt on future development.

    Notes on optimizing UnityEngine.UI setups:

    https://forum.unity.com/threads/how...form-data-into-an-array.1134520/#post-7289413

    At a minimum you want to clearly understand what performance issues you are having:

    - running too slowly?
    - loading too slowly?
    - using too much runtime memory?
    - final bundle too large?
    - too much network traffic?
    - something else?

    If you are unable to engage the profiler, then your next solution is gross guessing changes, such as "reimport all textures as 32x32 tiny textures" or "replace some complex 3D objects with cubes/capsules" to try and figure out what is bogging you down.

    Each experiment you do may give you intel about what is causing the performance issue that you identified. More importantly let you eliminate candidates for optimization. For instance if you swap out your biggest textures with 32x32 stamps and you STILL have a problem, you may be able to eliminate textures as an issue and move onto something else.

    This sort of speculative optimization assumes you're properly using source control so it takes one click to revert to the way your project was before if there is no improvement, while carefully making notes about what you have tried and more importantly what results it has had.
     
  3. chemicalcrux

    chemicalcrux

    Joined:
    Mar 16, 2017
    Posts:
    717
    Performance is probably irrelevant here. You will be much more interested in whatever is easiest to build upon!

    In my most recent game, I just wound up making a big ol' "GameController" singleton, which holds references to any other "controllers" (e.g. the InterfaceController that's in charge of turning different parts of the interface on and off).

    This is closer to your second option, although there's no inheritance. Everyone just asks the GameController where something is.
     
  4. MartinMa_

    MartinMa_

    Joined:
    Jan 3, 2021
    Posts:
    455
    Ok, thanks for your reply.I dont have any issues with this code, but i wanted to get opinion what is better. Create static instance or use Reference in Unity environment.Maybe it doesnt matter at all.
     
  5. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,384
    As was stated, this is less to do with performance, and more to do with design.

    OP's Option 1 is just the Singleton design pattern (as implemented the way is traditional in Unity).

    As well documented and well known pattern. This means most people know most of the pros and cons of the pattern. It also means that anyone tripping over your code will know exactly what you're doing here.

    Is it good? Eh, debatable. Singleton pattern is one of those patterns that is great for a small handful of references in a project that is rather static in its design (i.e. not maintained over many long years... which is like many games that don't have longterm player ecosystems). It lacks testability, but makes up for it in its quick and easy setup.

    Chemicalcrux's Solution is basically what is known as a "Service Locator" pattern where you have a global point of entry (in this case a singleton) that manages handing out references to all services/controllers within the project. It adds a small layer of testability to singleton since at setup of the service locator you can inject different versions of the services/controllers for testing purposes. Service locators are my preferred system when making video games... it IMO lands in between singleton and another pattern out there that's popular within the programmer zeitgeist. The sort of goldilocks of these designs.

    Which leads me to a pattern not talked about here... but I guarantee someone will pop in and mention (if not me). And also, the pattern is something I want to reference when I get to your option 2.

    Dependency Injection is a pattern where the dependencies... including services/controllers... are injected at the point of demand. They're treated as local members avoiding any of the pitfalls of globals (which you option 2 appears to be wanting to resolve). It's extremely testable. But many frameworks out there that intend to streamline the injection process add layers of complexity that obfuscate where data is coming from in your project.

    I have lengthy opinions about all 3 of these patterns... here is a very recent thread where I go on a tirade about them:
    https://forum.unity.com/threads/sin...ndency-injections-oh-my.1413726/#post-8884461

    OP's Option 2 is... to put it bluntly... weird.

    As was previously mentioned this is more about design. And design is more about making sure it's easy to use and "build upon".

    And... I'd argue this design is NOT that.

    For starters you have to inherit from BaseAbstractManager to actually tap into the ease of the design. And what if you can't always inherit from that? C# doesn't allow multiple inheritance.

    In an aside, I should also point out that by marking your Awake method private, if you then inherit from this and have a new Awake method in it, you block the running of Awake in BaseAbstractManager... thusly breaking its functionality just due to how Unity determines the message callback methods like Awake/Start/Update/etc. If you're creating classes that are intended to be inherited from like this, you should mark these common messages as 'protected virtual' so you don't accidentally replace the message callback handler.

    Now this inheritance issue where you can't inherit from anything else could be remedied by making your BaseAbstractManager a composited class. Maybe change its name to "ServiceProvider" and then during Awake of any MonoBehaviour that needs it you can get/add the component (add only if one doesn't exist... could be generalized as an extension method). But this doesn't fully remedy my other concerns with this design.

    My next concern is that this requires the injection of ALL managers rather than the managers needed. Why do I need to do that? Why create all that clutter in the interface of your class? This makes me think of the early design by Unity where MonoBehaviour has a property getter for all of the common component types (transform, camera, rigidbody, collider, etc) that really was just a forward to GetComponent of that type. Unity has since deprecated it because it's... bad design (as well as the approach was slow since caching references is more performant and it trained users to not cache references).

    Yet another concern is... this specific implementation forces it where all of your BaseAbstractManager are required to be a child of some GameObject that has CoreManager on it. This is extremely limiting.

    You have to spawn all objects that have a BaseAbstractManager on it under this thing. What if I forget to do this? What if I spawn a FX that requires access to the CameraManager and EventsManager so I have a 'MyFXController' that inherits from it... but I accidentally spawn it in the global space. I could just... not forget... but it's now a weak link in the design of things I have to remember. Or I have to create a spawn factory that handles this stuff for me.

    Also... what if you go to integrate with a library that doesn't like parenting because of how it works. I'm thinking of something like 'Netcode for Gameobjects' where the parent hierarchy of network spawned objects is VERY limited. This design would just fail outright when integrated with netcode for gameobjects.

    Also... speaking of. Back to my compositional paragraph above... since the design expects CoreManager to be in the parent hierarchy. Why not in any script that needs access to the managers just say:

    Code (csharp):
    1. private CoreManager _coreManager;
    2.  
    3. void Awake()
    4. {
    5.     _coreManager = this.GetComponentInParent<CoreManager>();
    6. }
    Then just accessing the members of coremanager directly rather than copying the references to local fields?

    Not that this still doesn't have the weird hierarchy design which is equally bad... but still... why the extra work?

    Which leads me back to @chemicalcrux's solution... which puts the service locator, which is what CoreManager is, into a global space. No requirement of it being in the parent hierarchy.

    What because it's no longer a global? Yeah... but no... it's still a global! Oh sure, it's not a static field somewhere out there. But that's language syntax. The reasons people dislike globals has nothing to do with the syntax. It has everything to do with the rigidity of the design. And this design is saying "The CoreManager must exist in a static location, that location being the parent of myself".

    Just make it a singleton and call it a day... or go full DI (in which case you're still going to have a service locator... see my link where I rant about DI and how at the end of the day most of the frameworks really are just glorified service locators).
     
    Yoreki, Sluggy and MartinMa_ like this.
  6. MartinMa_

    MartinMa_

    Joined:
    Jan 3, 2021
    Posts:
    455
    Thanks for your long explanation. All points all relevant I just find this not practical when I created some Scriptable Objects via static method, that is from my understanding called sooner then Awake and gg :).

    I will probably go with "Service Provider" this looks like best for my needs. I will just always repeat initialization I was trying to avoid code duplication that is why I tried to inherit it.
     
  7. MartinMa_

    MartinMa_

    Joined:
    Jan 3, 2021
    Posts:
    455
    Hello again, so I still playing with this and I think this could work in most of your points what you mentioned above.

    Core Manager:
    Code (CSharp):
    1. public class CoreManager : MonoBehaviour
    2. {
    3.     [SerializeField] private ControlsManager controlsManager;
    4.     [SerializeField] private UiManager uiManager;
    5.     [SerializeField] private ResourcesManager resourcesManager;
    6.     [SerializeField] private CameraManager cameraManager;
    7.     [SerializeField] private ActionsManager actionsManager;
    8.     [SerializeField] private TimeManager timeManager;
    9.     [SerializeField] private EventsManager eventsManager;
    10.     [SerializeField] private GridManager3D gridManager3d;
    11.  
    12.  
    13.     public static CoreManager instance { get; private set; }
    14.  
    15.     private void Awake()
    16.     {
    17.         instance = this;
    18.     }
    19.  
    20.     public T GetCoreComponent<T>()
    21.     {
    22.         foreach (var field in GetType().GetFields(
    23.                      BindingFlags.NonPublic |
    24.                      BindingFlags.Instance))
    25.         {
    26.             if (field.FieldType == typeof(T))
    27.             {
    28.                 return (T)field.GetValue(this);
    29.             }
    30.         }
    31.  
    32.         throw new MissingReferenceException($"Class {typeof(T)} is not referenced in CoreManager.");
    33.     }
    Reference consumer:
    Code (CSharp):
    1. public class UiManager : MonoBehaviour
    2. {
    3.     [SerializeField] private DebugDisplayComponent debugDisplayComponent;
    4.     private ControlsManager m_ControlsManager;
    5.  
    6.     private void Start()
    7.     {
    8.         m_ControlsManager = CoreManager.instance.GetCoreComponent<ControlsManager>();
    9.     }
    10.  
    11.     private void Update()
    12.     {
    13.         if (Input.GetKeyDown(m_ControlsManager.debugControls.toggleDebugDisplay))
    14.         {
    15.             debugDisplayComponent.gameObject.SetActive(!debugDisplayComponent.gameObject.activeSelf);
    16.         }
    17.     }
    18. }
     
    Last edited: Apr 7, 2023
  8. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,384
    Why use reflection?
     
    Sluggy likes this.
  9. Sluggy

    Sluggy

    Joined:
    Nov 27, 2012
    Posts:
    845
    Yeah, gonna have to mirror lordofduct's comments on most of this stuff. Personally I'm not a fan of these massive 'collection objects' or 'god objects' in the first place but for quick n dirty solutions then can at least get the job done.

    In that last example I'd suggest just leaving your values public and being done with it. If for some reason you are really THAT worried about a value being overrwritten (which, who knows? might actually be a feature you want someday) then serialize a private value and expose it through a property getter. Less code. Less chance for bugs. Less overhead.
     
    lordofduct likes this.
  10. MartinMa_

    MartinMa_

    Joined:
    Jan 3, 2021
    Posts:
    455
    How can I do it? When i put getter and setter SerialalizedFields I get compile error.You mean just add method getControlsManager()? Kind of what Lombok does in Java?

    If I do it like this i cannot set it in editor.


    [SerializeField] public ControlsManager controlsManager { get; private set; }
     
    Last edited: Apr 7, 2023
  11. MartinMa_

    MartinMa_

    Joined:
    Jan 3, 2021
    Posts:
    455
    I just wanted to take all fields, how else you do it?
     
  12. Sluggy

    Sluggy

    Joined:
    Nov 27, 2012
    Posts:
    845
    The usual pattern is something like this.
    Code (CSharp):
    1. [SerializeField] ControlsManager _controlsManager;
    2. public ControlsManager controlsManager => _controlsManager;
     
    MartinMa_ likes this.
  13. MartinMa_

    MartinMa_

    Joined:
    Jan 3, 2021
    Posts:
    455
    It is working, I will do it like it, it is very simple thanks.