Skip to content

Commit

Permalink
Fixed|Ring Zero: Suppressed a couple of unnecessary warnings
Browse files Browse the repository at this point in the history 
There is no need to show warnings about game plugin help strings and
patch loading failures when no game is loaded.
  • Loading branch information
@skyjake
skyjake committed Jul 12, 2013
1 parent 572e573 commit 46a30b6
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
7 changes: 5 additions & 2 deletions doomsday/client/src/dd_help.cpp
View file
Expand Up @@ -180,8 +180,11 @@ void DD_ReadGameHelp(void)
LOG_AS("DD_ReadGameHelp");
try
{
de::Uri uri(Path("$(App.DataPath)/$(GamePlugin.Name)/conhelp.txt"));
readStrings(App::fileSystem().find(uri.resolved()));
if(App_GameLoaded())
{
de::Uri uri(Path("$(App.DataPath)/$(GamePlugin.Name)/conhelp.txt"));
readStrings(App::fileSystem().find(uri.resolved()));
}
}
catch(Error const &er)
{
Expand Down
6 changes: 4 additions & 2 deletions doomsday/client/src/resource/r_data.cpp
View file
Expand Up @@ -362,8 +362,10 @@ static void loadPatchNames(String lumpName)
}
catch(LumpIndex::NotFoundError const &er)
{
// Log but otherwise ignore this error.
LOG_WARNING(er.asText() + ", ignoring.");
if(App_GameLoaded())
{
LOG_WARNING(er.asText());
}
}
}

Expand Down

3 comments on commit 46a30b6

@danij-deng
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should such warnings really be suppressed? This assumes that the user is not utilizing Doomsday's (admittedly limited at present) resource management and features in Ring Zero.

If the issue is that these load process messages are output when returning to/from Ring Zero and therefore unnecessary because these resources are not loaded, then these routines should not be called in the first place.

Also, why remove the ", ignoring." part from the warning message? Granted, one should think to interpret a warning as a non-serious issue, however this does not convey to the user what action Doomsday has taken (if any). That single extra word helps the user a great deal in my opinion.

@skyjake
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I want warnings to be something that the user really needs to be concerned over. That's why they are highlighted in the log. The ones in this commit are printed because the engine tries to load game data when no game is chosen, a task that will always fail and therefore shouldn't be attempted in the first place. At most I would print a verbose message here.

Re: ignoring

With things reported as WARNING, the action taken by the engine by default is "ignore", as processing continues without disruption. Mentioning "ignoring" therefore only brings value if there are other possible strategies for the engine to handle the situation. Remember that there's also the ERROR and CRITICAL levels for things that are not ignored.

@danij-deng
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said -- should't be attempted in the first place. Suppressing the warning is not needed, the real bug is that the function is being called when it shouldn't be.

Re: ignoring

The point being that it is not clear to the user because the user has no idea whether the engine has multiple strategies for handling the situation unless we tell them (thus, "ignoring" clarifies that it can be ignored). Simply logging "Warning: Tried to do XYZ but failed" does not tell the user anything and turning this into a VERBOSE (because its not fatal) masks the problem completely.

Please sign in to comment.