Add new thread-safe error logging API #3156
Conversation
ChrisThrasher
force-pushed
the
error_logging
branch
from
ec9c135 to
e26b13b
Compare
ChrisThrasher
force-pushed
the
error_logging
branch
from
e26b13b to
4aee858
Compare
| ((m_settings.attributeFlags & ContextSettings::Debug) != 0), | ||
| " ; sRGB = ", | ||
| m_settings.sRgbCapable, | ||
| std::noboolalpha); |
There was a problem hiding this comment.
This is one of the biggest downsides. clang-format is configured to put each function parameter on a separate line whereas chances of operator<< are more neatly packed into lines. This isn't a dealbreaker in my opinion but it is a bit of a bummer. If we really wanted to we could still make a local std::stringstream then pass that stream to log().
Pull Request Test Coverage Report for Build 9914129116Details
💛 - Coveralls |
|
Could you compare the pros/cons on this approach with #3010? That PR keeps the same interface and transparently adds thread-safety on top. This PR is still using streams under the hood, so we might as well keep using streams as the user-facing syntax until we switch to a format-based approach. The other benefit of #3010 is that it can be used to completely isolate the |
Description
Closes #3003
The thread safety itself is solved in a simple matter of locking a mutex each time the logging function is called. This ensures that logs are not interleaved.
On the API side of things, a new
sf::priv::logfunction is added which uses a bit of parameter pack magic to still let us use I/O stream formatters as a convenience. The only change to the public API is a new function that users can use to change the destination buffer for logs. This API is used in some new unit tests to confirm that everything works.This is a draft PR because if accepted, this PR needs to remove
sf::errentirely to finish the transformation to this new logging interface.