Is this an example of good abstraction, or terrible coupling?

[Sendatsu_Yoshimitsu]

I’m working on an RPG where a lot of common utilities, like spawning a character or changing levels, need to be triggered from a variety of narrowly scoped objects. A character whose conversation triggers a cutscene doesn’t need to know anything about the cutscene system, and doesn’t care what happens after it makes the request; it just needs to find some way of requesting that a given scene be loaded and played.

To accomplish this, I ended up with something which is basically a singleton masquerading as a service locator- I made a GameManager class with a static instance that allows any object to access it at runtime, but gave it no internal functionality. All of its public-facing methods are things like SpawnCharacter(characterID, location) or LoadCutscene(cutsceneID). Aside from these methods, the only stuff it contains are private references to the cutscene manager, character database, audio system and so forth. In the case of SpawnCharacter, for instance, all the game manager actually does is take the information passed in, and directly invokes CharacterDatabase.SpawnSomebody(ID, location). It doesn’t know or care what happens, so long as the data is passed on.

This feels like relatively decent design; it’s serving as a messenger, not an actor, and any bugs caused by calls to the game manager are going to be instantly traceable back to the class those calls get routed to.

On the flip side, however, single monolithic classes that everyone talks to are practically the definition of code smell, and I’ve coupled a huge amount of game functionality through the manager just by giving it access to a bunch of unrelated manager classes.

So is this architecture a really bad idea that’s going to explode in my face as the codebase grows, or am I safe to iterate around this concept as long as the manager only serves as a messenger, and never actually grows to contain functionalities beyond passing information and requests where they need to go?

[Baste]

What does:

callingCode → GameManager.SpawnCharacter(characterID, location) → CharacterDatabase.SpawnCharacter(characterID, location)

provide that you don’t get from:

callingCode->CharacterDatabase.SpawnCharacter(characterID, location)

?

It decouples your calling code from the Character Database, yes, but it now couples it with a GameManager. If your calling code needs to spawn characters, then it’s going to be coupled with the character database somehow.

Essentially, if your game manager doesn’t do anything, why does it need to exist?

[Sendatsu_Yoshimitsu]

It would be easy enough to obliterate the game manager entirely by making each of the manager objects their own self-contained unit, but I would still end up making each of those a singleton, since the core architectural need I’m trying to address is allowing objects to access a common set of utilities without making the entire project a huge network of dependency chains. If I make every single manager class static, I’ve suddenly made a huge portion of my code global, which at least intuitively feels more schizophrenic than having everyone blindly send data to a single manager who routes it.

[Dave-Carlile]

You mentioned Service Locator, which seems like a good way to handle this sort of thing. But you seem to just have a monolithic class that lets you call functions. Rather than that it seems that a service locator is supposed to return an interface to a set of functionality. In your example you’d get an interface to the character spawner, or the cutscene manager. Anyone can get that reference and use it as it needs, then release the reference when it’s no longer needed.

It’s decoupled because your’re only working with interfaces that you ask for from the locator.

This Game Programming Patterns book is a good resource for these sorts of things. It’s free online, but I always encourage people to purchase it since it’s a quality book.

[Sendatsu_Yoshimitsu]

@Dave-Carlile

You mentioned Service Locator, which seems like a good way to handle this sort of thing.

That is a great resource, thank you for the link! This is probably a silly question, but after pouring over it I’m having a bit of trouble figuring out how the dependency injection would be structured in Unity- is the idea that I would just create a single Locator class with a static instance for each manager/category of functionality I was indexing, then manually drag & drop its reference to the prefab for whichever manager class it needed to point to?

[daxiongmao]

Dave-Carlie’s can be a good idea. I am not sure I would go with a pure interface based setup or just get a reference to the actual implementation. It depends on if there may be more than one implementation.

The other setup I have used Is i have a singleton that that has events for those things. So I would fire off the actor action method, and then if someone was interested in that event the would be listening for it. If no one cares at that time then nothing happens. a publish subscribe type of system.

But in your case if you will always have one available or require something else to react then either having direct singletons or the service type patter can be used.

But with all singleton type patterns there always comes issues with finding them and initilization order.
Especially since unity likes monobehaviors for everything. Then being able to free things if they are no longer required.

A lot will come down to your usage pattern and lifetime requirements. Right now I use separate singletons. Other times I have used a centralized approach.

[BlackPete]

Just keep in mind that whatever you do, it’s supposed to make your job easier in the long term.

Single monolithic classes are fast to write in the short term, but are horrible to maintain in the long term. Especially when you need to add more stuff like new classes, skills, etc.

However on the other side of that coin, things abstracted and broken up so much that you have to touch dozens of files each time you need to add something is, well, inefficient. It’s also a good way to introduce bugs if you overlook something.

Find a nice balance where it’s easily extensible without a whole bunch of plumbing and updating a whole bunch of stuff.

[Sendatsu_Yoshimitsu]

I really appreciate all of the advice, thank you guys.

Just to clarify, on implementing a Locator object to allow whatever external interfaces need to use them, would the general idea be to do something like the following?

    public static Locator instance;

    public ConversationManager conversationManager; //public so I can set this from the inspector;

    public ConversationManager GetConversationManager()
    {
        return conversationManager;
    }

That’s not technically dependency injection, but since I’m doing this without explicit pointers it seems like a rough approximation of the same process. Once this is set up everything else becomes private and non-static, and any of my manager interfaces that need to effect functionality look something like:

public class SomeExternalClass : MonoBehaviour{
    public void StartConversation(conversationID)
    {
        Locator.instance.GetConversationManager().StartConversation(conversationID);
    }
}

[lordofduct]

1. you can use SerializeField to allow private fields be visible in the inspector:
   Unity - Scripting API: SerializeField

2. What’s the point of making GetConversationManager() if all it does is return an already public field?

3. If you’re going to make GetConversationManager() to return a field… you could make it static and thusly do the work of accessing instance for you.

[Sendatsu_Yoshimitsu]

@lordofduct

1. you can use SerializeField to allow private fields be visible in the inspector:
   https://docs.unity3d.com/ScriptReference/SerializeField.html

2. What’s the point of making GetConversationManager() if all it does is return an already public field?

3. If you’re going to make GetConversationManager() to return a field… you could make it static and thusly do the work of accessing instance for you.

I think I’m missing something really obvious with regards to how dependency injection is supposed to work. What I’m trying to do with the Locator example above is replicate the functionality from the end of the linked tutorial’s pseudocode example:

class Locator
{
public:
  static Audio* getAudio() { return service_; }

  static void provide(Audio* service)
  {
   service_ = service;
  }

private:
  static Audio* service_;
};

Since I’m doing this in Unity I can’t directly use pointers in a safe context, but the next-closest equivalent seems to be using an exposed field to manually give Locator references to whichever classes it’s supposed to supply to third parties, at which point we hit your point and I would be better off just making those classes explicit singletons to begin with. Is there really any benefit to forcing DI to fit here? I’m trying to make the foundational architecture as expansible as I can, but it’s increasingly feeling like the path of least resistance is to nix the gamemanager, and convert each of the smaller manager classes it communicates with to singletons.

[Baste]

I’ve yet to see a DI scheme in Unity that doesn’t boil down to an absurdly cumbersome way to globally fetch global data. Then again, people keep saying that DI is The Second Coming, so I might be missing something.

By all means, hide the access to the stuff doing your work behind an abstraction when that becomes necessary. For now, it sounds like you should just use the bog standard singletons and then refactor once that becomes cumbersome.

Refactoring isn’t dangerous! People should do it more often.

[Sendatsu_Yoshimitsu]

@Baste

I’ve yet to see a DI scheme in Unity that doesn’t boil down to an absurdly cumbersome way to globally fetch global data. Then again, people keep saying that DI is The Second Coming, so I might be missing something.

By all means, hide the access to the stuff doing your work behind an abstraction when that becomes necessary. For now, it sounds like you should just use the bog standard singletons and then refactor once that becomes cumbersome.

Refactoring isn’t dangerous! People should do it more often.

Alright, thank you very much for putting up with my inane questions- I’ve always found it far too tempting to think in monoliths, so getting feedback is incredibly helpful.

[Baste]

I mean, it could be that doing a full-blown DI framework before you implement… any features is a great idea, and I’m wrong about everything.

You never know!

[Sendatsu_Yoshimitsu]

@Baste

I mean, it could be that doing a full-blown DI framework before you implement… any features is a great idea, and I’m wrong about everything.

You never know!

That’s one reason I always feel so silly about asking programming questions in the format of “Is a good idea”- there are cases where it’s valuable to look before you leap, and I think foundational game architecture is one of them, but I almost always learn more when I just go “Screw it,” implement and see if it works, then iterate through fixing the problems that arise. (And then in retrospect, go “I’m never doing that again, holy crap” )

[KelsoMRK]

@Baste

Then again, people keep saying that DI is The Second Coming, so I might be missing something.

In the enterprise world it’s incredibly useful. Less so in game development and especially when you’re working on top of a pre-existing engine. If nothing else, the code bases tend to be dramatically smaller and Unity is already handling your platform deployment for you. Rare is the Unity-equivalent of “I need to inject a DataSource interface so that the backing implementation can change when I deploy to 4 different flavors of database”.

[Baste]

Heh. We’ve got 7 different definitions of our save-to-file and load-from-file code (PC standalone, Steam, GOG, WiiU, Switch, XBoxOne, PS4). So there are still instances where it’s necessary :p. We’ve just done that through 7 versions of the same file, each wrapped with it’s own #if UNITY_PLATFORM. Maybe we should’ve used injection instead? Problem is that the save code for one platform does not compile/is disallowed for the next, so all calls to the platform-specific code must be conditionally compiled.

[KelsoMRK]

Yeah that might have been a good candidate

Only way to DI that would be to put each in their own external lib I guess…but at that point is it any better than conditional compilation?

[fvde]

This thread makes me happy, because you are taking the time to ask yourself these questions, and sad at the same time because of the amount of floating misinformation…

What you are doing is indeed very high, global coupling. It saddens me how much Unity encourages this. And no it is not sending messages, it’s sending actions. What you’re considering now is not DI, it has nothing to do with pointers and the key to DI is IOC, Inversion of Control (worth googling!). That allows for truly decoupled services and crucially allows isolated unit testing by mocking everything else.

With all that being said: Do you need any of this? Probably not. From personal experience I can say that once you really grasp DI you will never want to go back, regardless of game or enterprise service.

TLDR: There’s very good DI frameworks, google IOC, pub sub models

[KelsoMRK]

@fvde

That allows for truly decoupled services and crucially allows isolated unit testing by mocking everything else.

Except game development tends to not be very service oriented and unit testing is notoriously difficult (especially in Unity).

@fvde

From personal experience I can say that once you really grasp DI you will never want to go back, regardless of game or enterprise service.

From personal experience it’s awesome for enterprise and next to worthless for game development

Me

• IT Consultant, specialist in spring boot based microservices

I know this is why you’re saying these things and I 100% agree with you outside of a Unity context

[BlackPete]

@Sendatsu_Yoshimitsu

I think I’m missing something really obvious with regards to how dependency injection is supposed to work. What I’m trying to do with the Locator example above is replicate the functionality from the end of the linked tutorial’s pseudocode example:

class Locator
{
public:
  static Audio* getAudio() { return service_; }

  static void provide(Audio* service)
  {
   service_ = service;
  }

private:
  static Audio* service_;
};

Since I’m doing this in Unity I can’t directly use pointers in a safe context, but the next-closest equivalent seems to be using an exposed field to manually give Locator references to whichever classes it’s supposed to supply to third parties, at which point we hit your point and I would be better off just making those classes explicit singletons to begin with. Is there really any benefit to forcing DI to fit here? I’m trying to make the foundational architecture as expansible as I can, but it’s increasingly feeling like the path of least resistance is to nix the gamemanager, and convert each of the smaller manager classes it communicates with to singletons.

I like this quote from a 2006 article: “Dependency Injection” is a 25-dollar term for a 5-cent concept.

The locator code you posted above might as well be a property.

Don’t get me wrong, there are cases where DI is useful (try working in a team with dozens if not over a hundred developers), but to repeat myself: Don’t add code for the sake of adding code. Do it only if it’s going to make your job easier in the long run.

A framework can make sense in a large team because you want the code base to be consistent and therefore easier to navigate, regardless of who wrote the code you’re currently looking at. It also reduces the odds of stepping on toes.

If you’re a one-man team, all that might be overkill. That’s not to say it’s a bad idea, but if you’re increasing the amount of work you need to do to keep the code safer from… yourself, you increase the chances of burnout.