Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate IConsolePrint functions and improve some of the messages #9359

Merged
merged 9 commits into from Jun 13, 2021

Conversation

rubidium42
Copy link
Contributor

@rubidium42 rubidium42 commented Jun 12, 2021

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
Copy link
Member

@TrueBrain TrueBrain left a comment

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 :)

src/console.cpp Outdated Show resolved Hide resolved
src/console_cmds.cpp Outdated Show resolved Hide resolved
src/console_cmds.cpp Outdated Show resolved Hide resolved
src/console_cmds.cpp Show resolved Hide resolved
src/network/network_server.cpp Outdated Show resolved Hide resolved
src/console.cpp Outdated Show resolved Hide resolved
src/console_cmds.cpp Outdated Show resolved Hide resolved
src/console_cmds.cpp Outdated Show resolved Hide resolved
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.
Copy link
Member

@TrueBrain TrueBrain left a comment

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