CleverRaven/Cataclysm-DDA — pull request #8250 by BevapDin (2014-07-16 14:03:57+00:00)
Title: Debug system [CR]

Lets see if this fixes issue #2993:

> Here's a list:
> - debugmsg(msg, ...) : alias of realDebugmsg(filename, line, msg, ...) .

Documented that this prints to the screen, waits for specific user input and should therefor be used rarely.

> - realDebugmsg(filename, line, msg, ...) : prints the message directly to the screen, pauses the game, opens debug.log , writes the message and closes it. Allows printf formatting.

I made clear in documentation that `realDebugmsg` should not be used (use `debugmsg` instead).

> - DebugLog() << msg : opens logg.txt , writes the message to it and closes it.

Both arguments are mandatory now, it's rarely used directly, but debug.h has a documentation about it.

> - dbg(level) << msg : alias of dout(level, class) << filename << line << msg .

Documented the usage of this macro.

> - dout(level, class) << msg writes msg to data/debug.txt , including timestamp, level (error/warning/info), and class (map/npc/game/map_gen). Disabled at compile by default. I suspect that the timestamps are not portable.

I rename `dout` to `DebugLog`, which is not really a change because `DebugLog` was already a macro that resolves to `dout` anyway. Using either is without difference. And that is of course pointless and at best confusing.

> - throw (std::string)msg : caught during game init and passed to debugmsg before exiting.

Not part of any debugging system. It is simply language construct used to report failures. But it should throw something derived from `std::exception`, certainly not a `std::string`.

----

Nearly all SDL functions are now checked for errors and those errors are properly logged, included the output of `SDL_GetError`.

I added a comment block at the top of debug.h explaining the debugging system. Please read this and let me know if I missed some aspect, or something is still unclear.

In short, there is only one function that writes the log file: `DebugLog`. It returns either a null-stream (that discards all input) or a stream to the log file. The user of this function does not see a difference, they only see a `std::ostream`.

`debugmsg` is a wrapper, it forwards the formatted message to `DebugLog` (so it gets written into the log file) and it displays the message on screen. It does *not* write to its own log file anymore. Using a stream instead of printf format allows all types to be written directly (they only need a operator<<). This works for example with `std::string` directly without using the `c_str()` function. Also works for `std::vector`.

`dbg` is another wrapper that must be declared inside a source file. It simplifies the usage of `DebugLog` further by adding the source file and line number to the debug message and by providing a default debug class.

----

> I'd say add a severity flag and a timestamp (when logging) to debugmsg and
convert everything to debugmsg calls. Existing debugmsg and equivalent
would get the "screen" flag (nonfatal error that should never happen, so we
want users to report it), debugLog calls would get a "log" flag (something
weird and nonfatal happened that might be useful for debugging, disabled by
default, possibly toggled to be displayed on screen when debug mode is on).

Severity flag is already implemented as debug level, through that could be more detailed.

Timestamp is automatically added by `DebugLog` and all debug logging functions call it.

All calls to `debugmsg` can be seen as having the screen-flag, all calls to `DebugLog` (including `debugmsg` and `dbg`) can be seen as having the "log" flag.

----

There is another debugging part not listed in the issue: `game::debugmon`.

It is usually used to enable writing debug messages with either `debugmsg` or `add_msg`. I suggest the following change for that:

Add a function `add_debug_msg` that behaves like a normal `add_msg` (with a specific color perhaps?) but only writes the message if `game::debugmon` is true.

----

There is the remaining issue how to enabled/disable the debug level/debug class. Both are basically bitmasks.

I have changed it to always include debug logging in the build (ignores `ENABLE_LOGGING`). What good is it to call error logging functions (and `DebugLog(D_ERROR)` is for *error*) when they never get reported.