Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Static GameManager - looking for criticism

Discussion in 'Scripting' started by Tom163, Oct 31, 2020.

  1. Tom163

    Tom163

    Joined:
    Nov 30, 2007
    Posts:
    1,290
    I'm looking for a non-Singleton way of setting up my global data and managers. After a bit of reading and experimenting, I've come up with this:

    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4.  
    5. public static class GameManager {
    6.  
    7.     public static WorldData World;
    8.  
    9.     [RuntimeInitializeOnLoadMethod]
    10.     static void OnRuntimeMethodLoad() {
    11.         LoadWorldData();
    12.     }
    13.  
    14.  
    15.     public static void LoadWorldData(string fileName="WorldData") {
    16.         World = Resources.Load<WorldData>(fileName);
    17.     }
    18.  
    19. }
    20.  
    Where WorldData is a Scriptable Object:

    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4.  
    5. [CreateAssetMenu(fileName = "New WorldData", menuName = "DataObjects/WorldData")]
    6. public class WorldData : ScriptableObject {
    7.  
    8.     public int GameTick = 0;
    9.  
    10. }
    11.  

    This appears to work wonderfully, and I can access my world data from everywhere while also being able to edit and load/save them nicely, have multiple versions I can switch between and all the other advantages of SOs.

    What am I missing? Why is this a bad idea? Why should I better write 20 functions and 10 singletons to do the same thing?

    (yes, I know Resources are considered evil and Addressables should be used, I can switch to using Addressables as soon as the system stops being a terrible mess that F***s up legacy resources.)
     
  2. PraetorBlue

    PraetorBlue

    Joined:
    Dec 13, 2012
    Posts:
    7,893
    There's nothing wrong with this except that in your quest to not use a singleton, you've gone ahead and implemented a singleton. ¯\_(ツ)_/¯
     
  3. Tom163

    Tom163

    Joined:
    Nov 30, 2007
    Posts:
    1,290
    Without the instancing nonsense and in a way that puts all the actual data and logic into Scriptable Objects, avoiding the usual singleton issues. At least that's what I think. But I posted here to be proven wrong, because I'd like to know now before half my game is based on something that might be silly. :)
     
  4. Suddoha

    Suddoha

    Joined:
    Nov 9, 2013
    Posts:
    2,824
    From a technical perspective there's absolutely nothing wrong with it, except that one thing that you already know. And besides that, If loading is only supposed to be done by the script - I'd not expose the field but use a private field + getter instead.

    However, from the perspectives of software architecture and design prinicples this is one of the worse things you can do. Access game manager here and there, querying the most atomic piece of data using the game manager and the world data couples the consumer to both types.
    With such convenience in place ("I can access X from everywhere"), people tend to literally do just that.
    In other words, you're likely going to write fewer classes that only get and use what they're supposed to use, thus coupling them to a greater context (worst case: that one specific application).

    If you visualize the dependencies you will quickly realize that this architecture has massive flaws -at least if good architecture is a concern.

    But again, if you only need to get things done quickly and/or don't need to worry about having code that is reusable somewhere else, you can simply ignore the above.
     
  5. Tom163

    Tom163

    Joined:
    Nov 30, 2007
    Posts:
    1,290
    Thanks for the advise there. What would be a better design? I have a turn-based (in parts) game, so information like the current game tick needs to be widely available. I'm trying to AVOID coupling but concentrating it all in one place. If I put it only into a Scriptable Object, then I need to link to that SO everywhere and worse, if I load games, I need to make sure I don't forget any one place that references it.

    Same if I use an in-scene GameController. I need to reference that everywhere, duplicat a lot of FindByTag() and GetComponent code all over the place.
     
  6. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,294
    The big, fat flaw with the design is this: