refactored logging and removed magic numbers

src/common/AudioQueue.cxx

@@ -29,7 +29,7 @@ AudioQueue::AudioQueue(uInt32 fragmentSize, uInt32 capacity, bool isStereo)
     mySize(0),
     myNextFragment(0),
     myIgnoreOverflows(true),
-    myOverflowLogger("audio buffer overflow", 1)
+    myOverflowLogger("audio buffer overflow", Logger::Level::INFO)
 {
   const uInt8 sampleSize = myIsStereo ? 2 : 1;
 

src/common/EventHandlerSDL2.cxx

@@ -32,9 +32,9 @@ EventHandlerSDL2::EventHandlerSDL2(OSystem& osystem)
   {
     ostringstream buf;
     buf << "ERROR: Couldn't initialize SDL joystick support: " << SDL_GetError() << endl;
-    Logger::log(buf.str(), 0);
+    Logger::error(buf.str());
   }
-  Logger::log("EventHandlerSDL2::EventHandlerSDL2 SDL_INIT_JOYSTICK", 2);
+  Logger::debug("EventHandlerSDL2::EventHandlerSDL2 SDL_INIT_JOYSTICK");
 #endif
 }
 

src/common/FrameBufferSDL2.cxx

@@ -41,10 +41,10 @@ FrameBufferSDL2::FrameBufferSDL2(OSystem& osystem)
   {
     ostringstream buf;
     buf << "ERROR: Couldn't initialize SDL: " << SDL_GetError() << endl;
-    Logger::log(buf.str(), 0);
+    Logger::error(buf.str());
     throw runtime_error("FATAL ERROR");
   }
-  Logger::log("FrameBufferSDL2::FrameBufferSDL2 SDL_Init()", 2);
+  Logger::debug("FrameBufferSDL2::FrameBufferSDL2 SDL_Init()");
 
   // We need a pixel format for palette value calculations
   // It's done this way (vs directly accessing a FBSurfaceSDL2 object)
@@ -287,7 +287,7 @@ bool FrameBufferSDL2::setVideoMode(const string& title, const VideoMode& mode)
     if(myWindow == nullptr)
     {
       string msg = "ERROR: Unable to open SDL window: " + string(SDL_GetError());
-      Logger::log(msg, 0);
+      Logger::error(msg);
       return false;
     }
     setWindowIcon();
@@ -303,7 +303,7 @@ bool FrameBufferSDL2::setVideoMode(const string& title, const VideoMode& mode)
   if(myRenderer == nullptr)
   {
     string msg = "ERROR: Unable to create SDL renderer: " + string(SDL_GetError());
-    Logger::log(msg, 0);
+    Logger::error(msg);
     return false;
   }
   clear();

src/common/Logger.cxx

@@ -26,13 +26,30 @@ Logger& Logger::instance()
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void Logger::log(const string& message, uInt8 level)
+void Logger::log(const string& message, Level level)
 {
   instance().logMessage(message, level);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void Logger::logMessage(const string& message, uInt8 level) const
+void Logger::error(const string& message)
+{
+  instance().logMessage(message, Level::ERR);
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void Logger::info(const string& message)
+{
+  instance().logMessage(message, Level::INFO);
+}
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void Logger::debug(const string& message)
+{
+  instance().logMessage(message, Level::DEBUG);
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void Logger::logMessage(const string& message, Level level) const
 {
   if (myLogCallback)
     myLogCallback(message, level);

src/common/Logger.hxx

@@ -23,17 +23,30 @@
 #include "bspf.hxx"
 
 class Logger {
+
   public:
 
-    using logCallback = std::function<void(const string&, uInt8)>;
+    enum class Level {
+      ERR = 0, // cannot use ERROR???
+      INFO = 1,
+      DEBUG = 2,
+      MIN = ERR,
+      MAX = DEBUG
+    };
+
+    using logCallback = std::function<void(const string&, Logger::Level)>;
 
   public:
 
     static Logger& instance();
 
-    static void log(const string& message, uInt8 level);
+    static void log(const string& message, Level level);
 
-    void logMessage(const string& message, uInt8 level) const;
+    static void error(const string& message);
+
+    static void info(const string& message);
+
+    static void debug(const string& message);
 
     void setLogCallback(logCallback callback);
 
@@ -49,6 +62,8 @@ class Logger {
 
   private:
 
+    void logMessage(const string& message, Level level) const;
+
     Logger(const Logger&) = delete;
     Logger(Logger&&) = delete;
     Logger& operator=(const Logger&) = delete;

src/common/PJoystickHandler.cxx

@@ -147,7 +147,7 @@ bool PhysicalJoystickHandler::remove(int id)
       ostringstream buf;
       buf << "Removed joystick " << mySticks[id]->ID << ":" << endl
           << "  " << mySticks[id]->about() << endl;
-      Logger::log(buf.str(), 1);
+      Logger::info(buf.str());
 
       // Remove joystick, but remember mapping
       it->second.mapping = stick->getMap();

src/common/SoundSDL2.cxx

@@ -52,14 +52,14 @@ SoundSDL2::SoundSDL2(OSystem& osystem, AudioSettings& audioSettings)
 {
   ASSERT_MAIN_THREAD;
 
-  Logger::log("SoundSDL2::SoundSDL2 started ...", 2);
+  Logger::debug("SoundSDL2::SoundSDL2 started ...");
 
   if (SDL_InitSubSystem(SDL_INIT_AUDIO) < 0) {
     ostringstream buf;
 
     buf << "WARNING: Failed to initialize SDL audio system! " << endl
         << "         " << SDL_GetError() << endl;
-    Logger::log(buf.str(), 0);
+    Logger::error(buf.str());
     return;
   }
 
@@ -69,7 +69,7 @@ SoundSDL2::SoundSDL2(OSystem& osystem, AudioSettings& audioSettings)
 
   mute(true);
 
-  Logger::log("SoundSDL2::SoundSDL2 initialized", 2);
+  Logger::debug("SoundSDL2::SoundSDL2 initialized");
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -107,7 +107,7 @@ bool SoundSDL2::openDevice()
 
     buf << "WARNING: Couldn't open SDL audio device! " << endl
         << "         " << SDL_GetError() << endl;
-    Logger::log(buf.str(), 0);
+    Logger::error(buf.str());
 
     return myIsInitializedFlag = false;
   }
@@ -120,8 +120,8 @@ void SoundSDL2::setEnabled(bool state)
   myAudioSettings.setEnabled(state);
   if (myAudioQueue) myAudioQueue->ignoreOverflows(!state);
 
-  Logger::log(state ? "SoundSDL2::setEnabled(true)" :
+  Logger::debug(state ? "SoundSDL2::setEnabled(true)" :
-                               "SoundSDL2::setEnabled(false)", 2);
+                "SoundSDL2::setEnabled(false)");
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -138,13 +138,13 @@ void SoundSDL2::open(shared_ptr<AudioQueue> audioQueue,
 
   myEmulationTiming = emulationTiming;
 
-  Logger::log("SoundSDL2::open started ...", 2);
+  Logger::debug("SoundSDL2::open started ...");
   mute(true);
 
   audioQueue->ignoreOverflows(!myAudioSettings.enabled());
   if(!myAudioSettings.enabled())
   {
-    Logger::log("Sound disabled\n", 1);
+    Logger::info("Sound disabled\n");
     return;
   }
 
@@ -160,12 +160,12 @@ void SoundSDL2::open(shared_ptr<AudioQueue> audioQueue,
   // Show some info
   myAboutString = about();
   if(myAboutString != pre_about)
-    Logger::log(myAboutString, 1);
+    Logger::info(myAboutString);
 
   // And start the SDL sound subsystem ...
   mute(false);
 
-  Logger::log("SoundSDL2::open finished", 2);
+  Logger::debug("SoundSDL2::open finished");
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

src/common/StaggeredLogger.cxx

@@ -36,7 +36,7 @@ namespace {
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-StaggeredLogger::StaggeredLogger(const string& message, uInt8 level)
+StaggeredLogger::StaggeredLogger(const string& message, Logger::Level level)
   : myMessage(message),
     myLevel(level),
     myCurrentEventCount(0),

src/common/StaggeredLogger.hxx

@@ -25,6 +25,7 @@
 
 #include "bspf.hxx"
 #include "TimerManager.hxx"
+#include "Logger.hxx"
 /**
  * This class buffers log events and logs them after a certain time window has expired.
  * The timout increases after every log line by a factor of two until a maximum is reached.
@@ -34,7 +35,7 @@
 class StaggeredLogger {
   public:
 
-    StaggeredLogger(const string& message, uInt8 level);
+    StaggeredLogger(const string& message, Logger::Level level);
 
     ~StaggeredLogger();
 
@@ -55,7 +56,7 @@ class StaggeredLogger {
     void logLine();
 
     string myMessage;
-    uInt8 myLevel;
+    Logger::Level myLevel;
 
     uInt32 myCurrentEventCount;
     bool myIsCurrentlyCollecting;

src/common/audio/Resampler.hxx

@@ -54,7 +54,7 @@ class Resampler {
       myFormatFrom(formatFrom),
       myFormatTo(formatTo),
       myNextFragmentCallback(nextFragmentCallback),
-      myUnderrunLogger("audio buffer underrun", 1)
+      myUnderrunLogger("audio buffer underrun", Logger::Level::INFO)
     {}
 
     virtual void fillFragment(float* fragment, uInt32 length) = 0;

src/common/main.cxx

@@ -74,7 +74,6 @@ void checkForCustomBaseDir(Settings::Options& options);
 */
 bool isProfilingRun(int ac, char* av[]);
 
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void parseCommandLine(int ac, char* av[],
     Settings::Options& globalOpts, Settings::Options& localOpts)
@@ -176,7 +175,7 @@ int main(int ac, char* av[])
   auto Cleanup = [&theOSystem]() {
     if(theOSystem)
     {
-      Logger::log("Cleanup from main", 2);
+      Logger::debug("Cleanup from main");
       theOSystem->saveConfig();
       theOSystem.reset();     // Force delete of object
     }
@@ -200,10 +199,10 @@ int main(int ac, char* av[])
 
   // Create the full OSystem after the settings, since settings are
   // probably needed for defaults
-  Logger::log("Creating the OSystem ...", 2);
+  Logger::debug("Creating the OSystem ...");
   if(!theOSystem->create())
   {
-    Logger::log("ERROR: Couldn't create OSystem", 0);
+    Logger::error("ERROR: Couldn't create OSystem");
     return Cleanup();
   }
 
@@ -213,21 +212,21 @@ int main(int ac, char* av[])
   string romfile = localOpts["ROMFILE"].toString();
   if(localOpts["listrominfo"].toBool())
   {
-    Logger::log("Showing output from 'listrominfo' ...", 2);
+    Logger::debug("Showing output from 'listrominfo' ...");
     theOSystem->propSet().print();
     return Cleanup();
   }
   else if(localOpts["rominfo"].toBool())
   {
-    Logger::log("Showing output from 'rominfo' ...", 2);
+    Logger::debug("Showing output from 'rominfo' ...");
     FilesystemNode romnode(romfile);
-    Logger::log(theOSystem->getROMInfo(romnode), 0);
+    Logger::error(theOSystem->getROMInfo(romnode));
 
     return Cleanup();
   }
   else if(localOpts["help"].toBool())
   {
-    Logger::log("Displaying usage", 2);
+    Logger::debug("Displaying usage");
     theOSystem->settings().usage();
     return Cleanup();
   }
@@ -242,12 +241,12 @@ int main(int ac, char* av[])
   FilesystemNode romnode(romfile);
   if(romfile == "" || romnode.isDirectory())
   {
-    Logger::log("Attempting to use ROM launcher ...", 2);
+    Logger::debug("Attempting to use ROM launcher ...");
     bool launcherOpened = romfile != "" ?
       theOSystem->createLauncher(romnode.getPath()) : theOSystem->createLauncher();
     if(!launcherOpened)
     {
-      Logger::log("Launcher could not be started, showing usage", 2);
+      Logger::debug("Launcher could not be started, showing usage");
       theOSystem->settings().usage();
       return Cleanup();
     }
@@ -273,7 +272,7 @@ int main(int ac, char* av[])
     }
     catch(const runtime_error& e)
     {
-      Logger::log(e.what(), 0);
+      Logger::error(e.what());
       return Cleanup();
     }
 
@@ -289,9 +288,9 @@ int main(int ac, char* av[])
   }
 
   // Start the main loop, and don't exit until the user issues a QUIT command
-  Logger::log("Starting main loop ...", 2);
+  Logger::debug("Starting main loop ...");
   theOSystem->mainLoop();
-  Logger::log("Finished main loop ...", 2);
+  Logger::debug("Finished main loop ...");
 
   // Cleanup time ...
   return Cleanup();

src/common/repository/KeyValueRepositoryConfigfile.cxx

@@ -42,7 +42,7 @@ std::map<string, Variant> KeyValueRepositoryConfigfile::load()
 
   ifstream in(myFilename);
   if(!in || !in.is_open()) {
-    Logger::log("ERROR: Couldn't load from settings file " + myFilename, 1);
+    Logger::error("ERROR: Couldn't load from settings file " + myFilename);
 
     return values;
   }
@@ -80,7 +80,7 @@ void KeyValueRepositoryConfigfile::save(const std::map<string, Variant>& values)
 {
 ofstream out(myFilename);
   if(!out || !out.is_open()) {
-    Logger::log("ERROR: Couldn't save to settings file " + myFilename, 1);
+    Logger::error("ERROR: Couldn't save to settings file " + myFilename);
 
     return;
   }

src/emucore/EventHandler.cxx

@@ -160,7 +160,7 @@ void EventHandler::addPhysicalJoystick(PhysicalJoystickPtr joy)
   ostringstream buf;
   buf << "Added joystick " << ID << ":" << endl
       << "  " << joy->about() << endl;
-  Logger::log(buf.str(), 1);
+  Logger::info(buf.str());
 #endif
 }
 

src/emucore/FrameBuffer.cxx

@@ -251,7 +251,7 @@ FBInitStatus FrameBuffer::createDisplay(const string& title,
     }
     else
     {
-      Logger::log("ERROR: Couldn't initialize video subsystem", 0);
+      Logger::error("ERROR: Couldn't initialize video subsystem");
       return FBInitStatus::FailNotSupported;
     }
   }
@@ -283,13 +283,13 @@ FBInitStatus FrameBuffer::createDisplay(const string& title,
   // Print initial usage message, but only print it later if the status has changed
   if(myInitializedCount == 1)
   {
-    Logger::log(about(), 1);
+    Logger::info(about());
   }
   else
   {
     string post_about = about();
     if(post_about != pre_about)
-      Logger::log(post_about, 1);
+      Logger::info(post_about);
   }
 
   return FBInitStatus::Success;

src/emucore/OSystem.cxx

@@ -139,7 +139,7 @@ bool OSystem::create()
   buf
       << "User game properties: '"
       << FilesystemNode(myPropertiesFile).getShortPath() << "'" << endl;
-  Logger::log(buf.str(), 1);
+  Logger::info(buf.str());
 
   // NOTE: The framebuffer MUST be created before any other object!!!
   // Get relevant information about the video hardware
@@ -222,7 +222,7 @@ void OSystem::loadConfig(const Settings::Options& options)
 
   mySettings->setRepository(createSettingsRepository());
 
-  Logger::log("Loading config options ...", 2);
+  Logger::debug("Loading config options ...");
   mySettings->load(options);
   mySettingsLoaded = true;
 
@@ -240,15 +240,15 @@ void OSystem::saveConfig()
   // Ask all subsystems to save their settings
   if(myFrameBuffer)
   {
-    Logger::log("Saving TV effects options ...", 2);
+    Logger::debug("Saving TV effects options ...");
     myFrameBuffer->tiaSurface().ntsc().saveConfig(settings());
   }
 
-  Logger::log("Saving config options ...", 2);
+  Logger::debug("Saving config options ...");
   mySettings->save();
 
   if(myPropSet && myPropSet->save(myPropertiesFile))
-    Logger::log("Saving properties set ...", 2);
+    Logger::debug("Saving properties set ...");
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -337,7 +337,7 @@ FBInitStatus OSystem::createFrameBuffer()
 
     case EventHandlerState::NONE:  // Should never happen
     default:
-      Logger::log("ERROR: Unknown emulation state in createFrameBuffer()", 0);
+      Logger::error("ERROR: Unknown emulation state in createFrameBuffer()");
       break;
   }
   return fbstatus;
@@ -385,7 +385,7 @@ string OSystem::createConsole(const FilesystemNode& rom, const string& md5sum,
   catch(const runtime_error& e)
   {
     buf << "ERROR: Couldn't create console (" << e.what() << ")";
-    Logger::log(buf.str(), 0);
+    Logger::error(buf.str());
     return buf.str();
   }
 
@@ -403,7 +403,7 @@ string OSystem::createConsole(const FilesystemNode& rom, const string& md5sum,
     myEventHandler->setMouseControllerMode(mySettings->getString("usemouse"));
     if(createFrameBuffer() != FBInitStatus::Success)  // Takes care of initializeVideo()
     {
-      Logger::log("ERROR: Couldn't create framebuffer for console", 0);
+      Logger::error("ERROR: Couldn't create framebuffer for console");
       myEventHandler->reset(EventHandlerState::LAUNCHER);
       return "ERROR: Couldn't create framebuffer for console";
     }
@@ -421,7 +421,7 @@ string OSystem::createConsole(const FilesystemNode& rom, const string& md5sum,
     buf << "Game console created:" << endl
         << "  ROM file: " << myRomFile.getShortPath() << endl << endl
         << getROMInfo(*myConsole);
-    Logger::log(buf.str(), 1);
+    Logger::info(buf.str());
 
     myFrameBuffer->setCursorState();
 
@@ -466,7 +466,7 @@ bool OSystem::createLauncher(const string& startdir)
     status = true;
   }
   else
-    Logger::log("ERROR: Couldn't create launcher", 0);
+    Logger::error("ERROR: Couldn't create launcher");
 #endif
 
   myLauncherUsed = myLauncherUsed || status;
@@ -493,14 +493,14 @@ string OSystem::getROMInfo(const FilesystemNode& romfile)
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void OSystem::logMessage(const string& message, uInt8 level)
+void OSystem::logMessage(const string& message, Logger::Level level)
 {
-  if(level == 0)
+  if(level == Logger::Level::ERR)
   {
     cout << message << endl << std::flush;
     myLogMessages += message + "\n";
   }
-  else if(level <= uInt8(mySettings->getInt("loglevel")) || !mySettingsLoaded)
+  else if(int(level) <= uInt8(mySettings->getInt("loglevel")) || !mySettingsLoaded)
   {
     if(mySettings->getBool("logtoconsole"))
       cout << message << endl << std::flush;

src/emucore/OSystem.hxx

@@ -56,6 +56,7 @@ class AudioSettings;
 #include "EventHandlerConstants.hxx"
 #include "FpsMeter.hxx"
 #include "Settings.hxx"
+#include "Logger.hxx"
 #include "bspf.hxx"
 #include "repository/KeyValueRepository.hxx"
 
@@ -438,7 +439,7 @@ class OSystem
       @param level    If 0, always output the message, only append when
                       level is less than or equal to that in 'loglevel'
     */
-    void logMessage(const string& message, uInt8 level);
+    void logMessage(const string& message, Logger::Level level);
 
     //////////////////////////////////////////////////////////////////////
     // The following methods are system-specific and *must* be

src/emucore/Settings.cxx

@@ -19,6 +19,7 @@
 
 #include "OSystem.hxx"
 #include "Version.hxx"
+#include "Logger.hxx"
 #include "AudioSettings.hxx"
 
 #ifdef DEBUGGER_SUPPORT
@@ -142,7 +143,7 @@ Settings::Settings()
   setPermanent("dialogpos", 0);
 
   // Misc options
-  setPermanent("loglevel", "1");
+  setPermanent("loglevel", int(Logger::Level::INFO));
   setPermanent("logtoconsole", "0");
   setPermanent("avoxport", "");
   setPermanent("fastscbios", "true");
@@ -349,8 +350,8 @@ void Settings::validate()
   else if(i > 2)  setValue("romviewer", "2");
 
   i = getInt("loglevel");
-  if(i < 0 || i > 2)
+  if(i < int(Logger::Level::MIN) || i > int(Logger::Level::MAX))
-    setValue("loglevel", "1");
+    setValue("loglevel", int(Logger::Level::INFO));
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

src/gui/LoggerDialog.cxx

@@ -27,6 +27,7 @@
 #include "StringParser.hxx"
 #include "Widget.hxx"
 #include "Font.hxx"
+#include "Logger.hxx"
 #include "LoggerDialog.hxx"
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -57,9 +58,9 @@ LoggerDialog::LoggerDialog(OSystem& osystem, DialogContainer& parent,
 
   // Level of logging (how much info to print)
   VariantList items;
-  VarList::push_back(items, "None", "0");
+  VarList::push_back(items, "None", int(Logger::Level::ERR));
-  VarList::push_back(items, "Basic", "1");
+  VarList::push_back(items, "Basic", int(Logger::Level::INFO));
-  VarList::push_back(items, "Verbose", "2");
+  VarList::push_back(items, "Verbose", int(Logger::Level::DEBUG));
   myLogLevel =
     new PopUpWidget(this, font, xpos, ypos, font.getStringWidth("Verbose"),
                     lineHeight, items, "Log level ",
@@ -90,7 +91,7 @@ void LoggerDialog::loadConfig()
   myLogInfo->setSelected(0);
   myLogInfo->scrollToEnd();
 
-  myLogLevel->setSelected(instance().settings().getString("loglevel"), "1");
+  myLogLevel->setSelected(instance().settings().getString("loglevel"), int(Logger::Level::INFO);
   myLogToConsole->setState(instance().settings().getBool("logtoconsole"));
 }