Question about global singleton

Development Programming & Scripting C++
1

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


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:


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.

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…

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:




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:




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 :wink:

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 :slight_smile:

Kind regards

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.

6

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

Thank you very much fo detailed reply, after some research, I think I finally nailed the singletons :slight_smile: 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 :smiley:

7

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?

8

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.

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.

9

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