0

I've been using DI for a game engine project for a while and I just hit a wall; given the below order of creation: The job system does not depend on anything and everything depends on the file logger. It makes sense to create the job system, then the file logger, then pass the created references for each dependency down to its dependents' constructors.

App::App(const std::string& cmdString)
    : EngineSubsystem()
    , _theJobSystem{std::make_unique<JobSystem>(-1, static_cast<std::size_t>(JobType::Max), new std::condition_variable)}
    , _theFileLogger{std::make_unique<FileLogger>(*_theJobSystem.get(), "game")}
    , _theConfig{std::make_unique<Config>(KeyValueParser{cmdString})}
    , _theRenderer{std::make_unique<Renderer>(*_theJobSystem.get(), *_theFileLogger.get(), *_theConfig.get())}
    , _theInputSystem{std::make_unique<InputSystem>(*_theFileLogger.get(), *_theRenderer.get())}
    , _theUI{std::make_unique<UISystem>(*_theFileLogger.get(), *_theRenderer.get(), *_theInputSystem.get())}
    , _theConsole{std::make_unique<Console>(*_theFileLogger.get(), *_theRenderer.get())}
    , _theAudioSystem{std::make_unique<AudioSystem>(*_theFileLogger.get()) }
    , _theGame{std::make_unique<Game>()}
{
    SetupEngineSystemPointers();
    SetupEngineSystemChainOfResponsibility();
    LogSystemDescription();
}

void App::SetupEngineSystemPointers() {
    g_theJobSystem = _theJobSystem.get();
    g_theFileLogger = _theFileLogger.get();
    g_theConfig = _theConfig.get();
    g_theRenderer = _theRenderer.get();
    g_theUISystem = _theUI.get();
    g_theConsole = _theConsole.get();
    g_theInputSystem = _theInputSystem.get();
    g_theAudioSystem = _theAudioSystem.get();
    g_theGame = _theGame.get();
    g_theApp = this;
}

void App::SetupEngineSystemChainOfResponsibility() {
    g_theConsole->SetNextHandler(g_theUISystem);
    g_theUISystem->SetNextHandler(g_theInputSystem);
    g_theInputSystem->SetNextHandler(g_theApp);
    g_theApp->SetNextHandler(nullptr);
    g_theSubsystemHead = g_theConsole;
}

As you can see, passing the different subsystems around to the other subsystem constructors is starting to get messy. In particular when dealing with jobs, logging, console commands, UI, configuration, and audio (and physics, not pictured).

(Side note: These are going to eventually be replaced with interfaces created via factories for cross-compatibility, i.e. the Renderer is strictly a DirectX/Windows-only renderer but I want to eventually support OpenGL/Linux; that's why everything is passed around as references and created as pointers instead of a concrete types)

I've run in to situations where pretty much all the subsystems are in some way dependent on every other subsystem.

But, due to construction-order problems, Dependency Injection does not work because one or more of the required-to-exist subsystems hasn't been constructed yet. Same problem with two-phase construction: the subsystem may not have been initialized by the time it's needed further downstream.

I looked in to the service locator pattern and this question deems it a bad idea, but the game industry likes using bad ideas (like global variables to every subsystem for game-specific code to use) if they work.

Would converting to a service locator fix this problem?

What other implementations do you know of that could also fix the issue?

Casey
  • 10,297
  • 11
  • 59
  • 88
  • If we are so already committed to bad ideas ... what about just plain-old singletons? – ph3rin Jun 13 '21 at 03:29
  • @Meowmere Again, construction-order problems, a needed subsystem may not exist where it's needed by another system. – Casey Jun 13 '21 at 03:36
  • I have some probable solution. Let me do some crazy meta-programming. I'll come back later. – ph3rin Jun 13 '21 at 03:44
  • There cannot ve a construction dependency cycle. A post construction cycle is plausible. There cannot be a destruction dependency cycle. You are using construction parameters for both construction dependency and post. Right? – Yakk - Adam Nevraumont Jun 13 '21 at 03:51
  • @Yakk-AdamNevraumont What? – Casey Jun 13 '21 at 03:56
  • Just realized, you could also use the [local static variable trick](https://stackoverflow.com/questions/335369/finding-c-static-initialization-order-problems/335746#335746). No need for meta-programming. – ph3rin Jun 13 '21 at 04:02
  • @Meowmere How does that solve the dependency issue? – Casey Jun 13 '21 at 04:06
  • @Casey Everything from now on is lazily initialized. They won't get initialized until needed. That happens recursively to transitive dependencies. – ph3rin Jun 13 '21 at 04:09
  • @Meowmere I can easily break that with a simple scenario: An update to the job system makes it so it should log when a job is started/stopped etc, this requires access to a working FileLogger instance. The File Logger requires a working JobSystem instance in order to handle asynchronous logging to a file. Uh oh, the job system needs the file logger and the file logger needs the job system...engine crashes, house explodes. – Casey Jun 13 '21 at 04:48
  • @Casey By that time the JobSystem is already created, so the static initialization would not happen again (it only gets executed once the first time it is hit) – ph3rin Jun 13 '21 at 04:59
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/233705/discussion-between-meowmere-and-casey). – ph3rin Jun 13 '21 at 06:33
  • @casey Construction time dependencies - needing an X to construct a Y - is different than "runtime" and destruction time dependencies. Your pattern converts runtime dependencies to construction time, and explodes. If you don't need X at construction time, you should pass in a getter for X not a reference to X. – Yakk - Adam Nevraumont Jun 13 '21 at 13:30

1 Answers1

0

I ultimately went with the ServiceLocator pattern, deriving every subsystem that was a dependency as a Service:

App::App(const std::string& cmdString)
    : EngineSubsystem()
    , _theConfig{std::make_unique<Config>(KeyValueParser{cmdString})}
{
    SetupEngineSystemPointers();
    SetupEngineSystemChainOfResponsibility();
    LogSystemDescription();
}

void App::SetupEngineSystemPointers() {
    ServiceLocator::provide(*static_cast<IConfigService*>(_theConfig.get()));

    _theJobSystem = std::make_unique<JobSystem>(-1, static_cast<std::size_t>(JobType::Max), new std::condition_variable);
    ServiceLocator::provide(*static_cast<IJobSystemService*>(_theJobSystem.get()));

    _theFileLogger = std::make_unique<FileLogger>("game");
    ServiceLocator::provide(*static_cast<IFileLoggerService*>(_theFileLogger.get()));

    _theRenderer = std::make_unique<Renderer>();
    ServiceLocator::provide(*static_cast<IRendererService*>(_theRenderer.get()));

    _theInputSystem = std::make_unique<InputSystem>();
    ServiceLocator::provide(*static_cast<IInputService*>(_theInputSystem.get()));

    _theAudioSystem = std::make_unique<AudioSystem>();
    ServiceLocator::provide(*static_cast<IAudioService*>(_theAudioSystem.get()));

    _theUI = std::make_unique<UISystem>();
    _theConsole = std::make_unique<Console>();
    _theGame = std::make_unique<Game>();


    g_theJobSystem = _theJobSystem.get();
    g_theFileLogger = _theFileLogger.get();
    g_theConfig = _theConfig.get();
    g_theRenderer = _theRenderer.get();
    g_theUISystem = _theUI.get();
    g_theConsole = _theConsole.get();
    g_theInputSystem = _theInputSystem.get();
    g_theAudioSystem = _theAudioSystem.get();
    g_theGame = _theGame.get();
    g_theApp = this;
}

void App::SetupEngineSystemChainOfResponsibility() {
    g_theConsole->SetNextHandler(g_theUISystem);
    g_theUISystem->SetNextHandler(g_theInputSystem);
    g_theInputSystem->SetNextHandler(g_theRenderer);
    g_theRenderer->SetNextHandler(g_theApp);
    g_theApp->SetNextHandler(nullptr);
    g_theSubsystemHead = g_theConsole;
}
Casey
  • 10,297
  • 11
  • 59
  • 88