phosit/0ad
1
0
Fork 0
forked from 0ad/0ad
Code Pull Requests Activity

Don't use std::shared_ptr to store m_ScriptContext and m_ScriptInterface in the CGUIManager

Browse Source 
`std::shared_ptr` is intrusive. When a function expects a
`std::shared_ptr` the caller has to use it too and can't store the
element on the stack for example.

Comments by: @vladislavbelov
Differential Revision: https://code.wildfiregames.com/D5221
This was SVN commit r28131.
This commit is contained in:
phosit 2024-06-27 19:09:30 +00:00
parent 75753abd2e
commit 4bcbc72274
12 changed files with 67 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
source/gui/CGUI.cpp
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2022 Wildfire Games.
/* Copyright (C) 2024 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -95,7 +95,7 @@ void CollectVisibleObjectsRecursively(const std::vector<IGUIObject*>& objects, C
} // anonynous namespace
CGUI::CGUI(const std::shared_ptr<ScriptContext>& context)
CGUI::CGUI(ScriptContext& context)
: m_BaseObject(std::make_unique<CGUIDummyObject>(*this)),
m_FocusedObject(nullptr),
m_InternalNameNumber(0),

4
source/gui/CGUI.h
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2022 Wildfire Games.
/* Copyright (C) 2024 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -66,7 +66,7 @@ private:
using ConstructObjectFunction = IGUIObject* (*)(CGUI&);
public:
CGUI(const std::shared_ptr<ScriptContext>& context);
CGUI(ScriptContext& context);
~CGUI();
/**

19
source/gui/GUIManager.cpp
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2023 Wildfire Games.
/* Copyright (C) 2024 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -21,6 +21,7 @@
#include "gui/CGUI.h"
#include "lib/timer.h"
#include "lobby/IXmppClient.h"
#include "ps/CLogger.h"
#include "ps/Filesystem.h"
#include "ps/GameSetup/Config.h"
@ -67,12 +68,12 @@ static Status ReloadChangedFileCB(void* param, const VfsPath& path)
return static_cast<CGUIManager*>(param)->ReloadChangedFile(path);
}
CGUIManager::CGUIManager()
CGUIManager::CGUIManager(ScriptContext& scriptContext, ScriptInterface& scriptInterface) :
m_ScriptContext{scriptContext},
m_ScriptInterface{scriptInterface}
{
m_ScriptContext = g_ScriptContext;
m_ScriptInterface.reset(new ScriptInterface("Engine", "GUIManager", m_ScriptContext));
m_ScriptInterface->SetCallbackData(this);
m_ScriptInterface->LoadGlobalScripts();
m_ScriptInterface.SetCallbackData(this);
m_ScriptInterface.LoadGlobalScripts();
if (!CXeromyces::AddValidator(g_VFS, "gui_page", "gui/gui_page.rng"))
LOGERROR("CGUIManager: failed to load GUI page grammar file 'gui/gui_page.rng'");
@ -119,7 +120,7 @@ void CGUIManager::PushPage(const CStrW& pageName, Script::StructuredClone initDa
// Store the callback handler in the current GUI page before opening the new one
if (!m_PageStack.empty() && !callbackFunction.isUndefined())
{
m_PageStack.back().SetCallbackFunction(*m_ScriptInterface, callbackFunction);
m_PageStack.back().SetCallbackFunction(m_ScriptInterface, callbackFunction);
// Make sure we unfocus anything on the current page.
m_PageStack.back().gui->SendFocusMessage(GUIM_LOST_FOCUS);
@ -154,7 +155,7 @@ CGUIManager::SGUIPage::SGUIPage(const CStrW& pageName, const Script::StructuredC
{
}
void CGUIManager::SGUIPage::LoadPage(std::shared_ptr<ScriptContext> scriptContext)
void CGUIManager::SGUIPage::LoadPage(ScriptContext& scriptContext)
{
// If we're hotloading then try to grab some data from the previous page
Script::StructuredClone hotloadData;
@ -380,7 +381,7 @@ void CGUIManager::TickObjects()
// We share the script context with everything else that runs in the same thread.
// This call makes sure we trigger GC regularly even if the simulation is not running.
m_ScriptInterface->GetContext().MaybeIncrementalGC(1.0f);
m_ScriptContext.MaybeIncrementalGC(1.0f);
// Save an immutable copy so iterators aren't invalidated by tick handlers
PageStackType pageStack = m_PageStack;

14
source/gui/GUIManager.h
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2022 Wildfire Games.
/* Copyright (C) 2024 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -44,14 +44,14 @@ class CGUIManager
{
NONCOPYABLE(CGUIManager);
public:
CGUIManager();
CGUIManager(ScriptContext& scriptContext, ScriptInterface& scriptInterface);
~CGUIManager();
std::shared_ptr<ScriptInterface> GetScriptInterface()
ScriptInterface& GetScriptInterface()
{
return m_ScriptInterface;
}
std::shared_ptr<ScriptContext> GetContext() { return m_ScriptContext; }
ScriptContext& GetContext() { return m_ScriptContext; }
std::shared_ptr<CGUI> GetActiveGUI() { return top(); }
/**
@ -143,7 +143,7 @@ private:
/**
* Create the CGUI with it's own ScriptInterface. Deletes the previous CGUI if it existed.
*/
void LoadPage(std::shared_ptr<ScriptContext> scriptContext);
void LoadPage(ScriptContext& scriptContext);
/**
* Sets the callback handler when a new page is opened that will be performed when the page is closed.
@ -169,8 +169,8 @@ private:
std::shared_ptr<CGUI> top() const;
std::shared_ptr<ScriptContext> m_ScriptContext;
std::shared_ptr<ScriptInterface> m_ScriptInterface;
ScriptContext& m_ScriptContext;
ScriptInterface& m_ScriptInterface;
/**
* The page stack must not move pointers on push/pop, or pushing a page in a page's init method

14
source/gui/tests/test_CGUIText.h
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2022 Wildfire Games.
/* Copyright (C) 2024 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -73,13 +73,13 @@ public:
void test_empty()
{
CGUI gui(g_ScriptContext);
CGUI gui{*g_ScriptContext};
CGUIText empty;
}
void test_wrapping()
{
CGUI gui(g_ScriptContext);
CGUI gui{*g_ScriptContext};
const CStrW font = L"console";
// Make sure this matches the value of the file.
@ -186,7 +186,7 @@ public:
// +------------------------------+
//
CGUI gui(g_ScriptContext);
CGUI gui{*g_ScriptContext};
const CStrW firstWord = L"Shortword";
const CStrW secondWord = L"(Veryverylongword)";
const CStrW text = firstWord + L" " + secondWord;
@ -266,7 +266,7 @@ public:
void test_overflow()
{
CGUI gui(g_ScriptContext);
CGUI gui{*g_ScriptContext};
const CStrW font = L"console";
// Make sure this matches the value of the file.
@ -318,7 +318,7 @@ public:
{
TS_ASSERT_OK(g_VFS->Mount(L"", DataDir() / "mods" / "mod" / "", VFS_MOUNT_MUST_EXIST));
CGUI gui(g_ScriptContext);
CGUI gui{*g_ScriptContext};
const CStrW font = L"sans-bold-13";
CGUIString string;
@ -334,7 +334,7 @@ public:
void test_multiple_blank_spaces()
{
CGUI gui(g_ScriptContext);
CGUI gui{*g_ScriptContext};
const CStrW font = L"console";
// Make sure this matches the value of the file.

4
source/gui/tests/test_GUISetting.h
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2023 Wildfire Games.
/* Copyright (C) 2024 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 @@ public:
void test_movability()
{
CGUI gui(g_ScriptContext);
CGUI gui{*g_ScriptContext};
TestGUIObject object(gui);
static_assert(std::is_move_constructible_v<CGUISimpleSetting<CStr>>);

17
source/gui/tests/test_GuiManager.h
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2023 Wildfire Games.
/* Copyright (C) 2024 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -32,10 +32,12 @@
#include "scriptinterface/Object.h"
#include <memory>
#include <optional>
class TestGuiManager : public CxxTest::TestSuite
{
std::unique_ptr<CConfigDB> configDB;
std::optional<ScriptInterface> scriptInterface;
public:
void setUp()
@ -48,12 +50,14 @@ public:
CXeromyces::Startup();
g_GUI = new CGUIManager();
scriptInterface.emplace("Engine", "GUIManager", *g_ScriptContext);
g_GUI = new CGUIManager{*g_ScriptContext, *scriptInterface};
}
void tearDown()
{
delete g_GUI;
scriptInterface.reset();
CXeromyces::Terminate();
configDB.reset();
g_VFS.reset();
@ -63,8 +67,7 @@ public:
void test_EventObject()
{
// Load up a test page.
const ScriptInterface& scriptInterface = *(g_GUI->GetScriptInterface());
ScriptRequest rq(scriptInterface);
ScriptRequest rq{g_GUI->GetScriptInterface()};
JS::RootedValue val(rq.cx);
Script::CreateObject(rq, &val);
@ -127,8 +130,7 @@ public:
LoadHotkeys(*configDB);
// Load up a test page.
const ScriptInterface& scriptInterface = *(g_GUI->GetScriptInterface());
ScriptRequest rq(scriptInterface);
ScriptRequest rq{g_GUI->GetScriptInterface()};
JS::RootedValue val(rq.cx);
Script::CreateObject(rq, &val);
@ -202,8 +204,7 @@ public:
void test_PageRegainedFocusEvent()
{
// Load up a test page.
const ScriptInterface& scriptInterface = *(g_GUI->GetScriptInterface());
ScriptRequest rq(scriptInterface);
ScriptRequest rq{g_GUI->GetScriptInterface()};
JS::RootedValue val(rq.cx);
Script::CreateObject(rq, &val);

6
source/lobby/scripting/JSInterface_Lobby.cpp
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2021 Wildfire Games.
/* Copyright (C) 2024 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -56,7 +56,7 @@ void StartXmppClient(const ScriptRequest& rq, const std::wstring& username, cons
g_XmppClient =
IXmppClient::create(
g_GUI->GetScriptInterface().get(),
&g_GUI->GetScriptInterface(),
utf8_from_wstring(username),
utf8_from_wstring(password),
utf8_from_wstring(room),
@ -76,7 +76,7 @@ void StartRegisterXmppClient(const ScriptRequest& rq, const std::wstring& userna
g_XmppClient =
IXmppClient::create(
g_GUI->GetScriptInterface().get(),
&g_GUI->GetScriptInterface(),
utf8_from_wstring(username),
utf8_from_wstring(password),
std::string(),

6
source/main.cpp
View File

@ -678,9 +678,12 @@ static void RunGameOrAtlas(const PS::span<const char* const> argv)
g_Mods.UpdateAvailableMods(modInterface);
}
std::optional<ScriptInterface> guiScriptInterface;
if (isVisual)
{
InitGraphics(args, 0, installedMods);
guiScriptInterface.emplace("Engine", "gui", *g_ScriptContext);
InitGraphics(args, 0, installedMods, *g_ScriptContext, *guiScriptInterface);
MainControllerInit();
}
else if (!InitNonVisual(args))
@ -709,6 +712,7 @@ static void RunGameOrAtlas(const PS::span<const char* const> argv)
modsToInstall.clear();
ShutdownNetworkAndUI();
guiScriptInterface.reset();
ShutdownConfigAndSubsequent();
MainControllerShutdown();
flags &= ~INIT_MODS;

17
source/ps/GameSetup/GameSetup.cpp
View File

@ -612,7 +612,8 @@ bool Init(const CmdLineArgs& args, int flags)
return true;
}
void InitGraphics(const CmdLineArgs& args, int flags, const std::vector<CStr>& installedMods)
void InitGraphics(const CmdLineArgs& args, int flags, const std::vector<CStr>& installedMods,
ScriptContext& scriptContext, ScriptInterface& scriptInterface)
{
const bool setup_vmode = (flags & INIT_HAVE_VMODE) == 0;
@ -636,7 +637,7 @@ void InitGraphics(const CmdLineArgs& args, int flags, const std::vector<CStr>& i
if(g_DisableAudio)
ISoundManager::SetEnabled(false);
g_GUI = new CGUIManager();
g_GUI = new CGUIManager{scriptContext, scriptInterface};
CStr8 renderPath = "default";
CFG_GET_VAL("renderpath", renderPath);
@ -668,17 +669,13 @@ void InitGraphics(const CmdLineArgs& args, int flags, const std::vector<CStr>& i
if (!AutostartVisualReplay(args.Get("replay-visual")) && !Autostart(args))
{
const bool setup_gui = ((flags & INIT_NO_GUI) == 0);
// We only want to display the splash screen at startup
std::shared_ptr<ScriptInterface> scriptInterface = g_GUI->GetScriptInterface();
ScriptRequest rq(scriptInterface);
ScriptRequest rq{g_GUI->GetScriptInterface()};
JS::RootedValue data(rq.cx);
if (g_GUI)
{
Script::CreateObject(rq, &data, "isStartup", true);
if (!installedMods.empty())
Script::SetProperty(rq, data, "installedMods", installedMods);
}
InitPs(setup_gui, installedMods.empty() ? L"page_pregame.xml" : L"page_modmod.xml", g_GUI->GetScriptInterface().get(), data);
InitPs(setup_gui, installedMods.empty() ? L"page_pregame.xml" : L"page_modmod.xml", &g_GUI->GetScriptInterface(), data);
}
}
catch (PSERROR_Game_World_MapLoadFailed& e)
@ -858,7 +855,7 @@ bool Autostart(const CmdLineArgs& args)
{
JSI_GUIManager::RegisterScriptFunctions(rq);
// TODO: this loads pregame, which is hardcoded to exist by various code paths. That ought be changed.
InitPs(false, L"page_pregame.xml", g_GUI->GetScriptInterface().get(), JS::UndefinedHandleValue);
InitPs(false, L"page_pregame.xml", &g_GUI->GetScriptInterface(), JS::UndefinedHandleValue);
}
JSI_Game::RegisterScriptFunctions(rq);

5
source/ps/GameSetup/GameSetup.h
View File

@ -24,6 +24,8 @@
class CmdLineArgs;
class Paths;
class ScriptContext;
class ScriptInterface;
/**
* initialize global modules that are be needed before Init.
@ -73,7 +75,8 @@ extern void InitInput();
/**
* `ShutdownNetworkAndUI` has to be called later.
*/
extern void InitGraphics(const CmdLineArgs& args, int flags, const std::vector<CStr>& installedMods = std::vector<CStr>());
void InitGraphics(const CmdLineArgs& args, int flags, const std::vector<CStr>& installedMods,
ScriptContext& scriptContext, ScriptInterface& scriptInterface);
/**
* `ShutdownNetworkAndUI` has to be called later.

7
source/tools/atlas/GameInterface/Handlers/GraphicsSetupHandlers.cpp
View File

@ -42,6 +42,7 @@
#include "renderer/backend/IDevice.h"
#include "renderer/Renderer.h"
#include "renderer/SceneRenderer.h"
#include "scriptinterface/ScriptInterface.h"
#include <optional>
@ -65,6 +66,8 @@ const int g_InitFlags = INIT_HAVE_VMODE | INIT_NO_GUI;
// This isn't used directly. When it's emplaced and when it's reset it does mutate `g_Logger`.
std::optional<FileLogger> g_FileLogger;
std::optional<ScriptInterface> g_ScriptInterface;
}
MESSAGEHANDLER(Init)
@ -131,7 +134,8 @@ MESSAGEHANDLER(InitGraphics)
g_VideoMode.GetBackendDevice()->OnWindowResize(g_xres, g_yres);
InitGraphics(g_AtlasGameLoop->args, g_InitFlags, {});
g_ScriptInterface.emplace("Engine", "GUIManager", *g_ScriptContext);
InitGraphics(g_AtlasGameLoop->args, g_InitFlags, {}, *g_ScriptContext, *g_ScriptInterface);
}
@ -147,6 +151,7 @@ MESSAGEHANDLER(Shutdown)
g_AtlasGameLoop->view = AtlasView::GetView_None();
ShutdownNetworkAndUI();
g_ScriptInterface.reset();
ShutdownConfigAndSubsequent();
g_FileLogger.reset();
}