Skip to content

Consolidate IConsolePrint functions and improve some of the messages - #9359

Merged
rubidium42 merged 9 commits into
OpenTTD:masterfrom
rubidium42:console_simplification
Jun 13, 2021
Merged

rubidium42 merged 9 commits into
OpenTTD:masterfrom
rubidium42:console_simplification

Conversation

@rubidium42

@rubidium42 rubidium42 commented Jun 12, 2021

Copy link
Copy Markdown
Contributor

Motivation / Problem

Where to start?

  • Some warnings get "WARNING" added, others do not.
  • Some errors get "ERROR" added, others do not.
  • Some issues that mean a command cannot be run are shown as warnings, whereas other times they are called as errors.
  • Some messages starting with "WARNING" are shown in the error color.
  • Vast inconsistency in writing of messages.
  • Pointless "-" in front of the help messages, but not for all of them.
  • Some "warnings" (or rather issues that would prevent the command to be not run) would not get their "error" shown based on a setting.
  • IConsoleError/IConsoleWarning/IConsoleDebug/IConsoleHelp "helper" functions did not support formatting of strings.
  • IConsolePrint and IConsolePrintF were used interchangeably, well... IConsolePrintF was used when IConsolePrint could have been used.

Description

  • Consolidate all the console printing functions into IConsolePrint, which now has the same functionality as IConsolePrintF.
  • Remove IConsoleWarning( in lieu of IConsolePrint(CC_WARNING, .
  • Remove the filtering of some warning messages.
  • Remove IConsoleError( in lieu of IConsolePrint(CC_ERROR, .
  • Remove IConsoleHelp( in lieu of IConsolePrint(CC_HELP, .
  • Add CC_HELP to distinguish between CC_HELP and CC_WARNING.
  • Revisit all error messages on their writing and their "color", i.e. make some errors warnings and vice versa.

Feel free to propose other improvements to strings printed to the in-game console.

If this approach is taken over adding formatting functionality to IConsoleError/Warning, then this closes #8853 and closes #8894.

Limitations

None I think, unless someone did matching on colors of strings, or was actually parsing the strings... as then those might now be broken in some cases.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@rubidium42
rubidium42 force-pushed the console_simplification branch 2 times, most recently from 986d851 to c40bad9 Compare June 13, 2021 08:36

@TrueBrain TrueBrain left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The first commit is ofc a big no-no, so if anyone during bisecting happens to go back to it, they can get some weird prints, I guess .. but from what I could tell, only in a handful of places, so you have to be really unlucky :P

There also really isn't another way to get where you wanted to go, so I think it is worth doing it like this :)

Comment thread src/console.cpp Outdated
Comment thread src/console_cmds.cpp Outdated
Comment thread src/console_cmds.cpp Outdated
Comment thread src/console_cmds.cpp
Comment thread src/network/network_server.cpp Outdated
Comment thread src/console.cpp Outdated
Comment thread src/console_cmds.cpp Outdated
Comment thread src/console_cmds.cpp Outdated
@rubidium42
rubidium42 force-pushed the console_simplification branch from c40bad9 to f6afef6 Compare June 13, 2021 12:00
If a command cannot be executed for whatever reason, it makes no sense to call it a warning. Something has been done wrong.
Also make writing of these error message consistent while changing their "type".
Always start with a capital, do not add "ERROR: " in front of it.
Both did not support format parameters, so in many places IConsolePrint(CC_ERROR, "message") was used with a style different from what IConsoleError would do.
Also make some strings more consistent with the rest of the console strings.
@rubidium42
rubidium42 force-pushed the console_simplification branch from f6afef6 to 9117efb Compare June 13, 2021 13:04

@TrueBrain TrueBrain left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For sure this is an improvement :) Other nitpicks can be dealt with another time :) Tnx a bunch for this!

@rubidium42
rubidium42 merged commit fc63432 into OpenTTD:master Jun 13, 2021
@rubidium42
rubidium42 deleted the console_simplification branch June 13, 2021 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

There are 17 places where "IConsoleError" can be used instead of "IConsolePrintF".

2 participants