Some way to silence most of the output

Bug #1128024 reported by Hans Joachim Desserud
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
Won't Fix
Low
Unassigned
widelands (Debian)
Fix Released
Unknown

Bug Description

When running WL from the terminal it outputs a lot of information during the game. It might be useful for debugging issues, but a lot of it seems unnecessary for regular users, and could probably be silenced. Alternatively, we could split the messages into the usual levels (error, warning, info, debug) and only print errors+warnings unless more information is specified. This would keep the amount of output low for normal users but would give developers/debuggers access to all.

(As a side-note; the combat messages gives you information you may not have access to in the game, but I don't know if there is a way to gain an advantage of this except being able to tell that there is a fight going on somewhere on the map.)

This has been requested earlier (see the bug in Debian), but I was reminded of when I saw the amount of output we get with the new caching system in place. Regardless I think it is nice to forward the request upstream to get a final answer, rather than it being stuck in the Debian BTS.)

Related branches

Revision history for this message
Nicolai Hähnle (nha) wrote :

Those debug outputs should probably just be removed, or perhaps commented / #ifdef'd out in the code. In the case of the caching code, it could perhaps be reduced to some kind of warning if surfaces are evicted after less than X seconds.

At the end of the day, the debug output from random bug reports doesn't tend to be very useful. At least in the game logic, I always tend to just analyse what happens and then throw in very analysis-specific log output.

Changed in widelands (Debian):
status: Unknown → Confirmed
Revision history for this message
SirVer (sirver) wrote :

I agree with nicolai in general - the debug output is not very often useful in tracking down bugs. They are useful to make sure new feautures work as intended.

E.g. the current (very verbose) debug output for the surface cache is useful to me to check that the new feauture behaves the way I expect it too. After a few weeks of testing this I will remove the output again.

another very noisy part of widelands is the economy code. I think we can drop a good amount of debug output there as well.

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

#2: I see I failed to mention this in the original report, but I do of course see the value of additional debug messages related to major changes which has just landed. For instance the the cache information recently added is useful in case we need to track down bugs in it or notice any odd behaviour. I would assume the detailed combat output was added when that had just been implemented too. ;)

So just to clarify, this bug report is more about what to do with the output in the long term.

Revision history for this message
cghislai (charlyghislain) wrote :

Maybe we could pipe it to a log file by default, to not scare new users but still be able to access it for bug hunting.

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

(Marking confirmed based on general agreement in the comments)

Changed in widelands:
status: New → Confirmed
Revision history for this message
SirVer (sirver) wrote :

Setting to incomplete for bug sweeping.

Changed in widelands:
status: Confirmed → Incomplete
Revision history for this message
Hans Joachim Desserud (hjd) wrote :

Log file was discussed in bug 536522, but seems like we didn't see the need for it.

I think apart from warnings/errors which we naturally wish to keep, this is a possible strategy:
* New features will be introduced with logging, which is removed at a later stage when we have had a couple of months to verify it is working as expected.
* Old which is unnecessarily noisy can be cleaned up when nearby code is touched, or it seems natural. (We may want to consider which parts of this logging is considered important enough to keep and which can go away. Some of them might be important enough or candidates for promotion to warnings)

Not sure about the c++ world, but I would assume breakpoints and inspecting variables during runtime is more useful that print statements for debugging when we have an idea of where the bug is located.

Revision history for this message
SirVer (sirver) wrote :

I like the suggestion. I think that is a good way forward.

> Not sure about the c++ world, but I would assume breakpoints and inspecting variables during runtime is more useful that print statements for debugging when we have an idea of where the bug is located.

I find in my experience that mostly junior developers use debuggers and breakpoints - the more senior a dev gets the more she relies on printf debugging. It is actually a funny curve - very junior and more senior devs use more printf debugging, those in the middle seem to rely more on the debugger. Of course that is my tainted interpretation of what I see :).

Revision history for this message
GunChleoc (gunchleoc) wrote :

When I debug, I always use my own print output. It is still useful to have some important events in the normal log though, which help me find a general area on where to start searching. We could keep some extra messages for debug builds that aren't shown in release builds.

Revision history for this message
SirVer (sirver) wrote :

> We could keep some extra messages for debug builds that aren't shown in release builds.

I think that is not a good idea. Release builds should be functionality wise equivalent than debug builds, just faster. IMHO even all asserts() should also be in release builds (but that is currently not the case for Widelands) - after all release games are played much more often than debug builds, so they will likely uncover more bugs.

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

Hm, interesting note on debugging. :) I've also seen senior devs which only on rare occasions run the code when debugging, instead they simply read through it fixing the wrong parts as they go along. Works surprisingly fine when you got the test coverage to do it.

Revision history for this message
GunChleoc (gunchleoc) wrote :
Changed in widelands:
status: Incomplete → Won't Fix
Changed in widelands (Debian):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.