Engine logging API

[philsegeler]

We need a unified API for logging with proper subsystem handling, coloring and priorities. I am tired of using std::cout or OE_WriteToLog().

There are multiple ideas. I have the following proposal:

It should be as simple as possible. If one does not wish any fancy features then a

oe::log("whatever text i want to log");

should always work fine from the user perspective. It should also not need oe::init() to be run before usage.

Idea 1: Separate APIs for each subsystem

oe::log(...); // for the user
nre::log(...); // for the renderer
oe::internal_log(...); // for the various other engine parts, 'internal_' as a prefix
csl::log(...); // for the parser/interpreter
hpe::log(...); // for the hydrogen physics engine, when it finally appears :)
//etc.

Idea 2: Unified API with subsystem as enum optional final argument

It would default to no subsystem, meaning from the user

namespace oe{
    enum logger_subsystem{
        LOGGER_CSL
        LOGGER_NRE,
        LOGGER_INTERNAL,
        LOGGER_HPE,
        LOGGER_USER // the default
    };
};

oe::log(..., oe::LOGGER_NRE);

Backend: Separate classes for each subsystem or the same global class, but accessed through a different API?

Separate classes may bring independence for each subsystem, but we may need to duplicate everything a few times.

Alternatively we could use the same singleton class, but internally hardcode for example csl::log to execute oe::internal_log(...,oe::LOGGER_CSL); for convenience with only slightly more code.

It was suggested as an idea to use fmtlib for formatting and as a backend. I find this a good idea, since this is a header-only library. I see that it might also be useful for the interpreter due to fast conversions? It seems cross-platform enough, since it only requires C++11.

General

We could use oe::log(...), oe::warn_msg(...), oe::error_msg(...) to prevent an extra enum argument for severity.

Automatic newline at the end?

The initial argument list I propose is as follows:

template<typename T>
oe::log(T, int priority); // two extra enums could be possible for subsystem and severity, if we want something completely unified

T could be directly passed to fmtlib.

Priority should only be valid for normal info messages and not warnings or errors.

Configuration options

Following logger outputs:

namespace oe{
    enum logger_output{
        FILE_LOGGER_IO,
        STANDARD_LOGGER_IO, // we could seamlessly translate this to ADB for android or UART for embedded or any other output.
        CONSOLE_LOGGER_IO, // we could make an internal console as soon as i integrate 'SDL_ttf' and implement font rendering
        ALL_LOGGER_IO
    };
};

We can show/hide subsystems completely

oe::hide_logger_subsystem(oe::LOGGER_NRE);

or

nre::hide_logger_subsystem();

Respectively for:

oe::set_logfile(...); // some reasonable default
oe::set_min_priority(enum oe::logger_output, int some_num); // anything below 'some_num' should be hidden except warnings/errors/fatal
oe::show/hide_warnings(enum oe::logger_output);
oe::show/hide_errors(enum oe::logger_output); // should not hide warnings
oe::use_colors(bool, enum oe::logger_output); // only valid for std and console output
oe::show/hide_internals(enum oe::logger_output); // for the user to only see their own feedback

oe::show/hide_logger_output(enum oe::logger_output); // show/hide logger output completely
// these are  only for the internal console, so that the renderer can read all the logging and render it
// these cannot be implemented right now, but i find these a good idea for the future
oe::set_console_logger_font(std::string path_to_font);
oe::set_console_logger_linewrap(int characters); // 0 should disable line wrap
oe::set_console_logger_linecount(int num); // 200 lines by default?

oe::get_current_logger_output(); // output already formatted for line wrap

Note that everything should be configurable on runtime. ;)

Defaults

Should all internal info messages be hidden by default? Should colors be on by default? Should we output both to a file and std by default? Should we have different defaults for each output? How should the default log file be named?

[antsouchlos]

Something else we could think about is incorporating std::source_location now that we are moving to C++20 (#29) :P

[antsouchlos]

Logger architecture

I think internally we should build the logging system from multiple abstraction layers on top of each other.

For starters, I think a single singleton class used for all logging would be a good idea. On top of this we could then create a system handling logging capabilities for the different subsystems, also responsible for configuration:

namespace oe { namespace detail {

    // Singleton class: Backend for all logging
    class logger_t {
    public:
        // Use fmtlib syntax and simply forward str and args to fmtlib
        template<typename... Args>
        void log(const char* str, Args... args) { /* ... */ }
    private:
    };

}} // namespace oe::detail

Option 1: Single class handling all subsystems

namespace oe { namespace detail {

    // Singleton class: Logging backend
    class logger_t {
        /* ... */
    };

    enum class logger_subsystem { oe, nre, csl, hpe };

    // Singleton class: Logging frontend
    class logger_subsystem_manager_t {
    public:
        template<typename... Args>
        void log(logger_subsystem subsystem, const char* str, Args... args) { /* ... */ }
    };

}} // namespace oe::detail

// Usage
auto& log_mgr = logger_subsystem_manager_t::get_instance();
log_mgr.log(logger_subsystem::oe, "Some numbers: {} {} {}", 3, 1, 4);

Option 2: A separate class for each subsystem

To not be continued apparently :D

Other

We should also think about:

• adding the option to log messages with different colors
• creating a nice interface for selecting if the logs go to stdout, or to a file, or both

[philsegeler]

Logger architecture

I agree here.

Option 1

I prefer this option as the internal structure. Is there a particular reason why you used enum class instead of plain enum? I would prefer plain enum since it is more C-like.

Other

1. I do not know how fmtlib handles colors, you tell me :P
2. We could use this:

oe::log(...); // goes to everything by default
oe::log2file(...);
oe::log2std(...);
oe::log2console(...);

Warnings and errors I believe should still go to all log outputs by default, unless one disables this explicitly.

Perhaps we could do this like this?

template<typename... Args>
oe::log(const char* str, Args... args); // by default goes to all outputs
oe::log(int priority, const char* str, Args... args); // by default to all outputs

Do you prefer Idea 1 or Idea 2 as a user-facing API? I now mostly prefer Idea 1. Or do you have another idea?

source_location is interesting and I agree we should definitely use it!

[antsouchlos]

Enum class

I used enum class, since it limits the scope of the enum values.

With a C-like enum you have to prepend a prefix and the enum values are global:

enum logger_subsystem { LOGGER_OE, LOGGER_NRE, LOGGER_CSL, LOGGER_HPE };

// Usage
logger_subsystem ls = LOGGER_NRE;

With enum class you limit the scope to the given enum and don't pollute the global namespace:

enum class logger_subsystem { oe, nre, csl, hpe };

// Usage
logger_subsystem ls = logger_subsystem::oe;

Kind of similar reasoning to putting everything in namespaces instead of prepending OE_, NRE_, etc. to every single function and class.

Printing in color

As far as I've seen, fmtlib only really supports formatting in color when using the fmt::print function, not fmt::format. I think we would probably only want to use fmtlib to format our log messages, not print them. Using fmtlib to print them might complicate things a bit when deciding whether to print to file, stdout, etc.

That would mean that we have to implement our own color printing system :P (Only applicable to printing to console of course, since printing to file in color isn't really possible)

Preference among the Options

I would also prefer a unified API, although we might have to think about where we put it. If it is inside the oe namespace for example it might be difficult to access from other namespaces. Maybe create a new namespace? Something like pl for Phosphorus Logger? :D

[philsegeler]

Enum class

Hmm ok, i guess this makes sense, but we do not pollute the global namespace only the oe one, if we use C-like enums. On the other hand, enum class looks nicer. I am OK with using enum class for the logger, but I would still prefer using plain enum myself, since i can easily convert them to integers.

Printing in color

I propose we only have different predefined colors for now which we can enable/disable. I think it would not be worth it trying to recreate the fmt::print API ourselves :D

Preference among the Options

I am fine with pl for phosphorus logger internals, but the APIs should still be in oe and nre etc.. This way oe etc. can then use pl internally.