Skip to content

Commit

Permalink
Add LOG_NONE
Browse files Browse the repository at this point in the history
  • Loading branch information
captainurist committed Nov 3, 2024
1 parent 1c32315 commit 3a72e8e
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 55 deletions.
2 changes: 1 addition & 1 deletion src/Application/GameConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
4 changes: 1 addition & 3 deletions src/Application/Startup/GameStarter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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?
Expand All @@ -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) {
Expand Down
57 changes: 24 additions & 33 deletions src/Application/Startup/LogStarter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<DistLogSink>();
_logger = std::make_unique<Logger>(LOG_TRACE, _rootLogSink.get());

Expand All @@ -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<RotatingLogSink>("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<RotatingLogSink>("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());
Expand Down
16 changes: 3 additions & 13 deletions src/Application/Startup/LogStarter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<LogSink> _nullSink;
std::unique_ptr<BufferLogSink> _bufferLogSink;
std::unique_ptr<LogSink> _defaultLogSink;
std::unique_ptr<RotatingLogSink> _userLogSink;
Expand Down
4 changes: 4 additions & 0 deletions src/Library/Logger/LogCategory.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<LogLevel> _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<int> _adjustedLevel;
};
1 change: 1 addition & 0 deletions src/Library/Logger/LogEnumFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/Library/Logger/LogEnums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
8 changes: 7 additions & 1 deletion src/Library/Logger/LogEnums.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(LOG_CRITICAL) + 1;
} // namespace detail
7 changes: 7 additions & 0 deletions src/Library/Logger/Logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,15 @@

Logger *logger = nullptr;

static int adjustLevel(LogLevel level) {
return level == LOG_NONE ? detail::LOG_NONE_BARRIER : static_cast<int>(level);
}

Logger::Logger(LogLevel level, LogSink *sink) {
assert(sink);

_defaultCategory._level = level;
_defaultCategory._adjustedLevel = adjustLevel(level);
_sink = sink;

assert(logger == nullptr);
Expand Down Expand Up @@ -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)
Expand All @@ -54,6 +60,7 @@ void Logger::setLevel(LogCategory &category, std::optional<LogLevel> level) {
return;

category._level = level;
category._adjustedLevel = level.transform(&adjustLevel);

if (category._source) {
LogLevel effectiveLevel = level ? *level : *_defaultCategory._level;
Expand Down
4 changes: 2 additions & 2 deletions src/Library/Logger/Logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(level) >= (category._adjustedLevel ? *category._adjustedLevel : *_defaultCategory._adjustedLevel);
}

template<class... Args>
Expand Down Expand Up @@ -82,7 +82,7 @@ class Logger {
// Default category API.

[[nodiscard]] bool shouldLog(LogLevel level) const {
return level >= *_defaultCategory._level;
return static_cast<int>(level) >= *_defaultCategory._adjustedLevel;
}

template<class... Args>
Expand Down
3 changes: 2 additions & 1 deletion src/Library/Platform/Sdl/SdlEnumTranslation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/Media/FFmpegLogSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand Down

0 comments on commit 3a72e8e

Please sign in to comment.