From d8d0cfd5c8ef4465e5a4118701ad793d4ab032c4 Mon Sep 17 00:00:00 2001 From: johndoe6345789 Date: Sun, 4 Jan 2026 14:17:18 +0000 Subject: [PATCH] feat: Implement logger service and refactor logging across application controllers --- src/app/service_based_app.cpp | 32 ++++-- src/app/service_based_app.hpp | 2 + src/controllers/application_controller.cpp | 24 ++-- src/controllers/application_controller.hpp | 2 + src/controllers/lifecycle_controller.cpp | 24 ++-- src/controllers/lifecycle_controller.hpp | 2 + src/controllers/render_controller.cpp | 16 +-- src/controllers/render_controller.hpp | 2 + src/services/impl/logger_service.hpp | 100 ++++++++++++++++ src/services/interfaces/i_logger.hpp | 128 +++++++++++++++++++++ 10 files changed, 290 insertions(+), 42 deletions(-) create mode 100644 src/services/impl/logger_service.hpp create mode 100644 src/services/interfaces/i_logger.hpp diff --git a/src/app/service_based_app.cpp b/src/app/service_based_app.cpp index aa3d30f..0ac1fe5 100644 --- a/src/app/service_based_app.cpp +++ b/src/app/service_based_app.cpp @@ -17,25 +17,30 @@ #include "services/impl/sdl_audio_service.hpp" #include "services/impl/vulkan_gui_service.hpp" #include "services/impl/bullet_physics_service.hpp" +#include "services/impl/logger_service.hpp" #include namespace sdl3cpp::app { ServiceBasedApp::ServiceBasedApp(const std::filesystem::path& scriptPath) : scriptPath_(scriptPath) { - logging::Logger::GetInstance().Info("ServiceBasedApp::ServiceBasedApp: constructor starting"); + logging::Logger::GetInstance().Trace("ServiceBasedApp::ServiceBasedApp: constructor starting"); try { logging::Logger::GetInstance().Info("ServiceBasedApp::ServiceBasedApp: Setting up SDL"); SetupSDL(); logging::Logger::GetInstance().Info("ServiceBasedApp::ServiceBasedApp: Registering services"); RegisterServices(); - logging::Logger::GetInstance().Info("ServiceBasedApp::ServiceBasedApp: Creating controllers"); + + // Get the logger service after registration + logger_ = registry_.GetService(); + + logger_->Info("ServiceBasedApp::ServiceBasedApp: Creating controllers"); lifecycleController_ = std::make_unique(registry_); applicationController_ = std::make_unique(registry_); - logging::Logger::GetInstance().Info("ServiceBasedApp::ServiceBasedApp: constructor completed"); + logger_->Info("ServiceBasedApp::ServiceBasedApp: constructor completed"); } catch (const std::exception& e) { logging::Logger::GetInstance().Error("ServiceBasedApp::ServiceBasedApp: Failed to initialize ServiceBasedApp: " + std::string(e.what())); throw; @@ -43,14 +48,16 @@ ServiceBasedApp::ServiceBasedApp(const std::filesystem::path& scriptPath) } ServiceBasedApp::~ServiceBasedApp() { - logging::TraceGuard trace("ServiceBasedApp::~ServiceBasedApp"); + logger_->Trace("ServiceBasedApp", "~ServiceBasedApp", "", "Entering"); applicationController_.reset(); lifecycleController_.reset(); + + logger_->Trace("ServiceBasedApp", "~ServiceBasedApp", "", "Exiting"); } void ServiceBasedApp::Run() { - logging::Logger::GetInstance().Trace("ServiceBasedApp::Run: Entering"); + logger_->Trace("ServiceBasedApp", "Run", "", "Entering"); try { // Initialize all services @@ -97,25 +104,28 @@ void ServiceBasedApp::Run() { // Shutdown all services lifecycleController_->ShutdownAll(); - logging::Logger::GetInstance().Trace("ServiceBasedApp::Run: Exiting"); + logger_->Trace("ServiceBasedApp", "Run", "", "Exiting"); } catch (const std::exception& e) { - logging::Logger::GetInstance().Error("ServiceBasedApp::Run: Application error: " + std::string(e.what())); + logger_->Error("ServiceBasedApp::Run: Application error: " + std::string(e.what())); throw; } } void ServiceBasedApp::SetupSDL() { - logging::Logger::GetInstance().Trace("ServiceBasedApp::SetupSDL: Entering"); + logger_->Trace("ServiceBasedApp", "SetupSDL", "", "Entering"); // SDL initialization is handled by the window service // Don't initialize SDL here to avoid double initialization - logging::Logger::GetInstance().Trace("ServiceBasedApp::SetupSDL: Exiting"); + logger_->Trace("ServiceBasedApp", "SetupSDL", "", "Exiting"); } void ServiceBasedApp::RegisterServices() { - logging::Logger::GetInstance().Trace("ServiceBasedApp::RegisterServices: Entering"); + logger_->Trace("ServiceBasedApp", "RegisterServices", "", "Entering"); + + // Logger service (needed by all other services) + registry_.RegisterService(); // Event bus (needed by window service) registry_.RegisterService(); @@ -179,7 +189,7 @@ void ServiceBasedApp::RegisterServices() { // Physics service registry_.RegisterService(); - logging::Logger::GetInstance().Trace("ServiceBasedApp::RegisterServices: Exiting"); + logger_->Trace("ServiceBasedApp", "RegisterServices", "", "Exiting"); } } // namespace sdl3cpp::app \ No newline at end of file diff --git a/src/app/service_based_app.hpp b/src/app/service_based_app.hpp index 7721ac2..06e11d1 100644 --- a/src/app/service_based_app.hpp +++ b/src/app/service_based_app.hpp @@ -6,6 +6,7 @@ #include "di/service_registry.hpp" #include "controllers/lifecycle_controller.hpp" #include "controllers/application_controller.hpp" +#include "services/interfaces/i_logger.hpp" namespace sdl3cpp::app { @@ -35,6 +36,7 @@ private: di::ServiceRegistry registry_; std::unique_ptr lifecycleController_; std::unique_ptr applicationController_; + std::shared_ptr logger_; }; } // namespace sdl3cpp::app \ No newline at end of file diff --git a/src/controllers/application_controller.cpp b/src/controllers/application_controller.cpp index b86994e..2ecd76d 100644 --- a/src/controllers/application_controller.cpp +++ b/src/controllers/application_controller.cpp @@ -1,6 +1,6 @@ #include "application_controller.hpp" #include "render_controller.hpp" -#include "../logging/logger.hpp" +#include "../services/interfaces/i_logger.hpp" #include "../services/interfaces/i_window_service.hpp" #include "../services/interfaces/i_input_service.hpp" #include "../services/interfaces/i_physics_service.hpp" @@ -12,18 +12,18 @@ namespace sdl3cpp::controllers { ApplicationController::ApplicationController(di::ServiceRegistry& registry) - : registry_(registry), running_(false) { - logging::Logger::GetInstance().Trace("ApplicationController::ApplicationController: Created"); + : registry_(registry), logger_(registry.GetService()), running_(false) { + logger_->Trace("ApplicationController", "ApplicationController", "", "Created"); } ApplicationController::~ApplicationController() { - logging::Logger::GetInstance().Trace("ApplicationController::~ApplicationController: Destroyed"); + logger_->Trace("ApplicationController", "~ApplicationController", "", "Destroyed"); } void ApplicationController::Run() { - logging::Logger::GetInstance().Trace("ApplicationController::Run: Entering"); - logging::Logger::GetInstance().Info("ApplicationController::Run: ApplicationController::Run starting"); - logging::Logger::GetInstance().Info("ApplicationController::Run: Application starting main loop"); + logger_->Trace("ApplicationController::Run: Entering"); + logger_->Info("ApplicationController::Run: ApplicationController::Run starting"); + logger_->Info("ApplicationController::Run: Application starting main loop"); running_ = true; auto lastTime = std::chrono::high_resolution_clock::now(); @@ -35,7 +35,7 @@ void ApplicationController::Run() { // Exit after timeout for headless testing if (currentTime - startTime > timeout) { - logging::Logger::GetInstance().Info("Application timeout reached, exiting"); + logger_->Info("ApplicationController::Run: Application timeout reached, exiting"); running_ = false; break; } @@ -47,8 +47,8 @@ void ApplicationController::Run() { ProcessFrame(deltaTime); } - logging::Logger::GetInstance().Info("ApplicationController::Run: Application exiting main loop"); - logging::Logger::GetInstance().Trace("ApplicationController::Run: Exiting"); + logger_->Info("ApplicationController::Run: Application exiting main loop"); + logger_->Trace("ApplicationController::Run: Exiting"); } void ApplicationController::HandleEvents() { @@ -72,7 +72,7 @@ void ApplicationController::HandleEvents() { } void ApplicationController::ProcessFrame(float deltaTime) { - logging::TraceGuard trace; + logger_->Trace("ApplicationController::ProcessFrame: Entering"); // Update physics auto physicsService = registry_.GetService(); @@ -91,6 +91,8 @@ void ApplicationController::ProcessFrame(float deltaTime) { // auto renderController = std::make_unique(registry_); // renderController->RenderFrame(static_cast(std::chrono::duration_cast( // std::chrono::high_resolution_clock::now().time_since_epoch()).count()) / 1000.0f); + + logger_->Trace("ApplicationController::ProcessFrame: Exiting"); } } // namespace sdl3cpp::controllers diff --git a/src/controllers/application_controller.hpp b/src/controllers/application_controller.hpp index 316ed44..3306a67 100644 --- a/src/controllers/application_controller.hpp +++ b/src/controllers/application_controller.hpp @@ -1,6 +1,7 @@ #pragma once #include "../di/service_registry.hpp" +#include "../services/interfaces/i_logger.hpp" #include namespace sdl3cpp::controllers { @@ -31,6 +32,7 @@ private: void HandleEvents(); di::ServiceRegistry& registry_; + std::shared_ptr logger_; bool running_ = false; }; diff --git a/src/controllers/lifecycle_controller.cpp b/src/controllers/lifecycle_controller.cpp index 51c74f8..1aa99f1 100644 --- a/src/controllers/lifecycle_controller.cpp +++ b/src/controllers/lifecycle_controller.cpp @@ -1,37 +1,37 @@ #include "lifecycle_controller.hpp" -#include "../logging/logger.hpp" +#include "../services/interfaces/i_logger.hpp" namespace sdl3cpp::controllers { LifecycleController::LifecycleController(di::ServiceRegistry& registry) - : registry_(registry) { - logging::Logger::GetInstance().Trace("LifecycleController::LifecycleController: Created"); + : registry_(registry), logger_(registry.GetService()) { + logger_->Trace("LifecycleController", "LifecycleController", "", "Created"); } LifecycleController::~LifecycleController() { - logging::Logger::GetInstance().Trace("LifecycleController::~LifecycleController: Destroyed"); + logger_->Trace("LifecycleController", "~LifecycleController", "", "Destroyed"); } void LifecycleController::InitializeAll() { - logging::Logger::GetInstance().Trace("LifecycleController::InitializeAll: Entering"); - logging::Logger::GetInstance().Info("LifecycleController::InitializeAll: Initializing all services"); + logger_->Trace("LifecycleController", "InitializeAll", "", "Entering"); + logger_->Info("LifecycleController::InitializeAll: Initializing all services"); // ServiceRegistry handles initialization order based on dependencies registry_.InitializeAll(); - logging::Logger::GetInstance().Info("LifecycleController::InitializeAll: All services initialized"); - logging::Logger::GetInstance().Trace("LifecycleController::InitializeAll: Exiting"); + logger_->Info("LifecycleController::InitializeAll: All services initialized"); + logger_->Trace("LifecycleController", "InitializeAll", "", "Exiting"); } void LifecycleController::ShutdownAll() noexcept { - logging::Logger::GetInstance().Trace("LifecycleController::ShutdownAll: Entering"); - logging::Logger::GetInstance().Info("LifecycleController::ShutdownAll: Shutting down all services"); + logger_->Trace("LifecycleController", "ShutdownAll", "", "Entering"); + logger_->Info("LifecycleController::ShutdownAll: Shutting down all services"); // ServiceRegistry handles shutdown in reverse dependency order registry_.ShutdownAll(); - logging::Logger::GetInstance().Info("LifecycleController::ShutdownAll: All services shutdown"); - logging::Logger::GetInstance().Trace("LifecycleController::ShutdownAll: Exiting"); + logger_->Info("LifecycleController::ShutdownAll: All services shutdown"); + logger_->Trace("LifecycleController::ShutdownAll: Exiting"); } } // namespace sdl3cpp::controllers diff --git a/src/controllers/lifecycle_controller.hpp b/src/controllers/lifecycle_controller.hpp index 7fee97a..3448644 100644 --- a/src/controllers/lifecycle_controller.hpp +++ b/src/controllers/lifecycle_controller.hpp @@ -1,6 +1,7 @@ #pragma once #include "../di/service_registry.hpp" +#include "../services/interfaces/i_logger.hpp" namespace sdl3cpp::controllers { @@ -30,6 +31,7 @@ public: private: di::ServiceRegistry& registry_; + std::shared_ptr logger_; }; } // namespace sdl3cpp::controllers diff --git a/src/controllers/render_controller.cpp b/src/controllers/render_controller.cpp index 2b269e7..7b5307b 100644 --- a/src/controllers/render_controller.cpp +++ b/src/controllers/render_controller.cpp @@ -1,5 +1,5 @@ #include "render_controller.hpp" -#include "../logging/logger.hpp" +#include "../services/interfaces/i_logger.hpp" #include "../services/interfaces/i_graphics_service.hpp" #include "../services/interfaces/i_script_service.hpp" #include "../services/interfaces/i_gui_service.hpp" @@ -8,16 +8,16 @@ namespace sdl3cpp::controllers { RenderController::RenderController(di::ServiceRegistry& registry) - : registry_(registry) { - logging::Logger::GetInstance().Trace("RenderController::RenderController: Created"); + : registry_(registry), logger_(registry.GetService()) { + logger_->Trace("RenderController::RenderController: Created"); } RenderController::~RenderController() { - logging::Logger::GetInstance().Trace("RenderController::~RenderController: Destroyed"); + logger_->Trace("RenderController::~RenderController: Destroyed"); } void RenderController::RenderFrame(float time) { - logging::Logger::GetInstance().Trace("RenderController::RenderFrame: Entering"); + logger_->Trace("RenderController::RenderFrame: Entering"); // Get required services auto graphicsService = registry_.GetService(); @@ -26,8 +26,8 @@ void RenderController::RenderFrame(float time) { auto sceneService = registry_.GetService(); if (!graphicsService) { - logging::Logger::GetInstance().Error("RenderController::RenderFrame: Graphics service not available"); - logging::Logger::GetInstance().Trace("RenderController::RenderFrame: Exiting"); + logger_->Error("RenderController::RenderFrame: Graphics service not available"); + logger_->Trace("RenderController::RenderFrame: Exiting"); return; } @@ -61,7 +61,7 @@ void RenderController::RenderFrame(float time) { // End frame and present graphicsService->EndFrame(); - logging::Logger::GetInstance().Trace("RenderController::RenderFrame: Exiting"); + logger_->Trace("RenderController::RenderFrame: Exiting"); } } // namespace sdl3cpp::controllers diff --git a/src/controllers/render_controller.hpp b/src/controllers/render_controller.hpp index 34efeab..3b8a3cb 100644 --- a/src/controllers/render_controller.hpp +++ b/src/controllers/render_controller.hpp @@ -1,6 +1,7 @@ #pragma once #include "../di/service_registry.hpp" +#include "../services/interfaces/i_logger.hpp" #include namespace sdl3cpp::controllers { @@ -28,6 +29,7 @@ public: private: di::ServiceRegistry& registry_; + std::shared_ptr logger_; }; } // namespace sdl3cpp::controllers diff --git a/src/services/impl/logger_service.hpp b/src/services/impl/logger_service.hpp new file mode 100644 index 0000000..ebde8e7 --- /dev/null +++ b/src/services/impl/logger_service.hpp @@ -0,0 +1,100 @@ +#pragma once + +#include "../interfaces/i_logger.hpp" +#include "../../logging/logger.hpp" + +namespace sdl3cpp::services::impl { + +/** + * @brief Logger service implementation. + * + * Wraps the existing Logger singleton to provide DI-compatible logging. + * Small, focused service (~30 lines) that adapts the logger for dependency injection. + */ +class LoggerService : public ILogger { +public: + LoggerService() = default; + ~LoggerService() override = default; + + // ILogger interface + void SetLevel(LogLevel level) override { + logging::Logger::GetInstance().SetLevel(static_cast(level)); + } + + LogLevel GetLevel() const override { + return static_cast(logging::Logger::GetInstance().GetLevel()); + } + + void SetOutputFile(const std::string& filename) override { + logging::Logger::GetInstance().SetOutputFile(filename); + } + + void EnableConsoleOutput(bool enable) override { + logging::Logger::GetInstance().EnableConsoleOutput(enable); + } + + void Log(LogLevel level, const std::string& message) override { + logging::Logger::GetInstance().Log(static_cast(level), message); + } + + void Trace(const std::string& message) override { + logging::Logger::GetInstance().Trace(message); + } + + void Trace(const std::string& className, const std::string& methodName, const std::string& args, const std::string& message) override { + std::string formattedMessage = className + "::" + methodName; + if (!args.empty()) { + formattedMessage += "(" + args + ")"; + } + if (!message.empty()) { + formattedMessage += ": " + message; + } + logging::Logger::GetInstance().Trace(formattedMessage); + } + + void Debug(const std::string& message) override { + logging::Logger::GetInstance().Debug(message); + } + + void Info(const std::string& message) override { + logging::Logger::GetInstance().Info(message); + } + + void Warn(const std::string& message) override { + logging::Logger::GetInstance().Warn(message); + } + + void Error(const std::string& message) override { + logging::Logger::GetInstance().Error(message); + } + + void TraceFunction(const std::string& funcName) override { + logging::Logger::GetInstance().TraceFunction(funcName); + } + + void TraceVariable(const std::string& name, const std::string& value) override { + logging::Logger::GetInstance().TraceVariable(name, value); + } + + void TraceVariable(const std::string& name, int value) override { + logging::Logger::GetInstance().TraceVariable(name, value); + } + + void TraceVariable(const std::string& name, size_t value) override { + logging::Logger::GetInstance().TraceVariable(name, value); + } + + void TraceVariable(const std::string& name, bool value) override { + logging::Logger::GetInstance().TraceVariable(name, value); + } + + void TraceVariable(const std::string& name, float value) override { + logging::Logger::GetInstance().TraceVariable(name, value); + } + + void TraceVariable(const std::string& name, double value) override { + logging::Logger::GetInstance().TraceVariable(name, value); + } +}; + +} // namespace sdl3cpp::services::impl \ No newline at end of file diff --git a/src/services/interfaces/i_logger.hpp b/src/services/interfaces/i_logger.hpp new file mode 100644 index 0000000..9cc33f3 --- /dev/null +++ b/src/services/interfaces/i_logger.hpp @@ -0,0 +1,128 @@ +#pragma once + +#include + +namespace sdl3cpp::services { + +enum class LogLevel { + TRACE = 0, + DEBUG = 1, + INFO = 2, + WARN = 3, + ERROR = 4, + OFF = 5 +}; + +/** + * @brief Logger service interface. + * + * Provides logging functionality with different log levels. + * Small, focused service (~50 lines) for application logging. + */ +class ILogger { +public: + virtual ~ILogger() = default; + + /** + * @brief Set the logging level. + * + * @param level The minimum level to log + */ + virtual void SetLevel(LogLevel level) = 0; + + /** + * @brief Get the current logging level. + * + * @return Current log level + */ + virtual LogLevel GetLevel() const = 0; + + /** + * @brief Set the output file for logging. + * + * @param filename Path to the log file + */ + virtual void SetOutputFile(const std::string& filename) = 0; + + /** + * @brief Enable or disable console output. + * + * @param enable True to enable console output + */ + virtual void EnableConsoleOutput(bool enable) = 0; + + /** + * @brief Log a message with the specified level. + * + * @param level Log level + * @param message Log message + */ + virtual void Log(LogLevel level, const std::string& message) = 0; + + /** + * @brief Log a trace message. + * + * @param message Trace message + */ + virtual void Trace(const std::string& message) = 0; + + /** + * @brief Log a trace message with class, method, and arguments information. + * + * @param className Name of the class + * @param methodName Name of the method + * @param args Arguments passed to the method (as formatted string) + * @param message Optional additional message + */ + virtual void Trace(const std::string& className, const std::string& methodName, const std::string& args = "", const std::string& message = "") = 0; + + /** + * @brief Log a debug message. + * + * @param message Debug message + */ + virtual void Debug(const std::string& message) = 0; + + /** + * @brief Log an info message. + * + * @param message Info message + */ + virtual void Info(const std::string& message) = 0; + + /** + * @brief Log a warning message. + * + * @param message Warning message + */ + virtual void Warn(const std::string& message) = 0; + + /** + * @brief Log an error message. + * + * @param message Error message + */ + virtual void Error(const std::string& message) = 0; + + /** + * @brief Log function entry. + * + * @param funcName Function name + */ + virtual void TraceFunction(const std::string& funcName) = 0; + + /** + * @brief Trace a variable value. + * + * @param name Variable name + * @param value Variable value + */ + virtual void TraceVariable(const std::string& name, const std::string& value) = 0; + virtual void TraceVariable(const std::string& name, int value) = 0; + virtual void TraceVariable(const std::string& name, size_t value) = 0; + virtual void TraceVariable(const std::string& name, bool value) = 0; + virtual void TraceVariable(const std::string& name, float value) = 0; + virtual void TraceVariable(const std::string& name, double value) = 0; +}; + +} // namespace sdl3cpp::services \ No newline at end of file