From 3a72e8e4193eb97b80ff637a38a4efbe5c9e384b Mon Sep 17 00:00:00 2001 From: captainurist <73941350+captainurist@users.noreply.github.com> Date: Sun, 3 Nov 2024 11:46:57 +0000 Subject: [PATCH] Add LOG_NONE --- src/Application/GameConfig.h | 2 +- src/Application/Startup/GameStarter.cpp | 4 +- src/Application/Startup/LogStarter.cpp | 57 ++++++++----------- src/Application/Startup/LogStarter.h | 16 +----- src/Library/Logger/LogCategory.h | 4 ++ src/Library/Logger/LogEnumFunctions.h | 1 + src/Library/Logger/LogEnums.cpp | 1 + src/Library/Logger/LogEnums.h | 8 ++- src/Library/Logger/Logger.cpp | 7 +++ src/Library/Logger/Logger.h | 4 +- .../Platform/Sdl/SdlEnumTranslation.cpp | 3 +- src/Media/FFmpegLogSource.cpp | 5 +- 12 files changed, 57 insertions(+), 55 deletions(-) diff --git a/src/Application/GameConfig.h b/src/Application/GameConfig.h index 39022bd02e2c..df900e867d65 100644 --- a/src/Application/GameConfig.h +++ b/src/Application/GameConfig.h @@ -94,7 +94,7 @@ class GameConfig : public Config { Bool NoMargaret = {this, "no_margareth", false, "Disable Margaret's tour messages on Emerald Island."}; ConfigEntry<::LogLevel> LogLevel = {this, "log_level", LOG_ERROR, - "Default log level. One of 'trace', 'debug', 'info', 'warning', 'error' and 'critical'."}; + "Default log level. One of 'none', 'trace', 'debug', 'info', 'warning', 'error' and 'critical'."}; // TODO(captainurist): move all Trace* options into a separate section. diff --git a/src/Application/Startup/GameStarter.cpp b/src/Application/Startup/GameStarter.cpp index 9844589cd6e0..3ae485718b50 100644 --- a/src/Application/Startup/GameStarter.cpp +++ b/src/Application/Startup/GameStarter.cpp @@ -59,7 +59,6 @@ constexpr std::string_view configName = "openenroth.ini"; GameStarter::GameStarter(GameStarterOptions options): _options(std::move(options)) { // Init logging. - _logStarter.initPrimary(); Engine::LogEngineBuildInfo(); try { @@ -77,7 +76,6 @@ void GameStarter::initWithLogger() { // Resolve user path, create user fs & file logger. resolveUserPath(_environment.get(), &_options); _fsStarter.initUserFs(_options.ramFsUserData, _options.userPath); - _logStarter.initSecondary(ufs); // Resolve data path, create data fs. // TODO(captainurist): actually move datapath to config? @@ -99,7 +97,7 @@ void GameStarter::initWithLogger() { } // Finish logger init now that we know the desired log level. - _logStarter.initFinal(_options.logLevel ? *_options.logLevel : _config->debug.LogLevel.value()); + _logStarter.initialize(ufs, _options.logLevel ? *_options.logLevel : _config->debug.LogLevel.value()); // Create platform. if (_options.headless) { diff --git a/src/Application/Startup/LogStarter.cpp b/src/Application/Startup/LogStarter.cpp index 3d0ab885d34f..5a720da1d42f 100644 --- a/src/Application/Startup/LogStarter.cpp +++ b/src/Application/Startup/LogStarter.cpp @@ -9,24 +9,7 @@ #include "Library/Logger/DistLogSink.h" #include "Library/Logger/BufferLogSink.h" -LogStarter::LogStarter() = default; - -LogStarter::~LogStarter() { - // We never got around to finding out the desired log level => flush everything at LOG_TRACE. - if (_stage != STAGE_FINAL && _stage != STAGE_INITIAL) { - _stage = STAGE_SECONDARY; - try { - initFinal(LOG_TRACE); - } catch (...) { - // Nothing we can do here. - } - } -} - -void LogStarter::initPrimary() { - assert(_stage == STAGE_INITIAL); - _stage = STAGE_PRIMARY; - +LogStarter::LogStarter() { _rootLogSink = std::make_unique(); _logger = std::make_unique(LOG_TRACE, _rootLogSink.get()); @@ -35,27 +18,35 @@ void LogStarter::initPrimary() { _rootLogSink->addLogSink(_bufferLogSink.get()); } -void LogStarter::initSecondary(FileSystem *userFs) { - assert(_stage == STAGE_PRIMARY); - _stage = STAGE_SECONDARY; - - try { - _userLogSink = std::make_unique("logs/openenroth.log", userFs); - } catch (const std::exception &e) { - _logger->log(LOG_ERROR, "Could not open log file for writing: {}", e.what()); - _userLogSink.reset(); +LogStarter::~LogStarter() { + // We never got around to finding out the desired log level => flush everything at LOG_TRACE. + if (!_initialized) { + try { + initialize(nullptr, LOG_TRACE); + } catch (...) { + // Nothing we can do here. + } } } -void LogStarter::initFinal(LogLevel logLevel) { - assert(_stage == STAGE_SECONDARY); - _stage = STAGE_FINAL; +void LogStarter::initialize(FileSystem *userFs, LogLevel logLevel) { + assert(!_initialized); + _initialized = true; - // Create proper log sinks. + // Create & install default log sink. _defaultLogSink = LogSink::createDefaultSink(); _rootLogSink->addLogSink(_defaultLogSink.get()); - if (_userLogSink) - _rootLogSink->addLogSink(_userLogSink.get()); + + // Set up filesystem logging. We skip this on LOG_NONE b/c we don't want any FS changes in this case. + if (userFs && logLevel != LOG_NONE) { + try { + _userLogSink = std::make_unique("logs/openenroth.log", userFs); + _rootLogSink->addLogSink(_userLogSink.get()); + } catch (const std::exception &e) { + _logger->log(LOG_ERROR, "Could not open log file for writing: {}", e.what()); + _userLogSink.reset(); + } + } // Then init log level & flush. _rootLogSink->removeLogSink(_bufferLogSink.get()); diff --git a/src/Application/Startup/LogStarter.h b/src/Application/Startup/LogStarter.h index 1338fb2e4bf8..27e91d925a77 100644 --- a/src/Application/Startup/LogStarter.h +++ b/src/Application/Startup/LogStarter.h @@ -17,23 +17,13 @@ class LogStarter { LogStarter(); ~LogStarter(); - void initPrimary(); // Can use global logger after this. - void initSecondary(FileSystem *userFs); // Start writing filesystem log. - void initFinal(LogLevel logLevel); // Set log level & finalize logger init. + void initialize(FileSystem *userFs, LogLevel logLevel); // Set log level & finalize logger init. DistLogSink *rootSink() const; private: - enum class Stage { - STAGE_INITIAL, - STAGE_PRIMARY, - STAGE_SECONDARY, - STAGE_FINAL - }; - using enum Stage; - - private: - Stage _stage = STAGE_INITIAL; + bool _initialized = false; + std::unique_ptr _nullSink; std::unique_ptr _bufferLogSink; std::unique_ptr _defaultLogSink; std::unique_ptr _userLogSink; diff --git a/src/Library/Logger/LogCategory.h b/src/Library/Logger/LogCategory.h index 394935814daf..c694727ce6c7 100644 --- a/src/Library/Logger/LogCategory.h +++ b/src/Library/Logger/LogCategory.h @@ -55,4 +55,8 @@ class LogCategory { // Storing log level here is an implementation detail - it makes it possible to check the log level inside a // logging call in just two memory reads. std::optional _level; + + // Same as above, but we store `detail::LOG_NONE_BARRIER` instead of `LOG_NONE` here so that when we don't read the + // docs and try to log at `LOG_NONE`, the message is always ignored. + std::optional _adjustedLevel; }; diff --git a/src/Library/Logger/LogEnumFunctions.h b/src/Library/Logger/LogEnumFunctions.h index 963a4a4d07a8..7f5d9fa109d0 100644 --- a/src/Library/Logger/LogEnumFunctions.h +++ b/src/Library/Logger/LogEnumFunctions.h @@ -14,6 +14,7 @@ inline spdlog::level::level_enum translateLogLevel(LogLevel level) { case LOG_WARNING: return spdlog::level::warn; case LOG_ERROR: return spdlog::level::err; case LOG_CRITICAL: return spdlog::level::critical; + case LOG_NONE: return spdlog::level::off; default: assert(false); return spdlog::level::trace; diff --git a/src/Library/Logger/LogEnums.cpp b/src/Library/Logger/LogEnums.cpp index 3bebda8c2318..43073355ba6d 100644 --- a/src/Library/Logger/LogEnums.cpp +++ b/src/Library/Logger/LogEnums.cpp @@ -3,6 +3,7 @@ #include "Library/Serialization/EnumSerialization.h" MM_DEFINE_ENUM_SERIALIZATION_FUNCTIONS(LogLevel, CASE_INSENSITIVE, { + {LOG_NONE, "none"}, {LOG_TRACE, "trace"}, {LOG_DEBUG, "debug"}, {LOG_INFO, "info"}, diff --git a/src/Library/Logger/LogEnums.h b/src/Library/Logger/LogEnums.h index 5e098f368e07..b46ac83b66a7 100644 --- a/src/Library/Logger/LogEnums.h +++ b/src/Library/Logger/LogEnums.h @@ -6,12 +6,18 @@ * Log level as used by `Logger`. */ enum class LogLevel { + LOG_NONE, // Special log level essentially meaning "please don't log anything." Trying to log a message with this + // log level will always drop it. LOG_TRACE, LOG_DEBUG, LOG_INFO, LOG_WARNING, LOG_ERROR, - LOG_CRITICAL + LOG_CRITICAL, }; using enum LogLevel; MM_DECLARE_SERIALIZATION_FUNCTIONS(LogLevel) + +namespace detail { +constexpr int LOG_NONE_BARRIER = static_cast(LOG_CRITICAL) + 1; +} // namespace detail diff --git a/src/Library/Logger/Logger.cpp b/src/Library/Logger/Logger.cpp index 5e69f208a2e7..e3d2f8743d86 100644 --- a/src/Library/Logger/Logger.cpp +++ b/src/Library/Logger/Logger.cpp @@ -8,10 +8,15 @@ Logger *logger = nullptr; +static int adjustLevel(LogLevel level) { + return level == LOG_NONE ? detail::LOG_NONE_BARRIER : static_cast(level); +} + Logger::Logger(LogLevel level, LogSink *sink) { assert(sink); _defaultCategory._level = level; + _defaultCategory._adjustedLevel = adjustLevel(level); _sink = sink; assert(logger == nullptr); @@ -39,6 +44,7 @@ void Logger::setLevel(LogLevel level) { return; _defaultCategory._level = level; + _defaultCategory._adjustedLevel = adjustLevel(level); for (const LogCategory *category : LogCategory::instances()) if (category->_source && !category->_level) @@ -54,6 +60,7 @@ void Logger::setLevel(LogCategory &category, std::optional level) { return; category._level = level; + category._adjustedLevel = level.transform(&adjustLevel); if (category._source) { LogLevel effectiveLevel = level ? *level : *_defaultCategory._level; diff --git a/src/Library/Logger/Logger.h b/src/Library/Logger/Logger.h index c4c90823a50e..8335b15c9bc7 100644 --- a/src/Library/Logger/Logger.h +++ b/src/Library/Logger/Logger.h @@ -40,7 +40,7 @@ class Logger { // LogCategory API. [[nodiscard]] bool shouldLog(const LogCategory &category, LogLevel level) const { - return level >= (category._level ? *category._level : *_defaultCategory._level); + return static_cast(level) >= (category._adjustedLevel ? *category._adjustedLevel : *_defaultCategory._adjustedLevel); } template @@ -82,7 +82,7 @@ class Logger { // Default category API. [[nodiscard]] bool shouldLog(LogLevel level) const { - return level >= *_defaultCategory._level; + return static_cast(level) >= *_defaultCategory._adjustedLevel; } template diff --git a/src/Library/Platform/Sdl/SdlEnumTranslation.cpp b/src/Library/Platform/Sdl/SdlEnumTranslation.cpp index 1c16ddd1c22d..cb7861387628 100644 --- a/src/Library/Platform/Sdl/SdlEnumTranslation.cpp +++ b/src/Library/Platform/Sdl/SdlEnumTranslation.cpp @@ -227,7 +227,8 @@ SDL_LogPriority translatePlatformLogLevel(LogLevel logLevel) { case LOG_INFO: return SDL_LOG_PRIORITY_INFO; case LOG_WARNING: return SDL_LOG_PRIORITY_WARN; case LOG_ERROR: return SDL_LOG_PRIORITY_ERROR; - case LOG_CRITICAL: return SDL_LOG_PRIORITY_CRITICAL; + case LOG_CRITICAL: + case LOG_NONE: return SDL_LOG_PRIORITY_CRITICAL; // SDL doesn't have LOG_NONE, so this is the best we can do. } } diff --git a/src/Media/FFmpegLogSource.cpp b/src/Media/FFmpegLogSource.cpp index 7e8635d5f6a9..4e0a07a38fda 100644 --- a/src/Media/FFmpegLogSource.cpp +++ b/src/Media/FFmpegLogSource.cpp @@ -16,7 +16,9 @@ void FFmpegLogSource::setLevel(LogLevel level) { LogLevel FFmpegLogSource::translateFFmpegLogLevel(int level) { // Best effort translation here. - if (level <= AV_LOG_PANIC) { + if (level <= AV_LOG_QUIET) { + return LOG_NONE; + } else if (level <= AV_LOG_PANIC) { return LOG_CRITICAL; } else if (level <= AV_LOG_FATAL) { return LOG_CRITICAL; // AV_LOG_PANIC & AV_LOG_FATAL are both translated into LOG_CRITICAL. @@ -43,6 +45,7 @@ int FFmpegLogSource::translateLoggerLogLevel(LogLevel level) { case LOG_WARNING: return AV_LOG_WARNING; case LOG_ERROR: return AV_LOG_ERROR; case LOG_CRITICAL: return AV_LOG_FATAL; // max(AV_LOG_PANIC, AV_LOG_FATAL) + case LOG_NONE: return AV_LOG_QUIET; default: assert(false); return AV_LOG_TRACE;