diff --git a/.gitmodules b/.gitmodules index b979488c018f..62774f768dd3 100644 --- a/.gitmodules +++ b/.gitmodules @@ -35,3 +35,6 @@ [submodule "thirdparty/openal_soft"] path = thirdparty/openal_soft url = https://github.com/kcat/openal-soft.git +[submodule "thirdparty/spdlog"] + path = thirdparty/spdlog + url = https://github.com/gabime/spdlog.git diff --git a/src/Application/Game.cpp b/src/Application/Game.cpp index 3ef16b9e80d8..4d837a2cdb61 100644 --- a/src/Application/Game.cpp +++ b/src/Application/Game.cpp @@ -1313,9 +1313,9 @@ void Game::processQueuedMessages() { continue; } else { if (pParty->uFlags & PARTY_FLAGS_1_AIRBORNE) - logger->verbose("Party is airborne"); + logger->trace("Party is airborne"); if (pParty->uFlags & PARTY_FLAGS_1_STANDING_ON_WATER) - logger->verbose("Party on water"); + logger->trace("Party on water"); } if (pParty->bTurnBasedModeOn) { diff --git a/src/Application/Game.h b/src/Application/Game.h index 3c6cbb5f9045..04fa7a513e1c 100644 --- a/src/Application/Game.h +++ b/src/Application/Game.h @@ -8,7 +8,6 @@ #include "GameIocContainer.h" #include "Engine/Engine.h" -#include "Engine/EngineIocContainer.h" #include "Io/KeyboardInputHandler.h" #include "Io/Mouse.h" diff --git a/src/Application/GameConfig.cpp b/src/Application/GameConfig.cpp index fb0fe50d08f2..2a789baea760 100644 --- a/src/Application/GameConfig.cpp +++ b/src/Application/GameConfig.cpp @@ -1,9 +1,5 @@ #include "GameConfig.h" -#include - -#include "Engine/EngineIocContainer.h" - #include "Library/Logger/Logger.h" #include "Library/Serialization/EnumSerialization.h" @@ -21,15 +17,6 @@ MM_DEFINE_ENUM_SERIALIZATION_FUNCTIONS(PlatformWindowMode, CASE_INSENSITIVE, { {WINDOW_MODE_FULLSCREEN_BORDERLESS, "3"} }) -MM_DEFINE_ENUM_SERIALIZATION_FUNCTIONS(PlatformLogLevel, CASE_INSENSITIVE, { - {LOG_VERBOSE, "verbose"}, - {LOG_DEBUG, "debug"}, - {LOG_INFO, "info"}, - {LOG_WARNING, "warning"}, - {LOG_ERROR, "error"}, - {LOG_CRITICAL, "critical"}, -}) - MM_DEFINE_ENUM_SERIALIZATION_FUNCTIONS(RandomEngineType, CASE_INSENSITIVE, { {RANDOM_ENGINE_MERSENNE_TWISTER, "mersenne_twister"}, {RANDOM_ENGINE_SEQUENTIAL, "sequential"} diff --git a/src/Application/GameConfig.h b/src/Application/GameConfig.h index 46d0ffe7ddfc..18dd1483424f 100644 --- a/src/Application/GameConfig.h +++ b/src/Application/GameConfig.h @@ -7,10 +7,9 @@ #include "Io/Key.h" -#include "Platform/PlatformEnums.h" - #include "Library/Config/Config.h" #include "Library/Random/RandomEngineEnums.h" +#include "Library/Logger/LogEnums.h" #ifdef __ANDROID__ #define ConfigRenderer RENDERER_OPENGL_ES @@ -21,9 +20,10 @@ #endif MM_DECLARE_SERIALIZATION_FUNCTIONS(PlatformWindowMode) -MM_DECLARE_SERIALIZATION_FUNCTIONS(PlatformLogLevel) MM_DECLARE_SERIALIZATION_FUNCTIONS(RandomEngineType) +// TODO(captainurist): apply codestyle here. + class GameConfig : public Config { public: GameConfig(); @@ -83,8 +83,8 @@ class GameConfig : public Config { Bool NoMargaret = {this, "no_margareth", false, "Disable Margaret's tour messages on Emerald Island."}; - ConfigEntry LogLevel = {this, "log_level", LOG_ERROR, - "Default log level. One of 'verbose', 'debug', 'info', 'warning', 'error' and 'critical'."}; + ConfigEntry<::LogLevel> LogLevel = {this, "log_level", LOG_ERROR, + "Default log level. One of 'verbose', 'debug', 'info', 'warning', 'error' and 'critical'."}; Int TraceFrameTimeMs = {this, "trace_frame_time_ms", 50, &ValidateFrameTime, "Number of milliseconds per frame when recording game traces."}; diff --git a/src/Application/GamePathResolver.cpp b/src/Application/GamePathResolver.cpp index 5d15b1f48d80..72a1fdadbe37 100644 --- a/src/Application/GamePathResolver.cpp +++ b/src/Application/GamePathResolver.cpp @@ -2,7 +2,6 @@ #include "Application/GamePathResolver.h" -#include "Engine/EngineIocContainer.h" #include "Library/Logger/Logger.h" #include "Platform/Platform.h" diff --git a/src/Application/GameStarter.cpp b/src/Application/GameStarter.cpp index 63e8efefee9f..c2c00fa35e73 100644 --- a/src/Application/GameStarter.cpp +++ b/src/Application/GameStarter.cpp @@ -4,14 +4,14 @@ #include #include -#include "Engine/EngineIocContainer.h" #include "Engine/Engine.h" #include "Engine/EngineGlobals.h" #include "Library/Application/PlatformApplication.h" #include "Library/Logger/Logger.h" +#include "Library/Logger/LogSink.h" +#include "Library/Logger/BufferLogSink.h" -#include "Platform/PlatformLogger.h" #include "Platform/Platform.h" #include "Platform/Null/NullPlatform.h" @@ -21,47 +21,41 @@ GameStarter::GameStarter(GameStarterOptions options): _options(std::move(options)) { // Init logger. - _logger = PlatformLogger::createStandardLogger(); - auto setLogLevel = [logger = _logger.get()](PlatformLogLevel level) { - logger->setLogLevel(APPLICATION_LOG, level); - logger->setLogLevel(PLATFORM_LOG, level); - }; - if (_options.logLevel) - setLogLevel(*_options.logLevel); - _globalLogger = std::make_unique(_logger.get()); - ::logger = _globalLogger.get(); + _bufferSink = std::make_unique(); + _defaultSink = LogSink::createDefaultSink(); + _logger = std::make_unique(LOG_TRACE, _bufferSink.get()); + Engine::LogEngineBuildInfo(); // Create platform & init data paths. if (_options.headless) { _platform = std::make_unique(NullPlatformOptions()); } else { - // TODO(captainurist): this can't use log level from config. Introduce a buffer logger, log into a buffer, - // dump into the real logger when it is constructed. _platform = Platform::createStandardPlatform(_logger.get()); } resolveDefaults(_platform.get(), &_options); // Init config - needs data paths initialized. _config = std::make_shared(); - std::string configLogMessage; if (_options.useConfig) { if (std::filesystem::exists(_options.configPath)) { _config->load(_options.configPath); - configLogMessage = fmt::format("Configuration file '{}' loaded!", _options.configPath); + _logger->info("Configuration file '{}' loaded!", _options.configPath); } else { _config->reset(); - configLogMessage = fmt::format("Could not read configuration file '{}'! Loaded default configuration instead!", _options.configPath); + _logger->info("Could not read configuration file '{}'! Loaded default configuration instead!", _options.configPath); } } - if (!_options.logLevel) - setLogLevel(_config->debug.LogLevel.value()); if (_options.headless) _config->graphics.Renderer.setValue(RENDERER_NULL); // TODO(captainurist): we shouldn't be writing to config here. - // Write the first log messages. - Engine::LogEngineBuildInfo(); - if (!configLogMessage.empty()) - logger->info("{}", configLogMessage); + // Finish logger init now that we know the desired log level. + if (_options.logLevel) { + _logger->setLevel(*_options.logLevel); + } else { + _logger->setLevel(_config->debug.LogLevel.value()); + } + _logger->setSink(_defaultSink.get()); + _bufferSink->flush(_logger.get()); // Validate data paths. ::platform = _platform.get(); // TODO(captainurist): a hack to make validateDataPath work. diff --git a/src/Application/GameStarter.h b/src/Application/GameStarter.h index d82c3d4347db..0d07922e223c 100644 --- a/src/Application/GameStarter.h +++ b/src/Application/GameStarter.h @@ -7,6 +7,8 @@ class Platform; class PlatformLogger; class Logger; +class BufferLogSink; +class LogSink; class PlatformApplication; class GameConfig; class Game; @@ -31,8 +33,9 @@ class GameStarter { private: GameStarterOptions _options; - std::unique_ptr _logger; - std::unique_ptr _globalLogger; + std::unique_ptr _bufferSink; + std::unique_ptr _defaultSink; + std::unique_ptr _logger; std::unique_ptr _platform; std::unique_ptr _application; std::shared_ptr _config; diff --git a/src/Application/GameStarterOptions.h b/src/Application/GameStarterOptions.h index d6558bdc1800..55e807dcb0a6 100644 --- a/src/Application/GameStarterOptions.h +++ b/src/Application/GameStarterOptions.h @@ -5,12 +5,12 @@ #include "Engine/Graphics/RenderEnums.h" -#include "Platform/PlatformEnums.h" +#include "Library/Logger/LogEnums.h" struct GameStarterOptions { bool useConfig = true; // Load external config & save it on exit? std::string configPath; // Path to config, empty means use default. std::string dataPath; // Path to game data, empty means use default. - std::optional logLevel; // Override log level. + std::optional logLevel; // Override log level. bool headless = false; // Run in headless mode. }; diff --git a/src/Bin/OpenEnroth/OpenEnrothOptions.cpp b/src/Bin/OpenEnroth/OpenEnrothOptions.cpp index a2df200a1239..88d42fa3c093 100644 --- a/src/Bin/OpenEnroth/OpenEnrothOptions.cpp +++ b/src/Bin/OpenEnroth/OpenEnrothOptions.cpp @@ -3,7 +3,6 @@ #include #include "Application/GamePathResolver.h" -#include "Application/GameConfig.h" // For PlatformLogLevel serialization. #include "Library/Cli/CliApp.h" @@ -24,10 +23,10 @@ OpenEnrothOptions OpenEnrothOptions::parse(int argc, char **argv) { "Path to OpenEnroth config file, default is 'openenroth.ini' in data folder.")->option_text("PATH"); app->add_option( "--log-level", result.logLevel, - "Log level, one of 'verbose', 'debug', 'info', 'warning', 'error', 'critical'.")->option_text("LOG_LEVEL"); + "Log level, one of 'trace', 'debug', 'info', 'warning', 'error', 'critical'.")->option_text("LOG_LEVEL"); app->add_flag_callback( - "-v,--verbose", [&] { result.logLevel = LOG_VERBOSE; }, - "Set log level to 'verbose'."); + "-v,--verbose", [&] { result.logLevel = LOG_TRACE; }, + "Set log level to 'trace'."); app->set_help_flag("-h,--help", "Print help and exit."); CLI::App *retrace = app->add_subcommand("retrace", "Retrace traces and exit.", result.subcommand, SUBCOMMAND_RETRACE)->fallthrough(); diff --git a/src/Engine/AssetsManager.cpp b/src/Engine/AssetsManager.cpp index 85ad0074e053..043959704bfa 100644 --- a/src/Engine/AssetsManager.cpp +++ b/src/Engine/AssetsManager.cpp @@ -4,7 +4,6 @@ #include "Engine/Graphics/ImageLoader.h" #include "Engine/Graphics/Image.h" -#include "Engine/EngineIocContainer.h" #include "Engine/LodTextureCache.h" #include "Engine/LodSpriteCache.h" @@ -17,7 +16,7 @@ AssetsManager *assets = new AssetsManager(); void AssetsManager::releaseAllTextures() { - logger->verbose("Render - Releasing Textures."); + logger->trace("Render - Releasing Textures."); // clears any textures from gpu for (auto img : images) { img.second->releaseRenderId(); diff --git a/src/Engine/Components/Trace/EngineTraceRecorder.cpp b/src/Engine/Components/Trace/EngineTraceRecorder.cpp index febd14e2e560..8a228385296f 100644 --- a/src/Engine/Components/Trace/EngineTraceRecorder.cpp +++ b/src/Engine/Components/Trace/EngineTraceRecorder.cpp @@ -8,7 +8,6 @@ #include "Application/GameKeyboardController.h" // TODO(captainurist): Engine -> Application dependency #include "Engine/Engine.h" -#include "Engine/EngineIocContainer.h" #include "Engine/Components/Control/EngineController.h" #include "Engine/Components/Deterministic/EngineDeterministicComponent.h" diff --git a/src/Engine/EngineIocContainer.cpp b/src/Engine/EngineIocContainer.cpp index 8555efbc5b57..22293a8fd8d5 100644 --- a/src/Engine/EngineIocContainer.cpp +++ b/src/Engine/EngineIocContainer.cpp @@ -9,14 +9,10 @@ #include "Engine/Objects/Character.h" #include "Engine/Objects/SpriteObject.h" -#include "Library/Logger/Logger.h" - #include "Io/Mouse.h" using Io::Mouse; -Logger *logger = nullptr; - DecalBuilder *EngineIocContainer::ResolveDecalBuilder() { if (!decal_builder) { decal_builder = new DecalBuilder(); diff --git a/src/Engine/EngineIocContainer.h b/src/Engine/EngineIocContainer.h index d4dd1cebbd68..f4d5cfe6191c 100644 --- a/src/Engine/EngineIocContainer.h +++ b/src/Engine/EngineIocContainer.h @@ -4,7 +4,6 @@ struct BloodsplatContainer; struct DecalBuilder; -class Logger; namespace Io { class Mouse; } @@ -32,5 +31,3 @@ class EngineIocContainer { static std::shared_ptr particle_engine; static Vis *vis; }; - -extern Logger *logger; diff --git a/src/Engine/Events/EventMap.cpp b/src/Engine/Events/EventMap.cpp index 4a19105c8cb7..dd0afbeb298e 100644 --- a/src/Engine/Events/EventMap.cpp +++ b/src/Engine/Events/EventMap.cpp @@ -5,7 +5,6 @@ #include "Engine/Party.h" #include "Engine/Engine.h" -#include "Engine/EngineIocContainer.h" #include "Library/Logger/Logger.h" @@ -115,12 +114,12 @@ std::string EventMap::hint(int eventId) const { void EventMap::dump(int eventId) const { const auto *events = valuePtr(_eventsById, eventId); if (events) { - logger->verbose("Event: {}", eventId); + logger->trace("Event: {}", eventId); for (const EventIR &ir : *events) { - logger->verbose("{}", ir.toString()); + logger->trace("{}", ir.toString()); } } else { - logger->verbose("Event {} not found", eventId); + logger->trace("Event {} not found", eventId); } } diff --git a/src/Engine/Events/Processor.cpp b/src/Engine/Events/Processor.cpp index 8742c53f91ab..96c71a1464d1 100644 --- a/src/Engine/Events/Processor.cpp +++ b/src/Engine/Events/Processor.cpp @@ -1,8 +1,6 @@ #include -#include #include "Engine/Engine.h" -#include "Engine/EngineIocContainer.h" #include "Engine/Localization.h" #include "Engine/mm7_data.h" #include "Engine/Graphics/LocationFunctions.h" @@ -158,7 +156,7 @@ void eventProcessor(int eventId, Pid targetObj, bool canShowMessages, int startS EventInterpreter interpreter; bool mapExitTriggered = false; - logger->verbose("Executing regular event starting from step {}", startStep); + logger->trace("Executing regular event starting from step {}", startStep); if (activeLevelDecoration) { engine->_globalEventMap.dump(eventId); interpreter.prepare(engine->_globalEventMap, eventId, targetObj, canShowMessages); @@ -179,7 +177,7 @@ bool npcDialogueEventProcessor(int eventId, int startStep) { EventInterpreter interpreter; - logger->verbose("Executing NPC dialogue event starting from step {}", startStep); + logger->trace("Executing NPC dialogue event starting from step {}", startStep); LevelDecoration *oldDecoration = activeLevelDecoration; activeLevelDecoration = (LevelDecoration *)1; // Required for correct printing of messages engine->_globalEventMap.dump(eventId); diff --git a/src/Engine/Graphics/ClippingFunctions.cpp b/src/Engine/Graphics/ClippingFunctions.cpp index 95c9b572329c..a866b91498d2 100644 --- a/src/Engine/Graphics/ClippingFunctions.cpp +++ b/src/Engine/Graphics/ClippingFunctions.cpp @@ -1,12 +1,9 @@ #include "Engine/Graphics/ClippingFunctions.h" #include "Engine/Engine.h" -#include "Engine/EngineIocContainer.h" #include "Library/Logger/Logger.h" -#include "Camera.h" - //----- (00498377) -------------------------------------------------------- bool ClippingFunctions::ClipVertsToPortal(RenderVertexSoft *pPortalBounding, // test skipping this unsigned int uNumfrust, diff --git a/src/Engine/Graphics/ImageLoader.cpp b/src/Engine/Graphics/ImageLoader.cpp index 2175d8c45abd..a935676b307d 100644 --- a/src/Engine/Graphics/ImageLoader.cpp +++ b/src/Engine/Graphics/ImageLoader.cpp @@ -4,7 +4,6 @@ #include #include -#include "Engine/EngineIocContainer.h" #include "Engine/Graphics/IRender.h" #include "Engine/Graphics/Sprites.h" #include "Engine/Graphics/Texture_MM7.h" diff --git a/src/Engine/Graphics/Indoor.cpp b/src/Engine/Graphics/Indoor.cpp index 6da7db841da6..d817a279ef9f 100644 --- a/src/Engine/Graphics/Indoor.cpp +++ b/src/Engine/Graphics/Indoor.cpp @@ -1114,7 +1114,7 @@ int BLV_GetFloorLevel(const Vec3i &pos, int uSectorID, int *pFaceID) { // no face found - probably wrong sector supplied if (!FacesFound) { - logger->verbose("Floorlvl fail: {} {} {}", pos.x, pos.y, pos.z); + logger->trace("Floorlvl fail: {} {} {}", pos.x, pos.y, pos.z); if (pFaceID) *pFaceID = -1; diff --git a/src/Engine/Graphics/LightsStack.cpp b/src/Engine/Graphics/LightsStack.cpp index 9b6e509b030b..57fa570784db 100644 --- a/src/Engine/Graphics/LightsStack.cpp +++ b/src/Engine/Graphics/LightsStack.cpp @@ -1,7 +1,6 @@ #include "Library/Logger/Logger.h" #include "Engine/Graphics/LightsStack.h" -#include "Engine/EngineIocContainer.h" LightsStack_StationaryLight_::LightsStack_StationaryLight_() { this->uNumLightsActive = 0; diff --git a/src/Engine/Graphics/Nuklear.cpp b/src/Engine/Graphics/Nuklear.cpp index 3575c49a8df0..702dc0ea3fda 100644 --- a/src/Engine/Graphics/Nuklear.cpp +++ b/src/Engine/Graphics/Nuklear.cpp @@ -9,7 +9,6 @@ #include "Engine/AssetsManager.h" #include "Engine/Engine.h" #include "Engine/EngineGlobals.h" -#include "Engine/EngineIocContainer.h" #include "Engine/GameResourceManager.h" #include "Engine/Graphics/Nuklear.h" @@ -797,7 +796,7 @@ void Nuklear::Release(WindowType winType, bool is_reload) { if ((*it)->asset) { render->NuklearImageFree((*it)->asset); (*it)->asset->Release(); - logger->verbose("Nuklear: [{}] asset {} unloaded", wins[winType].tmpl, i); + logger->trace("Nuklear: [{}] asset {} unloaded", wins[winType].tmpl, i); delete *it; } } @@ -822,7 +821,7 @@ void Nuklear::Release(WindowType winType, bool is_reload) { if (!is_reload && (wins[winType].state == WINDOW_INITIALIZED || wins[winType].state == WINDOW_TEMPLATE_ERROR)) wins[winType].state = WINDOW_NOT_LOADED; - logger->verbose("Nuklear: [{}] template unloaded", wins[winType].tmpl); + logger->trace("Nuklear: [{}] template unloaded", wins[winType].tmpl); } else { logger->warning("Nuklear: [{}] template is not loaded", wins[winType].tmpl); } diff --git a/src/Engine/Graphics/OpenGL/GLShaderLoader.cpp b/src/Engine/Graphics/OpenGL/GLShaderLoader.cpp index fb3b39b99d3a..c3d932a66201 100644 --- a/src/Engine/Graphics/OpenGL/GLShaderLoader.cpp +++ b/src/Engine/Graphics/OpenGL/GLShaderLoader.cpp @@ -2,8 +2,6 @@ #include // NOLINT: this is not a C system include. -#include "Engine/EngineIocContainer.h" - #include "Library/Logger/Logger.h" #include "Library/Serialization/EnumSerialization.h" diff --git a/src/Engine/Graphics/OpenGL/RenderOpenGL.cpp b/src/Engine/Graphics/OpenGL/RenderOpenGL.cpp index 8dca6942bdee..364a9cce4002 100644 --- a/src/Engine/Graphics/OpenGL/RenderOpenGL.cpp +++ b/src/Engine/Graphics/OpenGL/RenderOpenGL.cpp @@ -271,7 +271,7 @@ int linevertscnt = 0; void RenderOpenGL::BeginLines2D() { if (linevertscnt) - logger->verbose("BeginLines with points still stored in buffer"); + logger->trace("BeginLines with points still stored in buffer"); DrawTwodVerts(); @@ -637,7 +637,7 @@ void RenderOpenGL::DrawTextureOffset(int pX, int pY, int move_X, int move_Y, void RenderOpenGL::DrawImage(GraphicsImage *img, const Recti &rect, uint paletteid, Color uColor32) { if (!img) { - logger->verbose("Null img passed to DrawImage"); + logger->trace("Null img passed to DrawImage"); return; } @@ -1243,7 +1243,7 @@ void RenderOpenGL::DrawFromSpriteSheet(Recti *pSrcRect, Pointi *pTargetPoint, in GraphicsImage *texture = pArcomageGame->pSprites; if (!texture) { - logger->verbose("Missing Arcomage Sprite Sheet"); + logger->trace("Missing Arcomage Sprite Sheet"); return; } @@ -2439,7 +2439,7 @@ void RenderOpenGL::DoRenderBillboards_D3D() { _set_ortho_modelview(); if (billbstorecnt) - logger->verbose("Billboard shader store isnt empty!"); + logger->trace("Billboard shader store isnt empty!"); // track loaded tex float gltexid{ 0 }; @@ -2752,7 +2752,7 @@ void RenderOpenGL::BeginScene2D() { // TODO(pskelton): use alpha from mask too void RenderOpenGL::DrawTextureNew(float u, float v, GraphicsImage *tex, Color colourmask) { if (!tex) { - logger->verbose("Null texture passed to DrawTextureNew"); + logger->trace("Null texture passed to DrawTextureNew"); return; } @@ -2859,7 +2859,7 @@ void RenderOpenGL::DrawTextureNew(float u, float v, GraphicsImage *tex, Color co // TODO(pskelton): add optional colour32 void RenderOpenGL::DrawTextureCustomHeight(float u, float v, class GraphicsImage *img, int custom_height) { if (!img) { - logger->verbose("Null texture passed to DrawTextureCustomHeight"); + logger->trace("Null texture passed to DrawTextureCustomHeight"); return; } diff --git a/src/Engine/Graphics/PaletteManager.cpp b/src/Engine/Graphics/PaletteManager.cpp index aff52a80dfdb..dec98682adef 100644 --- a/src/Engine/Graphics/PaletteManager.cpp +++ b/src/Engine/Graphics/PaletteManager.cpp @@ -4,7 +4,6 @@ #include #include "Engine/Graphics/Texture_MM7.h" -#include "Engine/EngineIocContainer.h" #include "Engine/LodTextureCache.h" #include "Library/Color/Color.h" diff --git a/src/Engine/Graphics/RenderBase.cpp b/src/Engine/Graphics/RenderBase.cpp index 18685d674eef..6a7ed40022e8 100644 --- a/src/Engine/Graphics/RenderBase.cpp +++ b/src/Engine/Graphics/RenderBase.cpp @@ -143,7 +143,7 @@ void RenderBase::DrawSpriteObjects() { (object->uType < SPRITE_TRAP_FIRE || object->uType > SPRITE_TRAP_BODY))) { // Not a trap. SpriteFrame *frame = object->getSpriteFrame(); if (frame->icon_name == "null" || frame->texture_name == "null") { - logger->verbose("Trying to draw sprite with null frame"); + logger->trace("Trying to draw sprite with null frame"); continue; } @@ -154,7 +154,7 @@ void RenderBase::DrawSpriteObjects() { pBillboardRenderList[::uNumBillboardsToDraw].hwsprite = frame->hw_sprites[octant]; // error catching if (frame->hw_sprites[octant]->texture->height() == 0 || frame->hw_sprites[octant]->texture->width() == 0) { - logger->verbose("Trying to draw sprite with empty octant texture"); + logger->trace("Trying to draw sprite with empty octant texture"); continue; } @@ -409,7 +409,7 @@ void RenderBase::TransformBillboardsAndSetPalettesODM() { TransformBillboard(&billboard, p); } else { - logger->verbose("Billboard with no sprite!"); + logger->trace("Billboard with no sprite!"); } } } diff --git a/src/Engine/Graphics/Sprites.cpp b/src/Engine/Graphics/Sprites.cpp index 32ae01ea37cf..ea39e2854e68 100644 --- a/src/Engine/Graphics/Sprites.cpp +++ b/src/Engine/Graphics/Sprites.cpp @@ -5,7 +5,6 @@ #include #include "Engine/Engine.h" -#include "Engine/EngineIocContainer.h" #include "Engine/OurMath.h" #include "Engine/Graphics/DecorationList.h" #include "Engine/Graphics/PaletteManager.h" diff --git a/src/Engine/Graphics/Vis.cpp b/src/Engine/Graphics/Vis.cpp index 61e34e20e126..662a6c95851b 100644 --- a/src/Engine/Graphics/Vis.cpp +++ b/src/Engine/Graphics/Vis.cpp @@ -6,7 +6,6 @@ #include #include "Engine/Engine.h" -#include "Engine/EngineIocContainer.h" #include "Engine/OurMath.h" #include "Engine/Graphics/Level/Decoration.h" diff --git a/src/Engine/Objects/Chest.cpp b/src/Engine/Objects/Chest.cpp index 4156aefc0953..62e537fdf1a9 100644 --- a/src/Engine/Objects/Chest.cpp +++ b/src/Engine/Objects/Chest.cpp @@ -404,7 +404,7 @@ void Chest::PlaceItems(int uChestID) { // only sued for setup vChests[uChestID].igChestItems[items_counter].SetIdentified(); } } else { - logger->verbose("Cannot place item with id {} in the chest!", std::to_underlying(chest_item_id)); + logger->trace("Cannot place item with id {} in the chest!", std::to_underlying(chest_item_id)); } } } diff --git a/src/Engine/Objects/ItemFunctions.h b/src/Engine/Objects/ItemFunctions.h new file mode 100644 index 000000000000..3f59c932d39b --- /dev/null +++ b/src/Engine/Objects/ItemFunctions.h @@ -0,0 +1,2 @@ +#pragma once + diff --git a/src/Engine/Objects/Monsters.cpp b/src/Engine/Objects/Monsters.cpp index a26cfdb8eace..0239b553f28e 100644 --- a/src/Engine/Objects/Monsters.cpp +++ b/src/Engine/Objects/Monsters.cpp @@ -4,7 +4,6 @@ #include #include "Engine/ErrorHandling.h" -#include "Engine/EngineIocContainer.h" #include "../Tables/FrameTableInc.h" diff --git a/src/Engine/Tables/ItemTable.cpp b/src/Engine/Tables/ItemTable.cpp index e3cc75a4e3c7..f9ee7923f1ba 100644 --- a/src/Engine/Tables/ItemTable.cpp +++ b/src/Engine/Tables/ItemTable.cpp @@ -8,7 +8,6 @@ #include "Engine/Spells/Spells.h" #include "Engine/Engine.h" #include "Engine/Party.h" -#include "Engine/EngineIocContainer.h" #include "Engine/GameResourceManager.h" #include "GUI/UI/UIHouses.h" diff --git a/src/Engine/Tables/TileTable.cpp b/src/Engine/Tables/TileTable.cpp index 259b10a201d6..cd3251662608 100644 --- a/src/Engine/Tables/TileTable.cpp +++ b/src/Engine/Tables/TileTable.cpp @@ -3,7 +3,6 @@ #include "Engine/ErrorHandling.h" #include "Engine/AssetsManager.h" -#include "Engine/EngineIocContainer.h" #include "Library/Random/Random.h" #include "Library/Logger/Logger.h" diff --git a/src/Engine/mm7text_ru.cpp b/src/Engine/mm7text_ru.cpp index 0179d4f1ffc9..559ea583d0c1 100644 --- a/src/Engine/mm7text_ru.cpp +++ b/src/Engine/mm7text_ru.cpp @@ -1,10 +1,8 @@ #include #include #include -#include #include "Engine/ErrorHandling.h" -#include "Engine/EngineIocContainer.h" #include "Library/Logger/Logger.h" diff --git a/src/GUI/GUIWindow.cpp b/src/GUI/GUIWindow.cpp index c7121b4e4230..644a468e0e7d 100644 --- a/src/GUI/GUIWindow.cpp +++ b/src/GUI/GUIWindow.cpp @@ -181,7 +181,7 @@ void GUIWindow::Release() { if (this->eWindowType == WINDOW_GameUI) nuklear->Release(WINDOW_GameUI); - logger->verbose("Release window: {}", toString(eWindowType)); + logger->trace("Release window: {}", toString(eWindowType)); } void GUIWindow::DeleteButtons() { @@ -409,7 +409,7 @@ GUIWindow::GUIWindow() : eWindowType(WINDOW_null) { GUIWindow::GUIWindow(WindowType windowType, Pointi position, Sizei dimensions, WindowData wData, const std::string &hint): eWindowType(windowType) { this->mouse = EngineIocContainer::ResolveMouse(); - logger->verbose("New window: {}", toString(windowType)); + logger->trace("New window: {}", toString(windowType)); lWindowList.push_front(this); this->uFrameWidth = dimensions.w; this->uFrameHeight = dimensions.h; diff --git a/src/Library/Application/PlatformApplication.cpp b/src/Library/Application/PlatformApplication.cpp index 503563e5fecb..ee728c9f237c 100644 --- a/src/Library/Application/PlatformApplication.cpp +++ b/src/Library/Application/PlatformApplication.cpp @@ -8,7 +8,6 @@ #include "Platform/Proxy/ProxyWindow.h" #include "Platform/Proxy/ProxyOpenGLContext.h" #include "Platform/Filters/FilteringEventHandler.h" -#include "Platform/PlatformLogger.h" #include "Utility/MapAccess.h" diff --git a/src/Library/Logger/BufferLogSink.h b/src/Library/Logger/BufferLogSink.h new file mode 100644 index 000000000000..dd12721fef96 --- /dev/null +++ b/src/Library/Logger/BufferLogSink.h @@ -0,0 +1,30 @@ +#pragma once + +#include +#include + +#include "LogSink.h" +#include "Logger.h" + +class BufferLogSink : public LogSink { + public: + virtual void write(const LogCategory &category, LogLevel level, std::string_view message) override { + _buffer.push_back({&category, level, std::string(message)}); + } + + void flush(Logger *target) { + for (const LogMessage &message : _buffer) + target->log(*message.category, message.level, "{}", message.message); + _buffer.clear(); + } + + private: + struct LogMessage { + const LogCategory *category = nullptr; + LogLevel level = LOG_TRACE; + std::string message; + }; + + private: + std::vector _buffer; +}; diff --git a/src/Library/Logger/CMakeLists.txt b/src/Library/Logger/CMakeLists.txt index 3af01b7e7e68..151f0e5c57db 100644 --- a/src/Library/Logger/CMakeLists.txt +++ b/src/Library/Logger/CMakeLists.txt @@ -1,11 +1,19 @@ cmake_minimum_required(VERSION 3.24 FATAL_ERROR) set(LIBRARY_LOGGER_SOURCES - Logger.cpp) + LogCategory.cpp + LogEnums.cpp + Logger.cpp + LogSink.cpp) set(LIBRARY_LOGGER_HEADERS - Logger.h) + BufferLogSink.h + LogCategory.h + LogEnums.h + Logger.h + LogSink.h + LogSource.h) add_library(library_logger STATIC ${LIBRARY_LOGGER_SOURCES} ${LIBRARY_LOGGER_HEADERS}) -target_link_libraries(library_logger PUBLIC platform utility) +target_link_libraries(library_logger PUBLIC library_serialization utility PRIVATE spdlog::spdlog) target_check_style(library_logger) diff --git a/src/Library/Logger/LogCategory.cpp b/src/Library/Logger/LogCategory.cpp new file mode 100644 index 000000000000..5b713d28f7b1 --- /dev/null +++ b/src/Library/Logger/LogCategory.cpp @@ -0,0 +1,28 @@ +#include "LogCategory.h" + +#include +#include + +static std::unordered_map &logCategoriesStorage() { + static std::unordered_map result; // Wrapping in a function static to avoid static init order fiasco. + return result; +} + +LogCategory::LogCategory(std::string_view name, LogSource *source): _name(name), _source(source) { + auto &storage = logCategoriesStorage(); + assert(!storage.contains(_name)); + storage.emplace(_name, this); +} + +LogCategory::~LogCategory() { + auto &storage = logCategoriesStorage(); + assert(storage.contains(_name)); + storage.erase(_name); +} + +std::vector LogCategory::instances() { + std::vector result; + for (const auto &[_, category] : logCategoriesStorage()) + result.push_back(category); + return result; +} diff --git a/src/Library/Logger/LogCategory.h b/src/Library/Logger/LogCategory.h new file mode 100644 index 000000000000..5036fe5d37fa --- /dev/null +++ b/src/Library/Logger/LogCategory.h @@ -0,0 +1,58 @@ +#pragma once + +#include +#include +#include + +#include "LogEnums.h" + +class LogSource; + +/** + * Log category, usually used to differentiate logging calls from different subsystems. + * + * Intended usage is to just create a `static` instance in a `.cpp` file, and use it when logging. + * + * Log category can be associated with a `LogSource`, which will then be used when changing per-category log levels. + */ +class LogCategory { + public: + /** + * Creates and registers a new log category instance. + * + * @param name Name of the log category. `LogCategory` doesn't copy the provided string, + * so the user is expected to pass a string constant. Different `LogCategory` + * instances are expected to have different names, and the constructor will + * assert if this is not satisfied. + * @param source Log source, if any. `LogCategory` doesn't take ownership of the provided + * `LogSource`, and thus lifetime management is up to the user. The best practice + * is to create both the `LogSource` and `LogCategory` as global variables. + */ + explicit LogCategory(std::string_view name, LogSource *source = nullptr); + ~LogCategory(); + + // LogCategory is non-movable & non-copyable. + LogCategory(const LogCategory &) = delete; + LogCategory(LogCategory &&) = default; + + std::string_view name() const { + return _name; + } + + LogSource *source() const { + return _source; + } + + static std::vector instances(); + + private: + friend class Logger; + + private: + std::string_view _name; + LogSource *_source = nullptr; + + // 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; +}; diff --git a/src/Library/Logger/LogEnums.cpp b/src/Library/Logger/LogEnums.cpp new file mode 100644 index 000000000000..3bebda8c2318 --- /dev/null +++ b/src/Library/Logger/LogEnums.cpp @@ -0,0 +1,15 @@ +#include "LogEnums.h" + +#include "Library/Serialization/EnumSerialization.h" + +MM_DEFINE_ENUM_SERIALIZATION_FUNCTIONS(LogLevel, CASE_INSENSITIVE, { + {LOG_TRACE, "trace"}, + {LOG_DEBUG, "debug"}, + {LOG_INFO, "info"}, + {LOG_WARNING, "warning"}, + {LOG_ERROR, "error"}, + {LOG_CRITICAL, "critical"}, + + // Compatibility: + {LOG_TRACE, "verbose"}, +}) diff --git a/src/Library/Logger/LogEnums.h b/src/Library/Logger/LogEnums.h new file mode 100644 index 000000000000..5e098f368e07 --- /dev/null +++ b/src/Library/Logger/LogEnums.h @@ -0,0 +1,17 @@ +#pragma once + +#include "Library/Serialization/SerializationFwd.h" + +/** + * Log level as used by `Logger`. + */ +enum class LogLevel { + LOG_TRACE, + LOG_DEBUG, + LOG_INFO, + LOG_WARNING, + LOG_ERROR, + LOG_CRITICAL +}; +using enum LogLevel; +MM_DECLARE_SERIALIZATION_FUNCTIONS(LogLevel) diff --git a/src/Library/Logger/LogSink.cpp b/src/Library/Logger/LogSink.cpp new file mode 100644 index 000000000000..1fe35439bf73 --- /dev/null +++ b/src/Library/Logger/LogSink.cpp @@ -0,0 +1,97 @@ +#include "LogSink.h" + +#include +#include +#include +#include +#include +#include + +#include // NOLINT +#include // NOLINT +#include // NOLINT +#include // NOLINT + +static spdlog::level::level_enum translateLogLevel(LogLevel level) { + switch (level) { + case LOG_TRACE: return spdlog::level::trace; + case LOG_DEBUG: return spdlog::level::debug; + case LOG_INFO: return spdlog::level::info; + case LOG_WARNING: return spdlog::level::warn; + case LOG_ERROR: return spdlog::level::err; + case LOG_CRITICAL: return spdlog::level::critical; + default: + assert(false); + return spdlog::level::trace; + } +} + +template +class SpdlogSink : public LogSink { + public: + template + explicit SpdlogSink(Args &&... args): _base(std::forward(args)...) {} + + virtual void write(const LogCategory &category, LogLevel level, std::string_view message) override { + // Note that we don't fill source location. + _base.log(spdlog::details::log_msg(category.name(), translateLogLevel(level), message)); + } + + BaseSink &base() { + return _base; + } + + private: + BaseSink _base; +}; + +#ifdef __ANDROID__ +class AndroidSinkMt : public LogSink { + public: + virtual void write(const LogCategory &category, LogLevel level, std::string_view message) override { + auto guard = std::lock_guard(_mutex); + + auto pos = _sinkByCategory.find(category.name()); + if (pos == _sinkByCategory.end()) { + pos = _sinkByCategory.emplace(std::piecewise_construct, + std::forward_as_tuple(category.name()), + std::forward_as_tuple(std::string(category.name()))).first; + } + + pos->second.log(spdlog::details::log_msg(category.name(), translateLogLevel(level), message)); + } + + private: + std::mutex _mutex; + std::unordered_map _sinkByCategory; +}; +#endif + +std::unique_ptr LogSink::createDefaultSink() { +#if defined(__ANDROID__) + return std::make_unique(); +#elif defined(_WINDOWS) + std::vector> sinks = { + std::make_shared(/*check_debugger_present=*/true), + std::make_shared() + }; + return std::make_unique>(std::move(sinks)); +#else + return std::make_unique>(); +#endif +} + + + + + + + + + + + + + + + diff --git a/src/Library/Logger/LogSink.h b/src/Library/Logger/LogSink.h new file mode 100644 index 000000000000..4d95ff6d63e9 --- /dev/null +++ b/src/Library/Logger/LogSink.h @@ -0,0 +1,17 @@ +#pragma once + +#include +#include + +#include "LogCategory.h" +#include "LogEnums.h" + +class LogSink { + public: + virtual void write(const LogCategory &category, LogLevel level, std::string_view message) = 0; + + /** + * @return Default sink for the current platform. Returned sink is thread-safe. + */ + static std::unique_ptr createDefaultSink(); +}; diff --git a/src/Library/Logger/LogSource.h b/src/Library/Logger/LogSource.h new file mode 100644 index 000000000000..e942f20300ba --- /dev/null +++ b/src/Library/Logger/LogSource.h @@ -0,0 +1,16 @@ +#pragma once + +#include "LogEnums.h" + +/** + * When integrating an external log source into the logging framework, you would usually need: + * 1. To forward external log messages into `Logger`. + * 2. To control the log level of the external log source. + * + * This class is an abstraction for #2. + */ +class LogSource { + public: + virtual LogLevel level() const = 0; + virtual void setLevel(LogLevel level) = 0; +}; diff --git a/src/Library/Logger/Logger.cpp b/src/Library/Logger/Logger.cpp index c392fd39905d..79d460c39388 100644 --- a/src/Library/Logger/Logger.cpp +++ b/src/Library/Logger/Logger.cpp @@ -2,10 +2,66 @@ #include -Logger::Logger(PlatformLogger *baseLogger): _baseLogger(baseLogger) { - assert(baseLogger); +#include "LogSink.h" +#include "LogSource.h" + +Logger *logger = nullptr; + +Logger::Logger(LogLevel level, LogSink *sink): _defaultCategory({}) { + assert(sink); + + _defaultCategory._level = level; + _sink = sink; + + assert(logger == nullptr); + logger = this; +} + +Logger::~Logger() { + assert(logger == this); + logger = nullptr; +} + +void Logger::logV(const LogCategory &category, LogLevel level, fmt::string_view fmt, fmt::format_args args) { + _sink->write(category, level, fmt::vformat(fmt, args)); +} + +LogLevel Logger::level() const { + return *_defaultCategory._level; +} + +void Logger::setLevel(LogLevel level) { + if (*_defaultCategory._level == level) + return; + + _defaultCategory._level = level; + + for (const LogCategory *category : LogCategory::instances()) + if (category->_source && !category->_level) + category->_source->setLevel(level); +} + +std::optional Logger::level(const LogCategory &category) const { + return category._level; +} + +void Logger::setLevel(LogCategory &category, std::optional level) { + if (category._level == level) + return; + + category._level = level; + + if (category._source) { + LogLevel effectiveLevel = level ? *level : *_defaultCategory._level; + category._source->setLevel(effectiveLevel); + } +} + +LogSink *Logger::sink() const { + return _sink; } -void Logger::logV(PlatformLogLevel logLevel, fmt::string_view fmt, fmt::format_args args) { - _baseLogger->log(APPLICATION_LOG, logLevel, fmt::vformat(fmt, args).c_str()); +void Logger::setSink(LogSink *sink) { + assert(sink); + _sink = sink; } diff --git a/src/Library/Logger/Logger.h b/src/Library/Logger/Logger.h index 2ae3077f5926..6e32554381a3 100644 --- a/src/Library/Logger/Logger.h +++ b/src/Library/Logger/Logger.h @@ -3,56 +3,142 @@ #include #include -#include "Platform/PlatformLogger.h" #include "Utility/Format.h" +#include "LogCategory.h" +#include "LogEnums.h" + +class LogSink; + +/** + * Main logging class. + * + * `Logger` is a singleton, but the user is supposed to create a `Logger` instance himself before using it. This would + * usually be done in the first few lines of `main`. + * + * Some notes on design decisions: + * 1. `Logger` supports hooking into other logging frameworks, translating the log levels appropriately. Thus, it's + * possible to create separate `LogCategory` instances for SDL and FFmpeg logs, and manage log levels through the + * `Logger` interface. Setting global log level to `LOG_CRITICAL` will then prevent both SDL and FFmpeg from + * emitting any non-critical log messages - the important point being that they will be filtered out at SDL/FFmpeg + * level, and not in the `Logger` code. + * 2. Point (1) above led to settling on making `Logger` a singleton. This is what pretty much all other logging + * libraries do, and thus supporting both multiple `Logger` instances AND being able to hook into external logging + * frameworks made very little sense. + * 3. This also made it possible to implement log categories efficiently by storing per-category log levels inside the + * `LogCategory` objects. + * 4. Different logging targets are implemented with the `LogSink` interface. `LogSink` also makes it possible to + * implement complex logging logic, i.e. writing all logs starting with `LOG_DEBUG` into a file, but printing only + * errors to the console. It's up to the user to properly implement the log level handling in this case. + */ class Logger { public: - explicit Logger(PlatformLogger *baseLogger); + explicit Logger(LogLevel level, LogSink *sink); + ~Logger(); + + // LogCategory API. + + bool shouldLog(const LogCategory &category, LogLevel level) const { + return level >= (category._level ? *category._level : *_defaultCategory._level); + } + + template + void log(const LogCategory &category, LogLevel level, fmt::format_string fmt, Args&&... args) { + if (shouldLog(category, level)) + logV(category, level, fmt, fmt::make_format_args(std::forward(args)...)); + } + + template + void trace(const LogCategory &category, fmt::format_string fmt, Args&&... args) { + log(category, LOG_TRACE, fmt, std::forward(args)...); + } + + template + void debug(const LogCategory &category, fmt::format_string fmt, Args&&... args) { + log(category, LOG_DEBUG, fmt, std::forward(args)...); + } + + template + void info(const LogCategory &category, fmt::format_string fmt, Args&&... args) { + log(category, LOG_INFO, fmt, std::forward(args)...); + } - bool shouldLog(PlatformLogLevel logLevel) const { - return _baseLogger == nullptr || _baseLogger->logLevel(APPLICATION_LOG) <= logLevel; + template + void warning(const LogCategory &category, fmt::format_string fmt, Args&&... args) { + log(category, LOG_WARNING, fmt, std::forward(args)...); } template - void log(PlatformLogLevel logLevel, fmt::format_string fmt, Args&&... args) { - if (shouldLog(logLevel)) - logV(logLevel, fmt, fmt::make_format_args(std::forward(args)...)); + void error(const LogCategory &category, fmt::format_string fmt, Args&&... args) { + log(category, LOG_ERROR, fmt, std::forward(args)...); } template - void verbose(fmt::format_string fmt, Args&&... args) { - log(LOG_VERBOSE, fmt, std::forward(args)...); + void critical(const LogCategory &category, fmt::format_string fmt, Args&&... args) { + log(category, LOG_CRITICAL, fmt, std::forward(args)...); + } + + // Default category API. + + bool shouldLog(LogLevel level) const { + return level >= *_defaultCategory._level; + } + + template + void log(LogLevel level, fmt::format_string fmt, Args&&... args) { + log(_defaultCategory, level, fmt, std::forward(args)...); + } + + template + void trace(fmt::format_string fmt, Args&&... args) { + trace(_defaultCategory, fmt, std::forward(args)...); } template void debug(fmt::format_string fmt, Args&&... args) { - log(LOG_DEBUG, fmt, std::forward(args)...); + debug(_defaultCategory, fmt, std::forward(args)...); } template void info(fmt::format_string fmt, Args&&... args) { - log(LOG_INFO, fmt, std::forward(args)...); + info(_defaultCategory, fmt, std::forward(args)...); } template void warning(fmt::format_string fmt, Args&&... args) { - log(LOG_WARNING, fmt, std::forward(args)...); + warning(_defaultCategory, fmt, std::forward(args)...); } template void error(fmt::format_string fmt, Args&&... args) { - log(LOG_ERROR, fmt, std::forward(args)...); + error(_defaultCategory, fmt, std::forward(args)...); } template void critical(fmt::format_string fmt, Args&&... args) { - log(LOG_CRITICAL, fmt, std::forward(args)...); + critical(_defaultCategory, fmt, std::forward(args)...); } + // Log level handling. NOT thread-safe. + + LogLevel level() const; + void setLevel(LogLevel level); + + std::optional level(const LogCategory &category) const; + void setLevel(LogCategory &category, std::optional level); + + // Sink handling. NOT thread-safe. + + LogSink *sink() const; + void setSink(LogSink *sink); + private: - void logV(PlatformLogLevel logLevel, fmt::string_view fmt, fmt::format_args args); + void logV(const LogCategory &category, LogLevel level, fmt::string_view fmt, fmt::format_args args); private: - PlatformLogger *_baseLogger = nullptr; + LogCategory _defaultCategory; + LogSink *_sink; }; + +extern Logger *logger; // Singleton logger instance. + diff --git a/src/Media/Audio/AudioPlayer.cpp b/src/Media/Audio/AudioPlayer.cpp index a5a763d40751..bb6fd14a5ef0 100644 --- a/src/Media/Audio/AudioPlayer.cpp +++ b/src/Media/Audio/AudioPlayer.cpp @@ -363,9 +363,9 @@ void AudioPlayer::playSound(SoundID eSoundID, SoundPlaybackMode mode, Pid pid) { } } else { if (si.sName.empty()) { - logger->verbose("AudioPlayer: playing sound {}", eSoundID); + logger->trace("AudioPlayer: playing sound {}", eSoundID); } else { - logger->verbose("AudioPlayer: playing sound {} with name '{}'", eSoundID, si.sName); + logger->trace("AudioPlayer: playing sound {} with name '{}'", eSoundID, si.sName); } } } diff --git a/src/Media/Audio/OpenALSoundProvider.cpp b/src/Media/Audio/OpenALSoundProvider.cpp index e923865cf83e..67cab3524489 100644 --- a/src/Media/Audio/OpenALSoundProvider.cpp +++ b/src/Media/Audio/OpenALSoundProvider.cpp @@ -20,7 +20,6 @@ #include #include "Engine/ErrorHandling.h" -#include "Engine/EngineIocContainer.h" #include "Library/Logger/Logger.h" #include "Media/MediaPlayer.h" diff --git a/src/Media/CMakeLists.txt b/src/Media/CMakeLists.txt index 4ed05eb0b843..0f59c6b8379f 100644 --- a/src/Media/CMakeLists.txt +++ b/src/Media/CMakeLists.txt @@ -2,13 +2,15 @@ cmake_minimum_required(VERSION 3.24 FATAL_ERROR) set(MEDIA_SOURCES Media.cpp - MediaLogger.cpp - MediaPlayer.cpp) + FFmpegLogProxy.cpp + MediaPlayer.cpp + FFmpegLogSource.cpp) set(MEDIA_HEADERS Media.h - MediaLogger.h - MediaPlayer.h) + FFmpegLogProxy.h + MediaPlayer.h + FFmpegLogSource.h) add_library(media STATIC ${MEDIA_SOURCES} ${MEDIA_HEADERS}) target_link_libraries(media PUBLIC media_audio library_logger utility application) diff --git a/src/Media/FFmpegLogProxy.cpp b/src/Media/FFmpegLogProxy.cpp new file mode 100644 index 000000000000..806ca744c8ce --- /dev/null +++ b/src/Media/FFmpegLogProxy.cpp @@ -0,0 +1,62 @@ +#include "FFmpegLogProxy.h" + +extern "C" { +#include +} + +#include +#include + +#include "FFmpegLogSource.h" + +static constinit std::mutex globalFFmpegLogMutex; // Access to global logger ptr must be serialized, so we need a global mutex. +static FFmpegLogProxy *globalFFmpegLogger = nullptr; + +static void ffmpegLogCallback(void *ptr, int level, const char *format, va_list args) { + auto guard = std::lock_guard(globalFFmpegLogMutex); + + assert(globalFFmpegLogger); + globalFFmpegLogger->log(ptr, level, format, args); +} + +FFmpegLogProxy::FFmpegLogProxy(Logger *logger): _logger(logger), _category("ffmpeg", &_source) { + assert(logger); + assert(globalFFmpegLogger == nullptr); + globalFFmpegLogger = this; + + av_log_set_callback(&ffmpegLogCallback); +} + +FFmpegLogProxy::~FFmpegLogProxy() { + auto guard = std::lock_guard(globalFFmpegLogMutex); + + assert(globalFFmpegLogger == this); + av_log_set_callback(&av_log_default_callback); + globalFFmpegLogger = nullptr; +} + +void FFmpegLogProxy::log(void *ptr, int level, const char *format, va_list args) { + LogState &state = _stateByThreadId[std::this_thread::get_id()]; + + char buffer[4096]; + int status = av_log_format_line2(ptr, level, format, args, buffer, sizeof(buffer), &state.prefixFlag); + if (status < 0) { + _logger->trace(_category, "av_log_format_line2 failed with error code {}", status); + } else { + state.message += buffer; + if (state.message.ends_with('\n')) { + _logger->log(_category, FFmpegLogSource::translateFFmpegLogLevel(level), "{}", state.message); + state.message.clear(); + } + } +} + + + + + + + + + + diff --git a/src/Media/FFmpegLogProxy.h b/src/Media/FFmpegLogProxy.h new file mode 100644 index 000000000000..0f1a47cb0c81 --- /dev/null +++ b/src/Media/FFmpegLogProxy.h @@ -0,0 +1,31 @@ +#pragma once + +#include + +#include +#include +#include + +#include "Library/Logger/Logger.h" + +#include "FFmpegLogSource.h" + +class FFmpegLogProxy { + public: + explicit FFmpegLogProxy(Logger *logger); + ~FFmpegLogProxy(); + + void log(void *ptr, int level, const char *format, va_list args); + + private: + struct LogState { + int prefixFlag = 1; + std::string message; + }; + + private: + Logger *_logger = nullptr; + FFmpegLogSource _source; + LogCategory _category; + std::unordered_map _stateByThreadId; +}; diff --git a/src/Media/FFmpegLogSource.cpp b/src/Media/FFmpegLogSource.cpp new file mode 100644 index 000000000000..7e8635d5f6a9 --- /dev/null +++ b/src/Media/FFmpegLogSource.cpp @@ -0,0 +1,50 @@ +#include "FFmpegLogSource.h" + +extern "C" { +#include +} + +#include + +LogLevel FFmpegLogSource::level() const { + return translateFFmpegLogLevel(av_log_get_level()); +} + +void FFmpegLogSource::setLevel(LogLevel level) { + av_log_set_level(translateLoggerLogLevel(level)); +} + +LogLevel FFmpegLogSource::translateFFmpegLogLevel(int level) { + // Best effort translation here. + 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. + } else if (level <= AV_LOG_ERROR) { + return LOG_ERROR; + } else if (level <= AV_LOG_WARNING) { + return LOG_WARNING; + } else if (level <= AV_LOG_INFO) { + return LOG_INFO; + } else if (level <= AV_LOG_VERBOSE) { + return LOG_DEBUG; + } else if (level <= AV_LOG_DEBUG) { + return LOG_DEBUG; // AV_LOG_DEBUG & AV_LOG_VERBOSE are both translated into LOG_DEBUG. + } else { + return LOG_TRACE; + } +} + +int FFmpegLogSource::translateLoggerLogLevel(LogLevel level) { + switch (level) { + case LOG_TRACE: return AV_LOG_TRACE; + case LOG_DEBUG: return AV_LOG_DEBUG; // max(AV_LOG_DEBUG, AV_LOG_VERBOSE) + case LOG_INFO: return AV_LOG_INFO; + 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) + default: + assert(false); + return AV_LOG_TRACE; + } +} diff --git a/src/Media/FFmpegLogSource.h b/src/Media/FFmpegLogSource.h new file mode 100644 index 000000000000..d1eec078291e --- /dev/null +++ b/src/Media/FFmpegLogSource.h @@ -0,0 +1,12 @@ +#pragma once + +#include "Library/Logger/LogSource.h" + +class FFmpegLogSource : public LogSource { + public: + virtual LogLevel level() const override; + virtual void setLevel(LogLevel level) override; + + static LogLevel translateFFmpegLogLevel(int level); + static int translateLoggerLogLevel(LogLevel level); +}; diff --git a/src/Media/MediaLogger.cpp b/src/Media/MediaLogger.cpp deleted file mode 100644 index 3a890dc568d6..000000000000 --- a/src/Media/MediaLogger.cpp +++ /dev/null @@ -1,60 +0,0 @@ -#include "MediaLogger.h" - -extern "C" { -#include -} - -#include -#include - -static std::mutex GlobalMediaLoggerMutex; -static MediaLogger *GlobalMediaLoggerInstance = nullptr; - -static void GlobalMediaLoggerCallback(void *ptr, int logLevel, const char *format, va_list args) { - auto lock = std::unique_lock(GlobalMediaLoggerMutex); - - assert(GlobalMediaLoggerInstance); - - GlobalMediaLoggerInstance->log(ptr, logLevel, format, args); -} - -MediaLogger::MediaLogger(Logger *logger): _logger(logger) { - assert(logger); -} - -void MediaLogger::log(void *ptr, int logLevel, const char *format, va_list args) { - if (!_logger->shouldLog(LOG_VERBOSE)) - return; - - LogState &state = _stateByThreadId[std::this_thread::get_id()]; - - char buffer[2048]; - int status = av_log_format_line2(ptr, logLevel, format, args, buffer, sizeof(buffer), &state.prefixFlag); - if (status < 0) { - _logger->verbose("av_log_format_line2 failed with error code {}", status); - } else { - state.message += buffer; - if (state.message.ends_with('\n')) { - _logger->verbose("{}", state.message); - state.message.clear(); - } - } -} - -void MediaLogger::setGlobalMediaLogger(MediaLogger *logger) { - auto lock = std::unique_lock(GlobalMediaLoggerMutex); - - GlobalMediaLoggerInstance = logger; - - if (logger) { - av_log_set_callback(&GlobalMediaLoggerCallback); - } else { - av_log_set_callback(nullptr); - } -} - -MediaLogger *MediaLogger::globalMediaLogger() { - auto lock = std::unique_lock(GlobalMediaLoggerMutex); - - return GlobalMediaLoggerInstance; -} diff --git a/src/Media/MediaLogger.h b/src/Media/MediaLogger.h deleted file mode 100644 index 8ec14bd12e0d..000000000000 --- a/src/Media/MediaLogger.h +++ /dev/null @@ -1,30 +0,0 @@ -#pragma once - -#include - -#include -#include -#include - -#include "Library/Logger/Logger.h" - -// TODO(captainurist): rework when I get to logging, this should just be a way to hook into av_log, an API taking std::function. -class MediaLogger { - public: - explicit MediaLogger(Logger *logger); - - void log(void *ptr, int logLevel, const char *format, va_list args); - - static void setGlobalMediaLogger(MediaLogger *logger); - static MediaLogger *globalMediaLogger(); - - private: - struct LogState { - int prefixFlag = 1; - std::string message; - }; - - private: - Logger *_logger = nullptr; - std::unordered_map _stateByThreadId; -}; diff --git a/src/Media/MediaPlayer.cpp b/src/Media/MediaPlayer.cpp index f4e4b7f6beaa..34c1592905dc 100644 --- a/src/Media/MediaPlayer.cpp +++ b/src/Media/MediaPlayer.cpp @@ -25,7 +25,6 @@ extern "C" { #include "Engine/Engine.h" #include "Engine/EngineGlobals.h" #include "Engine/ErrorHandling.h" -#include "Engine/EngineIocContainer.h" #include "Engine/Graphics/IRender.h" #include "Engine/Graphics/Image.h" @@ -33,12 +32,11 @@ extern "C" { #include "Media/Audio/AudioPlayer.h" #include "Media/Audio/OpenALSoundProvider.h" -#include "Media/MediaLogger.h" +#include "Media/FFmpegLogProxy.h" #include "Utility/Memory/FreeDeleter.h" #include "Utility/DataPath.h" -#include "GUI/GUIMessageQueue.h" #include "GUI/GUIWindow.h" using namespace std::chrono_literals; // NOLINT @@ -76,7 +74,7 @@ class AVStreamWrapper { if (dec_ctx != nullptr) { // Close the codec avcodec_close(dec_ctx); - logger->verbose("ffmpeg: close decoder context file"); + logger->trace("ffmpeg: close decoder context file"); dec_ctx = nullptr; } } @@ -339,7 +337,7 @@ class Movie : public IMovie { if (format_ctx) { // Close the video file avformat_close_input(&format_ctx); - logger->verbose("close video format context file\n"); + logger->trace("close video format context file\n"); format_ctx = nullptr; } if (avioContext) { @@ -499,7 +497,7 @@ class Movie : public IMovie { if (buffer) buffq.push(buffer); } } - logger->verbose("Audio Packets Queued"); + logger->trace("Audio Packets Queued"); // reset video to start int err = avformat_seek_file(format_ctx, -1, 0, 0, 0, AVSEEK_FLAG_BACKWARD); @@ -510,7 +508,7 @@ class Movie : public IMovie { return; } start_time = std::chrono::system_clock::now(); - logger->verbose("Video stream reset"); + logger->trace("Video stream reset"); int lastvideopts = -1; int desired_frame_number; @@ -874,7 +872,7 @@ void MPlayer::PlayFullscreenMovie(const std::string &pFilename) { GraphicsImage *tex = GraphicsImage::Create(pMovie_Track->GetWidth(), pMovie_Track->GetHeight()); if (pMovie->GetFormat() == "bink") { - logger->verbose("bink file"); + logger->trace("bink file"); pMovie->PlayBink(); } else { while (true) { @@ -974,11 +972,9 @@ void MPlayer::Unload() { MPlayer::MPlayer() { static int libavcodec_initialized = false; - mediaLogger = std::make_unique(logger); - MediaLogger::setGlobalMediaLogger(mediaLogger.get()); + logProxy = std::make_unique(logger); if (!libavcodec_initialized) { - // av_log_set_level(AV_LOG_TRACE); // Register all available file formats and codecs #ifndef FF_API_NEXT avcodec_register_all(); @@ -1010,8 +1006,6 @@ MPlayer::~MPlayer() { } delete provider; - - MediaLogger::setGlobalMediaLogger(nullptr); } // AudioBaseDataSource diff --git a/src/Media/MediaPlayer.h b/src/Media/MediaPlayer.h index 05c99b66478d..8e3e2e465b27 100644 --- a/src/Media/MediaPlayer.h +++ b/src/Media/MediaPlayer.h @@ -14,7 +14,7 @@ // MOVIE_Outro "end_seq1" class VideoList; -class MediaLogger; +class FFmpegLogProxy; class MPlayer { public: @@ -33,7 +33,7 @@ class MPlayer { bool StopMovie(); protected: - std::unique_ptr mediaLogger; + std::unique_ptr logProxy; VideoList *might_list; VideoList *magic_list; std::string sInHouseMovie; diff --git a/src/Platform/CMakeLists.txt b/src/Platform/CMakeLists.txt index b9752d7596f4..afde67de9266 100644 --- a/src/Platform/CMakeLists.txt +++ b/src/Platform/CMakeLists.txt @@ -16,7 +16,7 @@ set(PLATFORM_SOURCES Sdl/SdlEnumTranslation.cpp Sdl/SdlEventLoop.cpp Sdl/SdlGamepad.cpp - Sdl/SdlLogger.cpp + Sdl/SdlLogSource.cpp Sdl/SdlOpenGLContext.cpp Sdl/SdlPlatform.cpp Sdl/SdlPlatformSharedState.cpp @@ -37,7 +37,6 @@ set(PLATFORM_HEADERS PlatformEventLoop.h PlatformEvents.h PlatformGamepad.h - PlatformLogger.h PlatformOpenGLContext.h PlatformOpenGLOptions.h PlatformWindow.h @@ -50,7 +49,7 @@ set(PLATFORM_HEADERS Sdl/SdlEnumTranslation.h Sdl/SdlEventLoop.h Sdl/SdlGamepad.h - Sdl/SdlLogger.h + Sdl/SdlLogSource.h Sdl/SdlOpenGLContext.h Sdl/SdlPlatform.h Sdl/SdlPlatformSharedState.h @@ -75,4 +74,4 @@ target_link_libraries(platform_main PRIVATE SDL2::SDL2OE) add_library(platform STATIC ${PLATFORM_SOURCES} ${PLATFORM_HEADERS}) target_check_style(platform) -target_link_libraries(platform PRIVATE SDL2::SDL2OE) +target_link_libraries(platform PUBLIC utility library_logger PRIVATE SDL2::SDL2OE) diff --git a/src/Platform/Platform.h b/src/Platform/Platform.h index de5b9a5eefc3..572862f2565b 100644 --- a/src/Platform/Platform.h +++ b/src/Platform/Platform.h @@ -13,8 +13,10 @@ class PlatformEvent; class PlatformWindow; class PlatformEventLoop; class PlatformEventHandler; -class PlatformLogger; class PlatformGamepad; +class Logger; + +// TODO(captainurist): just move Platform as another lib in Library. /** * Platform abstraction layer. @@ -71,10 +73,12 @@ class Platform { /** * Creates a standard platform. * - * @param logger Logger to use. Must not be null. + * @param logger Logger to use. Must not be null. Note that `Logger` is a singleton, so we don't + * really need to pass it in. But we want to be explicit in stating that the + * `Platform` will use a `Logger` instance, and thus the instance should exist. * @return A newly created `Platform`. This method is guaranteed to succeed. */ - static std::unique_ptr createStandardPlatform(PlatformLogger *logger); + static std::unique_ptr createStandardPlatform(Logger *logger); /** * Creates a new platform window. diff --git a/src/Platform/PlatformEnums.h b/src/Platform/PlatformEnums.h index 964b43398e73..e9af376d3f5b 100644 --- a/src/Platform/PlatformEnums.h +++ b/src/Platform/PlatformEnums.h @@ -75,31 +75,6 @@ enum class PlatformStorage { }; using enum PlatformStorage; -/** - * Platform log level as used by `PlatformLogger`. - */ -enum class PlatformLogLevel { - LOG_VERBOSE, - LOG_DEBUG, - LOG_INFO, - LOG_WARNING, - LOG_ERROR, - LOG_CRITICAL -}; -using enum PlatformLogLevel; - -// TODO(captainurist): this will be dropped -/** - * Platform log category as used by `PlatformLogger`. - * - * Note that platform doesn't have an API to define custom log categories, this should be done in user code if needed. - */ -enum class PlatformLogCategory { - PLATFORM_LOG, - APPLICATION_LOG -}; -using enum PlatformLogCategory; - enum class PlatformKey : int { // usual text input KEY_CHAR, // TODO(captainurist): this doesn't belong here diff --git a/src/Platform/PlatformLogger.h b/src/Platform/PlatformLogger.h deleted file mode 100644 index 20a57102156c..000000000000 --- a/src/Platform/PlatformLogger.h +++ /dev/null @@ -1,43 +0,0 @@ -#pragma once - -#include - -#include "PlatformEnums.h" - -// TODO(captainurist): this should be just a callback, adding a log factory to platform was a bad idea after all. -/** - * Platform-specific logger that takes all the quirks of a specific platform into account (e.g. calling - * `OutputDebugString` / `WriteConsole` on Windows). - * - * Note that platform logger doesn't provide an API for log formatting and log category management. If needed, this - * functionality should be added at the next abstraction layer. - * - * The two existing log categories are provided to differentiate logging calls made by user code and by the platform - * implementation itself. - */ -class PlatformLogger { - public: - virtual ~PlatformLogger() = default; - - static std::unique_ptr createStandardLogger(); - - virtual void setLogLevel(PlatformLogCategory category, PlatformLogLevel logLevel) = 0; - virtual PlatformLogLevel logLevel(PlatformLogCategory category) const = 0; - - /** - * Logs provided message. The message will be ignored if the provided log level is lower than the log level set - * for the provided category. It is advised to also do the log level check at the call site to avoid message - * formatting overhead: - * \code - * if (logger->logLevel(APPLICATION_LOG) <= LOG_DEBUG) - * logger->log(APPLICATION_LOG, LOG_DEBUG, fmt::format("blablabla {}", s1).c_str()); - * \endcode - * - * This function is thread-safe. - * - * @param category Message log category. - * @param logLevel Message log level. - * @param message Message to log. - */ - virtual void log(PlatformLogCategory category, PlatformLogLevel logLevel, const char *message) = 0; -}; diff --git a/src/Platform/Posix/PosixPlatform.cpp b/src/Platform/Posix/PosixPlatform.cpp index 1dc42d2d0345..fe046920bb1e 100644 --- a/src/Platform/Posix/PosixPlatform.cpp +++ b/src/Platform/Posix/PosixPlatform.cpp @@ -1,12 +1,7 @@ #include #include "Platform/Sdl/SdlPlatform.h" -#include "Platform/Sdl/SdlLogger.h" -std::unique_ptr Platform::createStandardPlatform(PlatformLogger *logger) { +std::unique_ptr Platform::createStandardPlatform(Logger *logger) { return std::make_unique(logger); } - -std::unique_ptr PlatformLogger::createStandardLogger() { - return std::make_unique(); -} diff --git a/src/Platform/Sdl/SdlEnumTranslation.cpp b/src/Platform/Sdl/SdlEnumTranslation.cpp index fcfb49a27def..e124b2003f6d 100644 --- a/src/Platform/Sdl/SdlEnumTranslation.cpp +++ b/src/Platform/Sdl/SdlEnumTranslation.cpp @@ -220,9 +220,9 @@ SDL_GLprofile translatePlatformOpenGLProfile(PlatformOpenGLProfile profile) { return SDL_GL_CONTEXT_PROFILE_CORE; // Make the compiler happy. } -SDL_LogPriority translatePlatformLogLevel(PlatformLogLevel logLevel) { +SDL_LogPriority translatePlatformLogLevel(LogLevel logLevel) { switch (logLevel) { - case LOG_VERBOSE: return SDL_LOG_PRIORITY_VERBOSE; + case LOG_TRACE: return SDL_LOG_PRIORITY_VERBOSE; case LOG_DEBUG: return SDL_LOG_PRIORITY_DEBUG; case LOG_INFO: return SDL_LOG_PRIORITY_INFO; case LOG_WARNING: return SDL_LOG_PRIORITY_WARN; @@ -234,9 +234,9 @@ SDL_LogPriority translatePlatformLogLevel(PlatformLogLevel logLevel) { } } -PlatformLogLevel translateSdlLogLevel(SDL_LogPriority logLevel) { +LogLevel translateSdlLogLevel(SDL_LogPriority logLevel) { switch (logLevel) { - case SDL_LOG_PRIORITY_VERBOSE: return LOG_VERBOSE; + case SDL_LOG_PRIORITY_VERBOSE: return LOG_TRACE; case SDL_LOG_PRIORITY_DEBUG: return LOG_DEBUG; case SDL_LOG_PRIORITY_INFO: return LOG_INFO; case SDL_LOG_PRIORITY_WARN: return LOG_WARNING; @@ -244,6 +244,6 @@ PlatformLogLevel translateSdlLogLevel(SDL_LogPriority logLevel) { case SDL_LOG_PRIORITY_CRITICAL: return LOG_CRITICAL; default: assert(false); - return LOG_VERBOSE; + return LOG_TRACE; } } diff --git a/src/Platform/Sdl/SdlEnumTranslation.h b/src/Platform/Sdl/SdlEnumTranslation.h index 279c33071035..5f80758808d9 100644 --- a/src/Platform/Sdl/SdlEnumTranslation.h +++ b/src/Platform/Sdl/SdlEnumTranslation.h @@ -7,6 +7,8 @@ #include "Platform/PlatformEnums.h" #include "Platform/PlatformOpenGLOptions.h" +#include "Library/Logger/LogEnums.h" + PlatformKey translateSdlKey(SDL_Scancode key); PlatformKey translateSdlGamepadButton(SDL_GameControllerButton button); PlatformKey translateSdlGamepadAxis(SDL_GameControllerAxis axis); @@ -15,5 +17,5 @@ PlatformMouseButton translateSdlMouseButton(uint8_t mouseButton); PlatformMouseButtons translateSdlMouseButtons(uint32_t mouseButtons); int translatePlatformVSyncMode(PlatformVSyncMode vsyncMode); SDL_GLprofile translatePlatformOpenGLProfile(PlatformOpenGLProfile profile); -SDL_LogPriority translatePlatformLogLevel(PlatformLogLevel logLevel); -PlatformLogLevel translateSdlLogLevel(SDL_LogPriority logLevel); +SDL_LogPriority translatePlatformLogLevel(LogLevel logLevel); +LogLevel translateSdlLogLevel(SDL_LogPriority logLevel); diff --git a/src/Platform/Sdl/SdlEventLoop.cpp b/src/Platform/Sdl/SdlEventLoop.cpp index b33f8747d22a..9f11918b6056 100644 --- a/src/Platform/Sdl/SdlEventLoop.cpp +++ b/src/Platform/Sdl/SdlEventLoop.cpp @@ -16,7 +16,7 @@ #include "SdlPlatformSharedState.h" #include "SdlEnumTranslation.h" -#include "SdlLogger.h" +#include "SdlLogSource.h" #include "SdlWindow.h" #include "SdlGamepad.h" diff --git a/src/Platform/Sdl/SdlLogSource.cpp b/src/Platform/Sdl/SdlLogSource.cpp new file mode 100644 index 000000000000..a05e51dd409a --- /dev/null +++ b/src/Platform/Sdl/SdlLogSource.cpp @@ -0,0 +1,26 @@ +#include "SdlLogSource.h" + +#include + +#include "SdlEnumTranslation.h" + +LogLevel SdlLogSource::level() const { + // `SDL_LOG_CATEGORY_CUSTOM` will query `SDL_default_priority` on a call to `SDL_LogGetPriority`. + return translateSdlLogLevel(SDL_LogGetPriority(SDL_LOG_CATEGORY_CUSTOM)); +} + +void SdlLogSource::setLevel(LogLevel logLevel) { + SDL_LogPriority assertPriority = SDL_LogGetPriority(SDL_LOG_CATEGORY_ASSERT); + + // This sets default log priority. Note that this call is better than going through all the different + // categories and calling `SDL_LogSetPriority` because it sets a single global variable inside SDL + // (`SDL_default_priority`). Making several calls would instead allocate a linked list of category-priority + // pairs which will then be traversed in O(n) every time `SDL_Log` is called. + SDL_LogSetAllPriority(translatePlatformLogLevel(logLevel)); + + // Then we need to roll back assert priority. All SDL asserts are issued at `SDL_LOG_PRIORITY_WARN` which + // makes little sense, they should really be at `SDL_LOG_PRIORITY_CRITICAL`. See code in `sdlLogCallback`. + SDL_LogSetPriority(SDL_LOG_CATEGORY_ASSERT, assertPriority); + + // Note that we don't touch SDL_LOG_CATEGORY_APPLICATION because we're not using it. +} diff --git a/src/Platform/Sdl/SdlLogSource.h b/src/Platform/Sdl/SdlLogSource.h new file mode 100644 index 000000000000..94003f86be39 --- /dev/null +++ b/src/Platform/Sdl/SdlLogSource.h @@ -0,0 +1,9 @@ +#pragma once + +#include + +class SdlLogSource: public LogSource { + public: + virtual LogLevel level() const override; + virtual void setLevel(LogLevel level) override; +}; diff --git a/src/Platform/Sdl/SdlLogger.cpp b/src/Platform/Sdl/SdlLogger.cpp deleted file mode 100644 index 89e0233b99fc..000000000000 --- a/src/Platform/Sdl/SdlLogger.cpp +++ /dev/null @@ -1,65 +0,0 @@ -#include "SdlLogger.h" - -#include -#include - -#include - -#include "SdlEnumTranslation.h" - -// This one is not in `SdlEnumTranslation.h` because it's closely tied to the implementation below. -static SDL_LogCategory translatePlatformLogCategory(PlatformLogCategory category) { - if (category == APPLICATION_LOG) { - return SDL_LOG_CATEGORY_APPLICATION; - } else { - assert(category == PLATFORM_LOG); - - // `SDL_LOG_CATEGORY_CUSTOM` will query `SDL_default_priority` on a call to `SDL_LogGetPriority`. - return SDL_LOG_CATEGORY_CUSTOM; - } -} - -void SdlLogger::setLogLevel(PlatformLogCategory category, PlatformLogLevel logLevel) { - if (category == APPLICATION_LOG) { - SDL_LogSetPriority(SDL_LOG_CATEGORY_APPLICATION, translatePlatformLogLevel(logLevel)); - } else { - assert(category == PLATFORM_LOG); - SDL_LogPriority applicationPriority = SDL_LogGetPriority(SDL_LOG_CATEGORY_APPLICATION); - SDL_LogPriority assertPriority = SDL_LogGetPriority(SDL_LOG_CATEGORY_ASSERT); - - // This sets default log priority. Note that this call is better than going through all the different - // categories and calling `SDL_LogSetPriority` because it sets a single global variable inside SDL - // (`SDL_default_priority`). Making several calls would instead allocate a linked list of category-priority - // pairs which will then be traversed in O(n) every time we call `SDL_Log`. - SDL_LogSetAllPriority(translatePlatformLogLevel(logLevel)); - - // Then we need to roll back assert priority. All SDL asserts are issued at `SDL_LOG_PRIORITY_WARN` which - // makes little sense, they should be at `SDL_LOG_PRIORITY_CRITICAL`. - SDL_LogSetPriority(SDL_LOG_CATEGORY_ASSERT, assertPriority); - - // And also roll back application priority. - SDL_LogSetPriority(SDL_LOG_CATEGORY_APPLICATION, applicationPriority); - } -} - -PlatformLogLevel SdlLogger::logLevel(PlatformLogCategory category) const { - return translateSdlLogLevel(SDL_LogGetPriority(translatePlatformLogCategory(category))); -} - -void SdlLogger::log(PlatformLogCategory category, PlatformLogLevel logLevel, const char *message) { - // SDL_LogMessage is thread-safe, thus this function is also thread-safe. The only caveat is that SDL_Init - // should be called before creating several threads that do logging, but we don't care to check for that. - SDL_LogCategory sdlCategory = translatePlatformLogCategory(category); - SDL_LogPriority sdlPriority = translatePlatformLogLevel(logLevel); -#ifdef __APPLE__ - // SDL_LogMessage on apple uses NSLog, and it prepends timestamp - // TODO(captainurist): check if android does the same. - SDL_LogMessage(sdlCategory, sdlPriority, "%s", message); -#else - // On other OSes we have to add timestamp manually - time_t t = time(NULL); - struct tm tm = *localtime(&t); - SDL_LogMessage(sdlCategory, sdlPriority, "[%04d/%02d/%02d %02d:%02d:%02d] %s", - tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec, message); -#endif -} diff --git a/src/Platform/Sdl/SdlLogger.h b/src/Platform/Sdl/SdlLogger.h deleted file mode 100644 index 6f7f5d742ed2..000000000000 --- a/src/Platform/Sdl/SdlLogger.h +++ /dev/null @@ -1,11 +0,0 @@ -#pragma once - -#include - -class SdlLogger: public PlatformLogger { - public: - virtual void setLogLevel(PlatformLogCategory category, PlatformLogLevel logLevel) override; - virtual PlatformLogLevel logLevel(PlatformLogCategory category) const override; - - virtual void log(PlatformLogCategory category, PlatformLogLevel logLevel, const char *message) override; -}; diff --git a/src/Platform/Sdl/SdlPlatform.cpp b/src/Platform/Sdl/SdlPlatform.cpp index f3ec51b1537a..ada6b16a61b5 100644 --- a/src/Platform/Sdl/SdlPlatform.cpp +++ b/src/Platform/Sdl/SdlPlatform.cpp @@ -8,17 +8,30 @@ #include "Platform/PlatformEventHandler.h" +#include "Library/Logger/Logger.h" + #include "SdlPlatformSharedState.h" #include "SdlEventLoop.h" #include "SdlWindow.h" -#include "SdlLogger.h" #include "SdlGamepad.h" +#include "SdlEnumTranslation.h" + +static void SDLCALL sdlLogCallback(void *userdata, int category, SDL_LogPriority priority, const char *message) { + LogLevel level = translateSdlLogLevel(priority); + if (category == SDL_LOG_CATEGORY_ASSERT) + level = LOG_CRITICAL; // This is an assertion, damn it! But SDL issues these at SDL_LOG_PRIORITY_WARN. -SdlPlatform::SdlPlatform(PlatformLogger *logger) { + SdlPlatformSharedState *state = static_cast(userdata); + state->logger()->log(state->logCategory(), level, "{}", message); +} + +SdlPlatform::SdlPlatform(Logger *logger) { assert(logger); _state = std::make_unique(logger); + SDL_LogSetOutputFunction(&sdlLogCallback, _state.get()); + _initialized = SDL_Init(SDL_INIT_VIDEO | SDL_INIT_GAMECONTROLLER) == 0; if (!_initialized) { _state->logSdlError("SDL_Init"); diff --git a/src/Platform/Sdl/SdlPlatform.h b/src/Platform/Sdl/SdlPlatform.h index 1b493ddcb02a..4da00f3f1ba9 100644 --- a/src/Platform/Sdl/SdlPlatform.h +++ b/src/Platform/Sdl/SdlPlatform.h @@ -12,7 +12,7 @@ class SdlPlatformSharedState; class SdlPlatform: public Platform { public: - explicit SdlPlatform(PlatformLogger *logger); + explicit SdlPlatform(Logger *logger); virtual ~SdlPlatform(); virtual std::unique_ptr createWindow() override; diff --git a/src/Platform/Sdl/SdlPlatformSharedState.cpp b/src/Platform/Sdl/SdlPlatformSharedState.cpp index 2b51f7df93e5..7c4339e517ff 100644 --- a/src/Platform/Sdl/SdlPlatformSharedState.cpp +++ b/src/Platform/Sdl/SdlPlatformSharedState.cpp @@ -2,14 +2,16 @@ #include +#include "Library/Logger/Logger.h" + #include "Utility/MapAccess.h" #include "SdlWindow.h" #include "SdlPlatform.h" -#include "SdlLogger.h" +#include "SdlLogSource.h" #include "SdlGamepad.h" -SdlPlatformSharedState::SdlPlatformSharedState(PlatformLogger *logger): _logger(logger) { +SdlPlatformSharedState::SdlPlatformSharedState(Logger *logger): _logger(logger), _logCategory("sdl", &_logSource) { assert(logger); } @@ -18,11 +20,15 @@ SdlPlatformSharedState::~SdlPlatformSharedState() { } void SdlPlatformSharedState::logSdlError(const char *sdlFunctionName) { - // Note that we cannot use `SDL_Log` here because we have no guarantees on the actual type of the logger - // that was passed in constructor. - char buffer[1024]; - snprintf(buffer, sizeof(buffer), "SDL error in %s: %s.", sdlFunctionName, SDL_GetError()); - _logger->log(PLATFORM_LOG, LOG_ERROR, buffer); + _logger->error(_logCategory, "SDL error in {}: {}", sdlFunctionName, SDL_GetError()); +} + +const LogCategory &SdlPlatformSharedState::logCategory() const { + return _logCategory; +} + +Logger *SdlPlatformSharedState::logger() const { + return _logger; } void SdlPlatformSharedState::registerWindow(SdlWindow *window) { diff --git a/src/Platform/Sdl/SdlPlatformSharedState.h b/src/Platform/Sdl/SdlPlatformSharedState.h index 6dd5a62a1fb8..16c3b43b56f3 100644 --- a/src/Platform/Sdl/SdlPlatformSharedState.h +++ b/src/Platform/Sdl/SdlPlatformSharedState.h @@ -7,20 +7,26 @@ #include #include +#include "Library/Logger/LogCategory.h" + +#include "SdlLogSource.h" + class SdlWindow; class SdlPlatform; class SdlGamepad; class PlatformEvent; class PlatformEventHandler; -class PlatformLogger; class PlatformGamepad; +class Logger; class SdlPlatformSharedState { public: - explicit SdlPlatformSharedState(PlatformLogger *logger); + explicit SdlPlatformSharedState(Logger *logger); ~SdlPlatformSharedState(); void logSdlError(const char *sdlFunctionName); + const LogCategory &logCategory() const; + Logger *logger() const; void registerWindow(SdlWindow *window); void unregisterWindow(SdlWindow *window); @@ -34,7 +40,9 @@ class SdlPlatformSharedState { SdlGamepad *gamepad(SDL_JoystickID id) const; private: - PlatformLogger *_logger = nullptr; + Logger *_logger = nullptr; + SdlLogSource _logSource; + LogCategory _logCategory; std::unordered_map _windowById; std::unordered_map> _gamepadById; }; diff --git a/src/Platform/Win/WinPlatform.cpp b/src/Platform/Win/WinPlatform.cpp index 10ea810e7f7b..cc2050401b77 100644 --- a/src/Platform/Win/WinPlatform.cpp +++ b/src/Platform/Win/WinPlatform.cpp @@ -5,7 +5,7 @@ #define WIN32_LEAN_AND_MEAN #include -#include "Platform/Sdl/SdlLogger.h" +#include "Platform/Sdl/SdlLogSource.h" #include "Utility/String.h" @@ -99,10 +99,6 @@ std::string WinPlatform::winQueryRegistry(const std::wstring &path) const { return {}; } -std::unique_ptr Platform::createStandardPlatform(PlatformLogger *logger) { +std::unique_ptr Platform::createStandardPlatform(Logger *logger) { return std::make_unique(logger); } - -std::unique_ptr PlatformLogger::createStandardLogger() { - return std::make_unique(); -} diff --git a/test/Bin/GameTest/GameTestOptions.cpp b/test/Bin/GameTest/GameTestOptions.cpp index eade1f890e6c..dc434effcbae 100644 --- a/test/Bin/GameTest/GameTestOptions.cpp +++ b/test/Bin/GameTest/GameTestOptions.cpp @@ -19,13 +19,22 @@ GameTestOptions GameTestOptions::parse(int argc, char **argv) { auto testPathOption = app->add_option("--test-path", testPath, "Path to test data dir.")->check(CLI::ExistingDirectory)->option_text("PATH")->group(requiredOptions); - app->add_option("--data-path", result.dataPath, - "Path to game data dir.")->check(CLI::ExistingDirectory)->option_text("PATH")->group(otherOptions); - app->add_flag("--headless", result.headless, - "Run in headless mode.")->group(otherOptions); - app->add_flag("--gtest_list_tests", result.listRequested, - "List the names of all tests instead of running them.")->group(""); // group("") hides the option. + app->add_option( + "--data-path", result.dataPath, + "Path to game data dir.")->check(CLI::ExistingDirectory)->option_text("PATH")->group(otherOptions); + app->add_flag( + "--headless", result.headless, + "Run in headless mode.")->group(otherOptions); + app->add_option( + "--log-level", result.logLevel, + "Log level, one of 'trace', 'debug', 'info', 'warning', 'error', 'critical'.")->option_text("LOG_LEVEL"); + app->add_flag_callback( + "-v,--verbose", [&] { result.logLevel = LOG_TRACE; }, + "Set log level to 'trace'."); app->set_help_flag("-h,--help", "Print help and exit.")->group(otherOptions); + app->add_flag( + "--gtest_list_tests", result.listRequested, + "List the names of all tests instead of running them.")->group(""); // group("") hides the option. app->allow_extras(); app->parse(argc, argv, result.helpPrinted); diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt index d540b2522a53..407c0f23c86d 100644 --- a/thirdparty/CMakeLists.txt +++ b/thirdparty/CMakeLists.txt @@ -29,6 +29,12 @@ add_library(mini INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/mini/src/mini/ini.h) add_library(mini::mini ALIAS mini) target_include_directories(mini INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/mini/src) +# spdlog +set(SPDLOG_FMT_EXTERNAL ON) +set(SPDLOG_DISABLE_DEFAULT_LOGGER ON) +set(SPDLOG_WCHAR_SUPPORT ON) # Use unicode APIs on Windows. +add_subdirectory(spdlog) + # openal_soft if (OPENAL_FOUND) # Resolved in Dependencies.cmake diff --git a/thirdparty/spdlog b/thirdparty/spdlog new file mode 160000 index 000000000000..ddce42155e67 --- /dev/null +++ b/thirdparty/spdlog @@ -0,0 +1 @@ +Subproject commit ddce42155e67589a8b1534c4935242f759c07646