From 719f2d79679949075e2fea0325717f96a201245f Mon Sep 17 00:00:00 2001 From: elexis Date: Sun, 25 Aug 2019 11:02:55 +0000 Subject: [PATCH] Remove default CGame constructor values to make the code less error-prone, use CRenderer::IsInitialised() to test if the CGame should be rendered to remove indirection/proxies, making the code easier to read. 1c0536bf08 introduced a disableGraphics bool with a default value and relied on the default being reasonable except for the few needed cases. be93b31411 introduced the replayLog argument with a default value and relied on the default being reasonable except for the few needed cases. 5747619c39 fixed a bug in that commit because the default value hadn't actually been considered to be correct for all CGame constructor calls and was wrong for two. By requiring callers to specify the value, authors are forced to establish thought which value is the correct one, as opposed to hoping that a default value will be good by default. As you can see in the diff, it also makes it easier to compare what values changed if they are always defined in the caller. Use CRenderer::IsInitialised() to determine if this is a non-visual CGame, for the purpose of removing less transparent proxy functions that are unneeded as long as there are about 30 other calls testing for CRenderer::IsInitialised() to determine if the Game should be rendered. Supersedes: * CGame constructor argument bool disableGraphics from 1c0536bf08. * CGame::IsGraphicsDisabled() proxy from a533fff883 to the proxy from 1c0536bf08 and two local nonVisual = args.Has("autostart-nonvisual") variables in GameSetup.cpp from a533fff883. Call the Renderer destructor instead of calling delete on the non-pointer (SAFE_DELETE would not be supported for instance). Started as a preparation for D2197, but actually independent. Differential Revision: https://code.wildfiregames.com/D2211 This was SVN commit r22785. --- .../network/scripting/JSInterface_Network.cpp | 4 +-- source/network/tests/test_Net.h | 14 ++++----- source/ps/Game.cpp | 4 +-- source/ps/Game.h | 14 ++------- source/ps/GameSetup/GameSetup.cpp | 30 +++++++++---------- source/ps/Replay.cpp | 2 +- source/ps/VisualReplay.cpp | 2 +- source/ps/scripting/JSInterface_Game.cpp | 4 +-- source/ps/scripting/JSInterface_SavedGame.cpp | 4 +-- .../GameInterface/Handlers/MapHandlers.cpp | 2 +- 10 files changed, 34 insertions(+), 46 deletions(-) diff --git a/source/network/scripting/JSInterface_Network.cpp b/source/network/scripting/JSInterface_Network.cpp index 76533aa2a4..2753986ff9 100644 --- a/source/network/scripting/JSInterface_Network.cpp +++ b/source/network/scripting/JSInterface_Network.cpp @@ -66,7 +66,7 @@ void JSI_Network::StartNetworkHost(ScriptInterface::CxPrivate* pCxPrivate, const return; } - g_Game = new CGame(); + g_Game = new CGame(true); g_NetClient = new CNetClient(g_Game, true); g_NetClient->SetUserName(playerName); g_NetClient->SetHostingPlayerName(hostLobbyName); @@ -116,7 +116,7 @@ void JSI_Network::StartNetworkJoin(ScriptInterface::CxPrivate* pCxPrivate, const SDL_Delay(1000); } - g_Game = new CGame(); + g_Game = new CGame(true); g_NetClient = new CNetClient(g_Game, false); g_NetClient->SetUserName(playerName); g_NetClient->SetHostingPlayerName(hostJID.substr(0, hostJID.find("@"))); diff --git a/source/network/tests/test_Net.h b/source/network/tests/test_Net.h index fa04b75116..ecbf928bac 100644 --- a/source/network/tests/test_Net.h +++ b/source/network/tests/test_Net.h @@ -144,9 +144,9 @@ public: std::vector clients; - CGame client1Game(true); - CGame client2Game(true); - CGame client3Game(true); + CGame client1Game(false); + CGame client2Game(false); + CGame client3Game(false); CNetServer server; @@ -209,9 +209,9 @@ public: std::vector clients; - CGame client1Game(true); - CGame client2Game(true); - CGame client3Game(true); + CGame client1Game(false); + CGame client2Game(false); + CGame client3Game(false); CNetServer server; @@ -270,7 +270,7 @@ public: debug_printf("==== Connecting client 2B\n"); - CGame client2BGame(true); + CGame client2BGame(false); CNetClient client2B(&client2BGame, false); client2B.SetUserName(L"bob"); clients.push_back(&client2B); diff --git a/source/ps/Game.cpp b/source/ps/Game.cpp index 1e711005d1..7fc6e4b76d 100644 --- a/source/ps/Game.cpp +++ b/source/ps/Game.cpp @@ -63,10 +63,10 @@ CGame *g_Game=NULL; * Constructor * **/ -CGame::CGame(bool disableGraphics, bool replayLog): +CGame::CGame(bool replayLog): m_World(new CWorld(this)), m_Simulation2(new CSimulation2(&m_World->GetUnitManager(), g_ScriptRuntime, m_World->GetTerrain())), - m_GameView(disableGraphics ? NULL : new CGameView(this)), + m_GameView(CRenderer::IsInitialised() ? new CGameView(this) : nullptr), m_GameStarted(false), m_Paused(false), m_SimRate(1.0f), diff --git a/source/ps/Game.h b/source/ps/Game.h index 42bcbdd562..c11e3e1026 100644 --- a/source/ps/Game.h +++ b/source/ps/Game.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2017 Wildfire Games. +/* Copyright (C) 2019 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -81,7 +81,7 @@ class CGame CTurnManager* m_TurnManager; public: - CGame(bool disableGraphics = false, bool replayLog = true); + CGame(bool replayLog); ~CGame(); /** @@ -141,16 +141,6 @@ public: return m_GameStarted; } - /** - * Get if the graphics is disabled. - * - * @return bool true if the m_GameView is NULL, false otherwise. - */ - inline bool IsGraphicsDisabled() const - { - return !m_GameView; - } - /** * Get m_IsVisualReplay. * diff --git a/source/ps/GameSetup/GameSetup.cpp b/source/ps/GameSetup/GameSetup.cpp index b83f839aa2..d35b2f52df 100644 --- a/source/ps/GameSetup/GameSetup.cpp +++ b/source/ps/GameSetup/GameSetup.cpp @@ -696,17 +696,15 @@ static void ShutdownSDL() void EndGame() { - const bool nonVisual = g_Game && g_Game->IsGraphicsDisabled(); - if (g_Game && g_Game->IsGameStarted() && !g_Game->IsVisualReplay() && - g_AtlasGameLoop && !g_AtlasGameLoop->running && !nonVisual) + g_AtlasGameLoop && !g_AtlasGameLoop->running && CRenderer::IsInitialised()) VisualReplay::SaveReplayMetadata(g_GUI->GetActiveGUI()->GetScriptInterface().get()); SAFE_DELETE(g_NetClient); SAFE_DELETE(g_NetServer); SAFE_DELETE(g_Game); - if (!nonVisual) + if (CRenderer::IsInitialised()) { ISoundManager::CloseGame(); g_Renderer.ResetState(); @@ -715,7 +713,7 @@ void EndGame() void Shutdown(int flags) { - const bool nonVisual = g_Game && g_Game->IsGraphicsDisabled(); + const bool hasRenderer = CRenderer::IsInitialised(); if ((flags & SHUTDOWN_FROM_CONFIG)) goto from_config; @@ -732,11 +730,10 @@ void Shutdown(int flags) delete &g_TexMan; TIMER_END(L"shutdown TexMan"); - // destroy renderer if it was initialised - if (!nonVisual) + if (hasRenderer) { TIMER_BEGIN(L"shutdown Renderer"); - delete &g_Renderer; + g_Renderer.~CRenderer(); g_VBMan.Shutdown(); TIMER_END(L"shutdown Renderer"); } @@ -750,7 +747,7 @@ void Shutdown(int flags) ShutdownSDL(); TIMER_END(L"shutdown SDL"); - if (!nonVisual) + if (hasRenderer) g_VideoMode.Shutdown(); TIMER_BEGIN(L"shutdown UserReporter"); @@ -1259,8 +1256,7 @@ bool Autostart(const CmdLineArgs& args) if (autoStartName.empty()) return false; - const bool nonVisual = args.Has("autostart-nonvisual"); - g_Game = new CGame(nonVisual, !args.Has("autostart-disable-replay")); + g_Game = new CGame(!args.Has("autostart-disable-replay")); ScriptInterface& scriptInterface = g_Game->GetSimulation2()->GetScriptInterface(); JSContext* cx = scriptInterface.GetContext(); @@ -1532,7 +1528,7 @@ bool Autostart(const CmdLineArgs& args) FromJSVal_vector(cx, triggerScripts, triggerScriptsVector); } - if (nonVisual) + if (!CRenderer::IsInitialised()) { CStr nonVisualScript = "scripts/NonVisualTrigger.js"; triggerScriptsVector.push_back(nonVisualScript.FromUTF8()); @@ -1626,14 +1622,16 @@ bool Autostart(const CmdLineArgs& args) g_Game->StartGame(&attrs, ""); - if (nonVisual) + if (CRenderer::IsInitialised()) + { + InitPsAutostart(false, attrs); + } + else { // TODO: Non progressive load can fail - need a decent way to handle this LDR_NonprogressiveLoad(); ENSURE(g_Game->ReallyStartGame() == PSRETURN_OK); } - else - InitPsAutostart(false, attrs); } return true; @@ -1644,7 +1642,7 @@ bool AutostartVisualReplay(const std::string& replayFile) if (!FileExists(OsPath(replayFile))) return false; - g_Game = new CGame(false, false); + g_Game = new CGame(false); g_Game->SetPlayerID(-1); g_Game->StartVisualReplay(replayFile); diff --git a/source/ps/Replay.cpp b/source/ps/Replay.cpp index 472b25b068..ac3b1ded19 100644 --- a/source/ps/Replay.cpp +++ b/source/ps/Replay.cpp @@ -183,7 +183,7 @@ void CReplayPlayer::Replay(const bool serializationtest, const int rejointesttur Mod::CacheEnabledModVersions(g_ScriptRuntime); - g_Game = new CGame(true, false); + g_Game = new CGame(false); if (serializationtest) g_Game->GetSimulation2()->EnableSerializationTest(); if (rejointestturn > 0) diff --git a/source/ps/VisualReplay.cpp b/source/ps/VisualReplay.cpp index 0c0a05ac45..fcdb4e00aa 100644 --- a/source/ps/VisualReplay.cpp +++ b/source/ps/VisualReplay.cpp @@ -65,7 +65,7 @@ bool VisualReplay::StartVisualReplay(const OsPath& directory) if (!FileExists(replayFile)) return false; - g_Game = new CGame(false, false); + g_Game = new CGame(false); return g_Game->StartVisualReplay(replayFile); } diff --git a/source/ps/scripting/JSInterface_Game.cpp b/source/ps/scripting/JSInterface_Game.cpp index e48ac8fc03..c6752ccf41 100644 --- a/source/ps/scripting/JSInterface_Game.cpp +++ b/source/ps/scripting/JSInterface_Game.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2018 Wildfire Games. +/* Copyright (C) 2019 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -39,7 +39,7 @@ void JSI_Game::StartGame(ScriptInterface::CxPrivate* pCxPrivate, JS::HandleValue ENSURE(!g_NetClient); ENSURE(!g_Game); - g_Game = new CGame(); + g_Game = new CGame(true); // Convert from GUI script context to sim script context CSimulation2* sim = g_Game->GetSimulation2(); diff --git a/source/ps/scripting/JSInterface_SavedGame.cpp b/source/ps/scripting/JSInterface_SavedGame.cpp index 4a1b5eb030..47c76650ec 100644 --- a/source/ps/scripting/JSInterface_SavedGame.cpp +++ b/source/ps/scripting/JSInterface_SavedGame.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2018 Wildfire Games. +/* Copyright (C) 2019 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -83,7 +83,7 @@ JS::Value JSI_SavedGame::StartSavedGame(ScriptInterface::CxPrivate* pCxPrivate, if (err < 0) return JS::UndefinedValue(); - g_Game = new CGame(); + g_Game = new CGame(true); { CSimulation2* sim = g_Game->GetSimulation2(); diff --git a/source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp b/source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp index 653d3411a1..21393d2edd 100644 --- a/source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp +++ b/source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp @@ -57,7 +57,7 @@ namespace g_Game = NULL; } - g_Game = new CGame(false, false); + g_Game = new CGame(false); // Default to player 1 for playtesting g_Game->SetPlayerID(1);