Announcement

Collapse
No announcement yet.

Question about global singleton

Collapse
X
 
  • Filter
  • Time
  • Show
Clear All
new posts

    Question about global singleton

    Hi! Everywhere I look, global singleton looks like this:


    Code:
    UDataTableHolder* UDataTableHolder::Get()
    {
    
    	UDataTableHolder* DataInstance = Cast<UDataTableHolder>(GEngine->GameSingleton);
    
    	if (!DataInstance) return NULL;
    	if (!DataInstance->IsValidLowLevel()) return NULL;
    
    	return DataInstance;
    }
    It works fine, but here I thought, why not to do:

    Code:
    static UDataTableHolder* DataInstance;
    
    void UDataTableHolder::Get()
    {
            if(DataInstance == nullptr)
            {
    	DataInstance = Cast<UDataTableHolder>(GEngine->GameSingleton);
    
    	if (!DataInstance) return NULL;
    	if (!DataInstance->IsValidLowLevel()) return NULL;
            }
    	return DataInstance;
    }
    Which should, in theory, save some extra checks and casting time. Is there any catch? Like, maybe static variable declared outside the score, will actually be kept even after game stops playing in editor? But from what I think, it would become nullptr too

    #2
    Global Singletons are actually an anti pattern and should be avoided in most cases.

    Since this is a data table reference, it should only need to be fetched once and cached. Code that is doing a runtime/tick lookup of a data table instance most likely needs to be refactored.

    http://stackoverflow.com/questions/1...n-anti-pattern

    Comment


      #3
      Hmmmm, but I think for storing global databases which are static and never changes and are read only and are used everywhere around game - its fine! Maybe. Hm....

      Comment


        #4
        FMPOV simply saying that Singletons are an Anti-Pattern is not correct. I agree that one should avoid globals at any case. But using singleton doesnt mean you are writing bad code or something. The problem is
        the misuse of singletons, so implementing this pattern in cases where they dont fit well. Another problem is the question if a singleton would solve my current problem and doesnt end in problems later, because its really hard to foresee that. If you
        are 100% sure that your singleton class only needs one valid object during runtime it is ok to use this pattern. F.e. the Engine of UE4 can be a singleton, because there are no intentions to have multiple engine objects running at the same time in one application instance.

        Another example could be the GameSingleton, where you usually store persistent data and only "read" those data during runtime. As soon as you start to write in those objects it may gets complicated again.

        Btw. the code:

        Code:
        static UDataTableHolder* DataInstance;
        
        void UDataTableHolder::Get()
        {
                if(DataInstance == nullptr)
                {
        	DataInstance = Cast<UDataTableHolder>(GEngine->GameSingleton);
        
        	if (!DataInstance) return NULL;
        	if (!DataInstance->IsValidLowLevel()) return NULL;
                }
        	return DataInstance;
        }
        could be simplified and fixed with:

        Code:
        UDataTableHolder* UDataTableHolder::Get()
        {        
               	return Cast<UDataTableHolder>(GEngine->GameSingleton);       
        }
        First, you return a Singleton object which seems to already exist in GEngine, so why storing it in another Pointer?
        Second, the nullptr checks are obsolete (FMPOV), since you return NULL or nullptr indicating that this CAN RETURN NULL, a User would do these checks usually a second time after calling Get() to make sure he got a valid object. So instead i would rather place a logmessage, telling that the Object was null for some reason.
        Third, the check for IsValidLowLevel is referring to GC iirc. So when instatiating Singletons you need to make sure that you have a persistent object the whole time, f.e. via Calling AddToRoot() after Ctor.

        The way you describe for using this class is also something i like to do sometimes. Using it as a kind of manager or interface for a specific area you want to have access to, is also fine. And yes there are discussions about using classes called "Manager" -> this
        will lead to other topics with different discussion and in the end you have the feeling that you shouldnt write any code at all

        What bothers me a bit is, that you expect the GameSingleton to be a UDataTableHolder, implying that the inbuild game-wide Singleton is only used for a very small section of your game, what if you want to store other stuff than datatables in the gamesingleton?

        Im looking forward to hear other opinions

        Kind regards
        Wallhalla
        Live long and prosper.

        Comment


          #5
          Short question, as I may oversee something here:

          Why don't you store that stuff in the "GameInstance" class? That thing only exists once per Player and will survive level changes.
          It gets created when the Game gets started and destroyed when you close it. With "Game" I mean the actual process of your game.

          You can even add components to it that handle your DataTables etc, to keep the GameInstance itself clean.
          Open for contracted work | C++/BP (incl. Multiplayer) | Tutoring | VR

          My UE4 Blog/Page with Tutorials and more: Hit me for ALL the things!
          (Including 100+ Pages Multiplayer Network Compendium to get you started.)

          Comment


            #6
            Originally posted by eXi View Post
            Why don't you store that stuff in the "GameInstance" class?
            I never consider that an option. I came with opinion, that singleton is something very good, now I have to re-think it all over again

            Originally posted by Wallhalla View Post
            What bothers me a bit is, that you expect the GameSingleton to be a UDataTableHolder, implying that the inbuild game-wide Singleton is only used for a very small section of your game, what if you want to store other stuff than datatables in the gamesingleton?
            Thank you very much fo detailed reply, after some research, I think I finally nailed the singletons My UDataTableHolder is used almost from every game class I have, so that is why I thought its good idea to have it as singleton - I store strictly data from UTableRows in there. Even though it works fine mor my case, I think I will stay away from sigletons now. Before this whole topic, I had impressions that singletons are comesthing cool to use, but now I see

            Comment


              #7
              Originally posted by eXi View Post
              You can even add components to it that handle your DataTables etc, to keep the GameInstance itself clean.
              Apologies for bumping the old thread but there is something unclear to me
              What are those "components" you mention? Can I just make a C++ class and instantiate it in my GameInstance (blueprint) for private access?
              Hi!

              Comment


                #8
                Originally posted by fpcompany View Post
                Apologies for bumping the old thread but there is something unclear to me
                What are those "components" you mention? Can I just make a C++ class and instantiate it in my GameInstance (blueprint) for private access?
                Na, I kinda forget that the GameInstance is not an AActor child. But every other class that is a child of AActor can have components.

                https://docs.unrealengine.com/latest...ne/Components/

                I guess for this GameInstance, if you really want to have it clear of the DataTable code, then you could create a UObject Class and instantiate that in your GameInstance.
                Open for contracted work | C++/BP (incl. Multiplayer) | Tutoring | VR

                My UE4 Blog/Page with Tutorials and more: Hit me for ALL the things!
                (Including 100+ Pages Multiplayer Network Compendium to get you started.)

                Comment


                  #9
                  Originally posted by eXi View Post
                  I guess for this GameInstance, if you really want to have it clear of the DataTable code, then you could create a UObject Class and instantiate that in your GameInstance.
                  I decided to make parent c++ GameInstance and store my stuff there effectively making my blueprint GameInstance less clogged up.
                  I also figured out that when I access to said stuff from various places, I'm having concurrency problems so I just added locks here and there and it works OK now.

                  still not sure if it was a good idea though
                  Hi!

                  Comment

                  Working...
                  X