Circular dependencies and best practises to avoid them

[Sami]

Godot Version

v4.5.stable.official [876b29033]

Question / preface

First, I want to apologise for this thread being a pretty broad topic without a super clear and straightforward question. I hope it’s still okay to post a topic like this!

We have now been using Godot for a handful of months and have really liked the engine. However, the one thing that we have had the most problems is circular dependencies.

After adding new functionality, our game can break in seemingly an unrelated place, due to added new dependencies. It doesn’t help that the editor seems to be pretty bad at detecting this. We end up wasting time hunting down the culprit, as Godot shows an error in the last place, instead of in the place where the original ‘cyclic branch’ takes place.

A different loading order can cause the compilation of certain custom classes to fail, resulting in them being treated as their closest native type (such as Node2D, GDScript, or Resource). We discovered that adding standalone references to custom class names can help enforce a more reliable loading order and temporarily bypass the issue. However, this approach is not a stable or a long-term solution.

Dependencies in autoloads seem to trigger this more often, and their compile order is actually not the one defined in the globals list, because dependencies can shift their actual load order. Preloads and static initialisers seem to also take place before the first autoload, so we’ve really struggled on how to fix some of the issues popping up.

Example problematic use case

We allow managers to call functions on scene tree nodes and vice versa. For example, we let a CardManager act on CardUI nodes. But we also allow CardUI nodes to call functions of the CardManager. Cyclic dependencies like these have been working fine so far. But it seems like as soon as static initialisers and/or preloads get involved, they can become a vulnerability.

Signals?

While we do use signals a lot, maybe we should use them even more? One concern about them is, again, debugging. When signals fail, they do it silently, whether it was a missing parameter or simply a numeric value in the wrong type.

Best practices?

My guess is that we’re simply utilising some functionality improperly, or not in the way Godot’s best practices would recommend. Could someone more knowledgeable share best practises on how to avoid them? For example, is there a more direct way to control the order Godot loads scripts and assets to better enforce an order that doesn’t break?

[normalized]

Eliminate all circular dependencies. They are a sure sign of bad architecture. You can do without autoloads as well if the only thing you use them for is static persistent data. Put that into static classes.

[Sami]

Would you recommend using signals to avoid them? Like our CardManager and CardUI. Would you only let one of them to reference the other and not both ways?

Do you know if there’s a way to detect if we mistakenly make these circular references? Any guidelines would be very helpful!

[normalized]

Use signals as much as you like but try to keep them hierarchical. As for circulars, start with eliminating all mutual calling and references needed for doing it.

Oh, and this is just my personal (although informed) pet peeve - get rid of every class that has “manager” in its name. The minimum you can do in this regard is not calling it “manager”. If the thing is responsible for managing cards, call it Cards.

[Whebon]

We have a CardManager that creates and manages cards spread over several locations such as the hand, deck and discard pile. For certain effects, we allow cards to access cards in other locations via the CardManager. This creates a (runtime?) cyclic dependency between CardManager and Card.

This not a cycle that currently causes issues for us, but we do have similar larger cycles that break the compilation.

Is this a kind of circular dependency we need to elimninate? And if so, what would be a better architecture?

Also, is there any tool we can use to visualize the dependencies? That would save us a lot of work!

[normalized]

Follow the classic rule of thumb “Call down, signal up”. Cards have really no business calling anything in the manager directly. Let them just signal what they need and then let the manager… well… manage and give them what they ask for… if they deserved it.

[Whebon]

We have a card that deals damage equal to the number of cards in the deck. The card currently requests this number from the CardManager using a function call. How could we refactor this to use a signal instead?

[normalized]

The card shouldn’t deal the damage. That’s what you have the manager for, to manage stuff that happens between the cards, i. e. on the level above an individual card.

If you insist on card doing it, then the manager should be responsible to push that information on the card whenever it’s changed so the card is always up to date without needing to ask.

It’s all in the name. The manager is supposed to manage cards

[sancho2]

@normalized

Put that into static classes.

One day maybe, but for now the best you can do is static functions and variables; a static class that you can, never the less, instantiate.
The only advantage that I can see is that there should be slightly less overhead. But that advantage all but disappears if all you are doing is loading it with global constants.

[normalized]

@sancho2

One day maybe, but for now the best you can do is static functions and variables; a static class that you can, never the less, instantiate.

Umm… by “static class” I meant a class in which everything is declared static. You don’t need to instantiate that. Such class serves as a mere namespace. It’s a better way to store persistent global data than autoloaded nodes.

[TheKrel_Company]

To avoid circular dependency use clean architecture: CardUI should not know about the CardManager. There is several options to reach that goal:

• as mentioned in the other posts: use signals. Either CardUI has several signals for different state, or one signal that will propagate a specific object holding the different states of CardUI. The CardManager must have logic to handle those different states.

• use a service, a singleton that will act as a middleman between CardManager and CardUI. CardUI calls the singleton functions that triggers CardManager functions. And vice versa.

• dependency injection: pass CardManager reference to CardUI at _init as its base class (Node2D, Control, Node). It will act as a kind of interface. Store that ref in a meaningful variable and call the functions you need. Just be aware, that’s not a true interface, but duck typing, so the script editor will display only functions available in that base class, but the function from CardManager will be called.

[Whebon]

We have been using a lot of class_names for typing hints. These names lead to multiple cyclic references in the code base. Does there exist a tool to visualize the cycles in a graph or matrix? Or do we need to write our own?

[normalized]

I don’t know any such tools but you should take care not to get into situation of needing them in the first place. Like… ever. Always design in a way that doesn’t introduce any circular dependencies.

Circular dependencies are not something you search for after the fact and then “fix”. You just don’t do them. They compromise the architecture at the fundamental level. Or more precisely, they are a sign that your design is not well thought out and needs to fundamentally change.

At this point I’d re think the whole architecture and refactor all of the code.

So to directly answer the question from the title. The best practice to avoid them is not making them :D. And you’ll not make them if you design in a way that establishes a well defined one-way hierarchy of responsibilities between the classes.

[Whebon]

use a service, a singleton that will act as a middleman between CardManager and CardUI. CardUI calls the singleton functions that triggers CardManager functions. And vice versa.

I like the service approach. Instead of allowing CardUI to directly access the CardManager, let’s say we have a CardService that acts as a middleman:

class_name CardUI

func get_damage() -> int:
	return CardService.get_deck_size()

class_name CardService # could also be an autoload or singleton, but the idea is the same I believe

static func get_deck_size() -> int:
	return CardManager.deck.size()

To me it seems like this only increased the size of the cycle:

CardManager knows CardUI
CardUI knows CardService
CardService knows CardManager

Is this kind of circular dependency ok? If not, how should the service be designed instead?

[normalized]

@Whebon

Is this kind of circular dependency ok? If not, how should the service be designed instead?

Architecturally, in terms of spaghettization - it’s still a circular dependency of sorts. The described “middleman” is a global variable and you know what they say about global variables in an object oriented program. You may as well make the card manager itself global instead, with the same results but without introducing additional complexity and shoehorning more abstractions in. You can see how confusing the addition of this new totally abstract object gets just by looking at the names: CardManager and CardService? What’s supposed to do what with cards?

[TheKrel_Company]

Yes, you’re right. If CardManager and CardServie know each other, then you have a circular dependency. Sorry if I misled you. I answered without checking if what I thought was right. Services should be only called by other classes, and must not know anything about these classes.
@normalized is right, you’ll have to re architecture your project. Check how works the clean architecture, it would help you to avoid that issue.

[normalized]

Go through all calls that the card makes towards the card manager. Each call acquires some data and/or changes some state. Remove the call and intervene on the manager side to make sure that the data is delivered and state changed in timely fashion, without any card ever needing to give orders to the manager in form of explicit function calls. Subordinates don’t order managers.

A card can broadcast its needs via signals, but only the manager has the final word if (and when) those needs should or shouldn’t be met.

This may require you to change the way you were thinking about the problem so far. But being flexible in ways of thinking is what makes a good system architect.

As a rule of thumb; you want your class communication graph to look like a hierarchical tree, as much as possible. And you want this tree to be as shallow as possible.

[normalized]

I’ll give you an example using the problem you described earlier.

The problem:
A card needs to know how many cards are in the deck.

Currently (circular dependency):
The card makes a call towards the manager to retrieve the number of cards, whenever the card thinks it needs it.

Re-do:
The manager knows the card count in the deck. The manager knows which cards need to know the card count. The manager also knows when the count has changed. So whenever the count changes, update all cards that need to know, with the new count. The manager is doing its job - managing cards.

[Whebon]

Our project architecture is based on a deck builder tutorial. The complete source code can be found here.

The architecture looks very good to us. And the game is stable, even with circular references.

Most of these circular references are caused by the Events autoload. We have seen several videos recommending the event bus pattern, and so far, it hasn’t caused any issues in practice. However, we’re wondering: should it still be avoided purely because it introduces circular references?

If we remove the Events autoload from the graph, only a handful of circular references remain.

Cycle 1:

CardUI -> CardStateMachine -> CardState -> CardUI

A CardUI has a CardStateMachine with CardStates.
CardStates act on their corresponding CardUI, such as animating it to a position or checking if it can be played.

Cycle 2:

CharacterStats -> CardPile -> Card -> CharacterStats

CharacterStats has Cardpiles with Cards.
Cards need to know if they can be played, for which they need to access the CharacterStats’ mana.

One way to break this cycle is to have CharacterStats push its mana value on all Cards whenever it changes. However, this seems a bit wasteful if only 1 of the cards actually turns out needing that information.

Question

Do you have any design recommendations to cleanly break the mentioned cycles without introducing unnecessary spaghetti?

[normalized]

That graph looks scary. If it represents your project at its current state, and you have hard time controlling it, it’s unlikely that any stranger from the internet would be able to untangle it for you. Your best bet is to ask the author of that tutorial. After all you’re using their architecture.

I don’t have any specific advice, other than what I already said above. I’d just add this - don’t blindly follow tutorials. I skimmed through that tutorial and it’s almost like a showcase of bad aspects of object oriented design.

Allow me to elaborate a bit.

One of the big promises of object oriented programming is that it is supposed to let you model in the domain of your particular problem. Ironically, many oo programs end up modeling a great deal in the object oriented programming domain instead. A meta domain so to speak. This results in invention of weird abstractions that don’t exist in the problem domain.

It’s hard to tell if this is due to innate weaknesses of the object oriented paradigm or simply a fashion. I see this happening a lot. It’s particularly common with beginners. They seem to think the proper way to do object orientation is to spend a lot of time and effort in this meta domain, thinking of and inventing weird meta objects and abstractions that are then supposed to manipulate “regular” objects from the problem domain.

Such objects tend to create more problems than they solve because they introduce enormous amount of unneeded architectural complexity. They just burden the problem domain while not even being part of it. Whenever I see names like “SomethingManager”, “SomethingComponent”, “SomethingService”, “SomethingHandler”, etc… I know that sooner or later the major architectural problems will start appearing in that system.

The thing is - you don’t need to do this.

Let’s take your case as an example. What does the CardManager represent in your problem domain? Does such thing really exists there? It likely doesn’t. So why ever invent such an object?

Your problem domain is playing cards. The objects that exist there are: Player, Deck, Hand and Card. That’s all there is to it. It’s perfectly enough to make a model.

The Deck is the “manager” of cards it contains. The Hand is the “manager” of cards it holds. And the Player is the “manager” of both; Deck and Hand.

         Player
        /      \
     Deck       Hand
     /  \       /   \
  Card  Card  Card  Card

The model is now strictly in the problem domain, as it should be if we adhere to the primal principles of object oriented design. The hierarchy of responsibilities is simple and well defined. No confusing meta objects with ambiguous responsibilities. No artificial managers or handlers.

If something needs to happen between a card in the hand and a card in the deck, the Player is responsible to do it, as is the case in a real card game in the real world.

Judging from your responses so far, I’m pretty sure you will ignore what I’m saying. I’ll leave it here anyway though. You may still return to it someday in the future.