Logging interface

[viacheslavka]

It would be nice if there was some interface that allowed the engine and the drivers to inform the user about various errors and perhaps could be used for debug info during development. Ideally the user application should be able to intercept these log messages, so that they could be shown in a PGE console or something like that.

We could get away with passing an ostream reference to the engine and then writing to it, but that complicates intercepting messages, as it makes it hard to know where a particular message ends. For this reason, I think it would be better to follow the printf/std::format approach.

I propose to add an interface called LogListener with just one member function:

void LogListener::OnMessage(int level, const std::string_view& msg);

The level argument would represent different log levels (error, info, debug, etc), and the msg would be the final message that the user receives. The default implementation would just dump the message to std::cout or std:cerr based on the level.

Since performance may be critical here, it should be possible to prevent unimportant messages from being considered at all. This will have to be done with macros, since I don't think there's any other way to prevent arguments from being evaluated.

Right now, I think that logging should look like this:

// in drivers
SWE_DRV_LOG_ERROR("Failed to initialize the backend: blah blah blah");
// in the other parts of the engine
SWE_ENG_LOG_ERROR("Oh no, the driver crashed");

The necessity of different macro names for different parts of the engine is explained by the need to be able to find the engine's pointer to the LogListener implementation. It will most probably be stored in the WaveEngine class, so the drivers will need to refer to their pHost member, while the engine itself could use this.

I suppose we could just add a "get log listener" function everywhere that would return the currently used listener, but I'm not sure if that's a good way to approach this.

The macro arguments could be passed to snprintf or std::format before being sent to the listener, depending on whether we want to use C++20 features or not.

I'm really interested to hear your opinions. If you've got some ideas, please share.

[Moros1138]

First, I love the idea of meaningful error/debug messaging to the user. I have no real opinion about how a feature like this would or should be implemented.

I do have one thing and it's because C++20 features were mentioned. I know SWE is a standalone, you can choose to use it with whatever you want to use it with but let's be honest, it's going to be used with PGE applications, at least in the early days of it's existence, almost exclusively. I think this should be a consideration because PGE's minimum requirement is stated as being C++17.

I'm just imagining an influx of PGE users getting errors and asking the same question about it, over and over, related to having their projects set to use C++17 rather than C++20.

It's easier on us and the users to just avoid that before it becomes the issue I'm imagining.

UNLESS, there's functionality C++20 gives us, that we intend to use, that is of substantial impact to the end result, I would suggest remaining with C++17.

[viacheslavka]

This is a little tricky since it's a problem of forward-compatibility. snprintf and std::format use different format strings, so once we choose one, there's hardly any easy way back.

Personally, I don't mind sticking to snprintf for the foreseeable future. There will probably be some other breaking change in SWE at some point anyway. I'm sure we'll be able to piggyback this if needed.

[Moros1138]

I feel like pointing out something that may change our mind about std::format for now..

Currently std::format is flat out not supported in GCC and MacOS Clang.. Clang support is disabled by default. So requiring the use of std::format will put, in my opinion, an undue burden on the average end user.

Source of Image: cppreference.com

In my search I've seen reference to the text formatting library which std::format is based upon, which we could use, but then we're introducing a dependency.

[j-lohuis]

I also like the idea of providing some meaningful error/debug messages. As already pointed out, there is the issue with formatting and C++ standards. I think we should stick to the same standard as the PGE does as the SWE will probably be used in conjunction with it. Additionally, essentially every platform that the PGE, and therefore SWE, targets supports C++17.

I have the following suggestion which would make formatting a non-issue for the user:

Pretty much all functions return a bool on whether they've succeeded or failed iirc. So why don't we have a function like GetError() (or something similar) which returns a meaningful error message as a std::string or const char*. Then, we could format the error messages ourselves internally using whichever functions are available. The error message string would just be a buffer in the engine, which is overridden whenever new error messages are generated. The user is then easily able to decide where the error messages get printed.

Example:

bool OnUserCreate() override {
    if (!engine.InitialiseAudio()) {
        std::cerr << "SWE: " << engine.GetError() << std::endl;
        return false;
    }
// ...
}

This is also the method SDL uses.

[viacheslavka]

Formatting is already not a user issue in my proposal. The user receives a formatted string as the log message, and this formatting is done internally by SWE.

The main concern here is all the possible extensions SWE might have in the future. Right now it's not very extensible, but I think this will change in the future. And I believe that a similar system could be used in PGE, which already has a lot of third-party extensions, so I'm trying to plan for consistency between the two.

We could delegate formatting to the entity that emits the message, but I'm worried that it might lead to a lot of duplicated code, make logging a chore, or encourage third-party developers to introduce dependencies for this.

To be honest, I'm almost starting to think that we might want to look into developing our own string formatting solution after all. But that's also not a great idea since we'll have to throw that away once support for std::format becomes more widespread. It'd be nice to use https://github.com/fmtlib/fmt but I doubt that's realistic.

I think our realistic approaches are limited to these two options:

1. Use snprintf for formatting. This is good enough for most things but limits our ability to print our custom types.
2. Let log emitters do their own formatting. This will let us print pretty much anything but see above for a list of potential disadvantages

[Moros1138]

It'd be nice to use https://github.com/fmtlib/fmt but I doubt that's realistic.

I mean, we could ask Javid what he thinks about additional dependencies. Maybe he'll change his mind?

[OneLoneCoder]

Im with slavka on this. I like the idea of a logging solution, and its likely it will make its way into PGE3 as well. Don't lose sight of who the users are though - they are not us, and with all things OLC, things should try to fail "obviously". Audio is more complex than PGE however, and before I delve into device enumeration, a reasonable logging interface would be ideal. Im happy with a GetLatestErrors()-like approach, that maybe returns a vector of strings, or maybe {[INFO|WARN|FAIL], "message", "Line Details"} type.

[viacheslavka]

I'm not sure about the GetLatestErrors() approach because it either makes log messages missable, or requires us to store these messages for who knows how long. Also, it might mean having different functions for messages with different log levels.

Another little thing: my approach allows us to avoid making copies of strings that don't require any formatting, which might be beneficial. Hard to judge how that will actually affect the performance without any benchmarking, of course.