Browse Source

Remove `console` global

Ivan Savenko 7 months ago
parent
commit
782362e5ce

+ 13 - 9
clientapp/EntryPoint.cpp

@@ -210,8 +210,13 @@ int main(int argc, char * argv[])
 	CStopWatch total;
 	CStopWatch pomtime;
 	std::cout.flags(std::ios::unitbuf);
+
+	setThreadNameLoggingOnly("MainGUI");
+	boost::filesystem::path logPath = VCMIDirs::get().userLogsPath() / "VCMI_Client_log.txt";
+	if(vm.count("logLocation"))
+		logPath = vm["logLocation"].as<std::string>() + "/VCMI_Client_log.txt";
+
 #ifndef VCMI_IOS
-	console = new CConsoleHandler();
 
 	auto callbackFunction = [](std::string buffer, bool calledFromIngameConsole)
 	{
@@ -219,15 +224,14 @@ int main(int argc, char * argv[])
 		commandController.processCommand(buffer, calledFromIngameConsole);
 	};
 
-	*console->cb = callbackFunction;
-	console->start();
+	CConsoleHandler console(callbackFunction);
+	console.start();
+
+	logConfig = new CBasicLogConfigurator(logPath, &console);
+#else
+	logConfig = new CBasicLogConfigurator(logPath, nullptr);
 #endif
 
-	setThreadNameLoggingOnly("MainGUI");
-	boost::filesystem::path logPath = VCMIDirs::get().userLogsPath() / "VCMI_Client_log.txt";
-	if(vm.count("logLocation"))
-		logPath = vm["logLocation"].as<std::string>() + "/VCMI_Client_log.txt";
-	logConfig = new CBasicLogConfigurator(logPath, console);
 	logConfig->configureDefault();
 	logGlobal->info("Starting client of '%s'", GameConstants::VCMI_VERSION);
 	logGlobal->info("Creating console and configuring logger: %d ms", pomtime.getDiff());
@@ -236,7 +240,7 @@ int main(int argc, char * argv[])
 	// Init filesystem and settings
 	try
 	{
-		preinitDLL(::console, false);
+		preinitDLL(false);
 	}
 	catch (const DataLoadingException & e)
 	{

+ 5 - 2
launcher/mainwindow_moc.cpp

@@ -31,9 +31,12 @@ void MainWindow::load()
 	QDir::setCurrent(QApplication::applicationDirPath());
 
 #ifndef VCMI_MOBILE
-	console = new CConsoleHandler();
+	console = std::make_unique<CConsoleHandler>();
+	CBasicLogConfigurator logConfig(VCMIDirs::get().userLogsPath() / "VCMI_Launcher_log.txt", console.get());
+#else
+	CBasicLogConfigurator logConfig(VCMIDirs::get().userLogsPath() / "VCMI_Launcher_log.txt", nullptr);
 #endif
-	CBasicLogConfigurator logConfig(VCMIDirs::get().userLogsPath() / "VCMI_Launcher_log.txt", console);
+
 	logConfig.configureDefault();
 
 	try

+ 8 - 0
launcher/mainwindow_moc.h

@@ -12,6 +12,10 @@
 #include <QStringList>
 #include <QTranslator>
 
+VCMI_LIB_NAMESPACE_BEGIN
+class CConsoleHandler;
+VCMI_LIB_NAMESPACE_END
+
 namespace Ui
 {
 class MainWindow;
@@ -40,6 +44,10 @@ class MainWindow : public QMainWindow
 #endif
 	Ui::MainWindow * ui;
 
+#ifndef VCMI_MOBILE
+	std::unique_ptr<CConsoleHandler> console;
+#endif
+
 	void load();
 
 	enum TabRows

+ 15 - 20
lib/CConsoleHandler.cpp

@@ -15,12 +15,6 @@
 
 #include <boost/stacktrace.hpp>
 
-VCMI_LIB_NAMESPACE_BEGIN
-
-std::mutex CConsoleHandler::smx;
-
-DLL_LINKAGE CConsoleHandler * console = nullptr;
-
 VCMI_LIB_NAMESPACE_END
 
 #if defined(NDEBUG) && !defined(VCMI_ANDROID)
@@ -245,12 +239,12 @@ void CConsoleHandler::setColor(EConsoleTextColor::EConsoleTextColor color)
 #endif
 }
 
-int CConsoleHandler::run() const
+int CConsoleHandler::run()
 {
 	setThreadName("consoleHandler");
 	//disabling sync to make in_avail() work (othervice always returns 0)
 	{
-		TLockGuard _(smx);
+		std::lock_guard guard(smx);
 		std::ios::sync_with_stdio(false);
 	}
 	std::string buffer;
@@ -262,8 +256,8 @@ int CConsoleHandler::run() const
 		if (std::cin.rdbuf()->in_avail())
 		{
 			if ( getline(std::cin, buffer).good() )
-				if ( cb && *cb )
-					(*cb)(buffer, false);
+				if ( cb )
+					cb(buffer, false);
 		}
 		else
 			boost::this_thread::sleep_for(boost::chrono::milliseconds(100));
@@ -277,9 +271,13 @@ int CConsoleHandler::run() const
 	}
 	return -1;
 }
-CConsoleHandler::CConsoleHandler():
-	cb(new std::function<void(const std::string &, bool)>), 
-	thread(nullptr)
+
+CConsoleHandler::CConsoleHandler()
+	:CConsoleHandler(std::function<void(const std::string &, bool)>{})
+{}
+
+CConsoleHandler::CConsoleHandler(std::function<void(const std::string &, bool)> callback)
+	:cb(callback)
 {
 #ifdef VCMI_WINDOWS
 	handleIn = GetStdHandle(STD_INPUT_HANDLE);
@@ -309,27 +307,24 @@ CConsoleHandler::~CConsoleHandler()
 {
 	logGlobal->info("Killing console...");
 	end();
-	delete cb;
 	logGlobal->info("Killing console... done!");
 }
 void CConsoleHandler::end()
 {
-	if (thread)
+	if (thread.joinable())
 	{
 #ifndef VCMI_WINDOWS
-		thread->interrupt();
+		thread.interrupt();
 #else
 		TerminateThread(thread->native_handle(),0);
 #endif
-		thread->join();
-		delete thread;
-		thread = nullptr;
+		thread.join();
 	}
 }
 
 void CConsoleHandler::start()
 {
-	thread = new boost::thread(std::bind(&CConsoleHandler::run,console));
+	thread = boost::thread(std::bind(&CConsoleHandler::run, this));
 }
 
 VCMI_LIB_NAMESPACE_END

+ 47 - 50
lib/CConsoleHandler.h

@@ -16,14 +16,14 @@ namespace EConsoleTextColor
 /** The color enum is used for colored text console output. */
 enum EConsoleTextColor
 {
-    DEFAULT = -1,
-    GREEN,
-    RED,
-    MAGENTA,
-    YELLOW,
-    WHITE,
-    GRAY,
-    TEAL = -2
+	DEFAULT = -1,
+	GREEN,
+	RED,
+	MAGENTA,
+	YELLOW,
+	WHITE,
+	GRAY,
+	TEAL = -2
 };
 }
 
@@ -32,67 +32,64 @@ enum EConsoleTextColor
 class DLL_LINKAGE CConsoleHandler
 {
 public:
-    CConsoleHandler();
-    ~CConsoleHandler();
-    void start(); //starts listening thread
+	CConsoleHandler(std::function<void(const std::string &, bool)> callback);
+	CConsoleHandler();
+	~CConsoleHandler();
+	void start(); //starts listening thread
 
-    template<typename T> void print(const T &data, bool addNewLine = false, EConsoleTextColor::EConsoleTextColor color = EConsoleTextColor::DEFAULT, bool printToStdErr = false)
+	template<typename T> void print(const T &data, bool addNewLine = false, EConsoleTextColor::EConsoleTextColor color = EConsoleTextColor::DEFAULT, bool printToStdErr = false)
 	{
-        TLockGuard _(smx);
+		TLockGuard _(smx);
 #ifndef VCMI_WINDOWS
 		// with love from ffmpeg - library is trying to print some warnings from separate thread
 		// this results in broken console on Linux. Lock stdout to print all our data at once
 		flockfile(stdout);
 #endif
-        if(color != EConsoleTextColor::DEFAULT) setColor(color);
-        if(printToStdErr)
-        {
-            std::cerr << data;
-            if(addNewLine)
-            {
-                std::cerr << std::endl;
-            }
-            else
-            {
-                std::cerr << std::flush;
-            }
-        }
-        else
-        {
-            std::cout << data;
-            if(addNewLine)
-            {
-                std::cout << std::endl;
-            }
-            else
-            {
-                std::cout << std::flush;
-            }
-        }
+		if(color != EConsoleTextColor::DEFAULT) setColor(color);
+		if(printToStdErr)
+		{
+			std::cerr << data;
+			if(addNewLine)
+			{
+				std::cerr << std::endl;
+			}
+			else
+			{
+				std::cerr << std::flush;
+			}
+		}
+		else
+		{
+			std::cout << data;
+			if(addNewLine)
+			{
+				std::cout << std::endl;
+			}
+			else
+			{
+				std::cout << std::flush;
+			}
+		}
 
-        if(color != EConsoleTextColor::DEFAULT) setColor(EConsoleTextColor::DEFAULT);
+		if(color != EConsoleTextColor::DEFAULT) setColor(EConsoleTextColor::DEFAULT);
 #ifndef VCMI_WINDOWS
 		funlockfile(stdout);
 #endif
 	}
-	//function to be called when message is received - string: message, bool: whether call was made from in-game console
-	std::function<void(const std::string &, bool)> *cb;
 
 private:
-	int run() const;
+	int run();
 
 	void end(); //kills listening thread
 
-	static void setColor(EConsoleTextColor::EConsoleTextColor color); //sets color of text appropriate for given logging level
+	void setColor(EConsoleTextColor::EConsoleTextColor color); //sets color of text appropriate for given logging level
+
+	//function to be called when message is received - string: message, bool: whether call was made from in-game console
+	std::function<void(const std::string &, bool)> cb;
 
-	/// FIXME: Implement CConsoleHandler as singleton, move some logic into CLogConsoleTarget, etc... needs to be discussed:)
-	/// Without static, application will crash complaining about mutex deleted. In short: CConsoleHandler gets deleted before
-	/// the logging system.
-	static std::mutex smx;
+	std::mutex smx;
 
-	boost::thread * thread;
+	boost::thread thread;
 };
 
-extern DLL_LINKAGE CConsoleHandler * console;
-
 VCMI_LIB_NAMESPACE_END

+ 1 - 3
lib/GameLibrary.cpp

@@ -32,7 +32,6 @@
 #include "CStopWatch.h"
 #include "VCMIDirs.h"
 #include "filesystem/Filesystem.h"
-#include "CConsoleHandler.h"
 #include "rmg/CRmgTemplateStorage.h"
 #include "mapObjectConstructors/CObjectClassesHandler.h"
 #include "mapObjects/CObjectHandler.h"
@@ -47,9 +46,8 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 GameLibrary * LIBRARY = nullptr;
 
-DLL_LINKAGE void preinitDLL(CConsoleHandler * Console, bool extractArchives)
+DLL_LINKAGE void preinitDLL(bool extractArchives)
 {
-	console = Console;
 	LIBRARY = new GameLibrary();
 	LIBRARY->loadFilesystem(extractArchives);
 	settings.init("config/settings.json", "vcmi:settings");

+ 1 - 1
lib/GameLibrary.h

@@ -119,7 +119,7 @@ public:
 
 extern DLL_LINKAGE GameLibrary * LIBRARY;
 
-DLL_LINKAGE void preinitDLL(CConsoleHandler * Console, bool extractArchives);
+DLL_LINKAGE void preinitDLL(bool extractArchives);
 DLL_LINKAGE void loadDLLClasses(bool onlyEssential = false);
 
 

+ 2 - 2
lobby/EntryPoint.cpp

@@ -24,9 +24,9 @@ int main(int argc, const char * argv[])
 	CResourceHandler::load("config/filesystem.json"); // FIXME: we actually need only config directory for schemas, can be reduced
 
 #ifndef VCMI_IOS
-	console = new CConsoleHandler();
+	CConsoleHandler console;
 #endif
-	CBasicLogConfigurator logConfig(VCMIDirs::get().userLogsPath() / "VCMI_Lobby_log.txt", console);
+	CBasicLogConfigurator logConfig(VCMIDirs::get().userLogsPath() / "VCMI_Lobby_log.txt", &console);
 	logConfig.configureDefault();
 
 	auto databasePath = VCMIDirs::get().userDataPath() / "vcmiLobby.db";

+ 3 - 3
mapeditor/mainwindow.cpp

@@ -184,14 +184,14 @@ MainWindow::MainWindow(QWidget* parent) :
 
 	//configure logging
 	const boost::filesystem::path logPath = VCMIDirs::get().userLogsPath() / "VCMI_Editor_log.txt";
-	console = new CConsoleHandler();
-	logConfig = new CBasicLogConfigurator(logPath, console);
+	console = std::make_unique<CConsoleHandler>();
+	logConfig = new CBasicLogConfigurator(logPath, console.get());
 	logConfig->configureDefault();
 	logGlobal->info("Starting map editor of '%s'", GameConstants::VCMI_VERSION);
 	logGlobal->info("The log file will be saved to %s", logPath);
 
 	//init
-	preinitDLL(::console, extractionOptions.extractArchives);
+	preinitDLL(extractionOptions.extractArchives);
 
 	// Initialize logging based on settings
 	logConfig->configure();

+ 6 - 1
mapeditor/mainwindow.h

@@ -12,6 +12,7 @@ class ObjectBrowserProxyModel;
 VCMI_LIB_NAMESPACE_BEGIN
 class CMap;
 class CampaignState;
+class CConsoleHandler;
 class CGObjectInstance;
 VCMI_LIB_NAMESPACE_END
 
@@ -34,7 +35,11 @@ class MainWindow : public QMainWindow
 #ifdef ENABLE_QT_TRANSLATIONS
 	QTranslator translator;
 #endif
-	
+
+#ifndef VCMI_MOBILE
+	std::unique_ptr<CConsoleHandler> console;
+#endif
+
 	std::unique_ptr<CMap> openMapInternal(const QString &);
 	std::shared_ptr<CampaignState> openCampaignInternal(const QString &);
 

+ 3 - 3
serverapp/EntryPoint.cpp

@@ -71,14 +71,14 @@ int main(int argc, const char * argv[])
 	// Correct working dir executable folder (not bundle folder) so we can use executable relative paths
 	boost::filesystem::current_path(boost::filesystem::system_complete(argv[0]).parent_path());
 
-	console = new CConsoleHandler();
-	CBasicLogConfigurator logConfig(VCMIDirs::get().userLogsPath() / "VCMI_Server_log.txt", console);
+	CConsoleHandler console;
+	CBasicLogConfigurator logConfig(VCMIDirs::get().userLogsPath() / "VCMI_Server_log.txt", &console);
 	logConfig.configureDefault();
 	logGlobal->info(SERVER_NAME);
 
 	boost::program_options::variables_map opts;
 	handleCommandOptions(argc, argv, opts);
-	preinitDLL(console, false);
+	preinitDLL(false);
 	logConfig.configure();
 
 	loadDLLClasses();

+ 1 - 3
test/CVcmiTestConfig.cpp

@@ -11,7 +11,6 @@
 #include "StdInc.h"
 #include "CVcmiTestConfig.h"
 
-#include "../lib/CConsoleHandler.h"
 #include "../lib/logging/CBasicLogConfigurator.h"
 #include "../lib/VCMIDirs.h"
 #include "../lib/GameLibrary.h"
@@ -23,8 +22,7 @@
 
 void CVcmiTestConfig::SetUp()
 {
-	console = new CConsoleHandler();
-	preinitDLL(console, true);
+	preinitDLL(true);
 	loadDLLClasses(true);
 
 	/* TEST_DATA_DIR may be wrong, if yes below test don't run,