From 88054cdd84747598345af65600e3b06c6f52fdb1 Mon Sep 17 00:00:00 2001 From: Ghabry Date: Wed, 26 Feb 2025 00:46:47 +0100 Subject: [PATCH] Logging: Address review comments Removed old and broken code in Output::Quit Removed redundant noop_stream declaration Using Config Filesystem as a fallback path for the logging on all systems now --- src/game_config.cpp | 18 +++++++++--------- src/output.cpp | 5 ----- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/game_config.cpp b/src/game_config.cpp index 9164867e8f..c41840e1a3 100644 --- a/src/game_config.cpp +++ b/src/game_config.cpp @@ -276,8 +276,10 @@ Filesystem_Stream::OutputStream Game_Config::GetGlobalConfigFileOutput() { } Filesystem_Stream::OutputStream& Game_Config::GetLogFileOutput() { + // Invalid stream that consumes the output when logging is disabled or an error occurs + static Filesystem_Stream::OutputStream noop_stream; + if (!Player::player_config.log_enabled.Get()) { - static Filesystem_Stream::OutputStream noop_stream; return noop_stream; } @@ -310,14 +312,14 @@ Filesystem_Stream::OutputStream& Game_Config::GetLogFileOutput() { if (!path.empty()) { path = FileFinder::MakePath(path, OUTPUT_FILENAME); } - - // Use the config directory - path = GetGlobalConfigFilesystem().GetFullPath(); - #else - // Use the config directory - path = GetGlobalConfigFilesystem().GetFullPath(); #endif + if (path.empty()) { + // Fallback: Use the config directory + // Can still fail in the rare case that the config path is invalid + path = GetGlobalConfigFilesystem().GetFullPath(); + } + if (!path.empty()) { path = FileFinder::MakePath(path, OUTPUT_FILENAME); } @@ -335,7 +337,6 @@ Filesystem_Stream::OutputStream& Game_Config::GetLogFileOutput() { if (path.empty()) { print_err(); - static Filesystem_Stream::OutputStream noop_stream; return noop_stream; } @@ -343,7 +344,6 @@ Filesystem_Stream::OutputStream& Game_Config::GetLogFileOutput() { // Make Directory not supported on Android, assume the path exists if (!FileFinder::Root().MakeDirectory(FileFinder::GetPathAndFilename(path).first, true)) { print_err(); - static Filesystem_Stream::OutputStream noop_stream; return noop_stream; } #endif diff --git a/src/output.cpp b/src/output.cpp index 1e9470a5cc..b76dc8a45b 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -190,11 +190,6 @@ static void HandleErrorOutput(const std::string& err) { void Output::Quit() { Game_Config::CloseLogFile(); - - if (!Game_Config::GetLogFileOutput()) { - return; - Game_Config::GetLogFileOutput().Close(); - } } bool Output::TakeScreenshot() {