Allow NewGRFs to give error popups, without disabling them. #9119
Conversation
Translations always must be done using the web translator (https://github.com/OpenTTD/team/issues/new/choose), no exceptions. |
|
When deleting translations that you added as part of a PR, don't delete the whole file, only delete the string that you added. If you need to delete the string in multiple languages, try to do it in a single commit. |
Alright. How do I delete changes from a PR and how do I add strings to english without changing english.lng? I presume you would need at least 1 string for the thing you want to add. |
|
We have a CONTRIBUTING.md that you may find interesting reading |
There was a problem hiding this comment.
You will need to rebase your commits into a single commit which follows the commit message format. I suggest "Change: [NewGRF] NewGRFs can create a non-fatal error window"
src/newgrf_gui.cpp
Outdated
| if (c->error->severity == STR_NEWGRF_ERROR_MSG_FATAL) { | ||
| ShowErrorMessage(STR_NEWGRF_ERROR_FATAL_POPUP, INVALID_STRING_ID, WL_CRITICAL); | ||
| } else { | ||
| ShowErrorMessage(STR_NEWGRF_ERROR_POPUP, INVALID_STRING_ID, WL_CRITICAL); |
There was a problem hiding this comment.
I would guess we shouldn't fire this as WL_CRITICAL. WL_ERROR sounds more appropriate in this case?
EratoNysiad
force-pushed
the
ErrorsAreErrors
branch
5 times, most recently
from
6bedd78 to
ff5be1d
Compare
…p window now. Fixed issue OpenTTD#9115 by also creating an error pop-up for errors with the severity ERROR, instead of just FATAL. Also added a string for it, STR_NEWGRF_ERROR_POPUP, which is the string STR_NEWGRF_ERROR_FATAL_POPUP, but without the word "fatal". This makes ERROR functionally distinct from WARNING.
EratoNysiad
force-pushed
the
ErrorsAreErrors
branch
from
ff5be1d to
973452c
Compare
Fixes #9115 by also creating an error pop-up for errors with the severity ERROR, instead of just FATAL. Also added a string for it, STR_NEWGRF_ERROR_POPUP, which is the string STR_NEWGRF_ERROR_FATAL_POPUP, but without the word "fatal". Also provided a translation in a few languages I'm confident enough in to remove a word.
Motivation / Problem
Say you have a NewGRF, but you should only ever use it with another NewGRF, or a certain combination of settings is completely game breaking. You want to inform the players that they might need to rethink their choices, but you don't want to stop them from going ahead anyway, should they choose to ignore it.
AndyTheNorth raised a similar problem; people kept complaining that they didn't have vehicle newgrfs and they couldn't use FIRS as a result. The ideal solution would have been an error popup to explain to users that they don't have a compatible boat newgrf. The only way of doing this would have been by means of
error(FATAL,string(STR_ERROR));in NML. This would disable the NewGRF, meaning NewGRFs that aren't FIRS compatible would have a hard time becoming FIRS compatible, and that I can't choose to just not have a ship newgrf because I don't use ships.In NML you can have 4 different types of errors. NOTICE, WARNING, ERROR, and FATAL. You would expect all of these to behave differently in some way, but the only difference between WARNING and ERROR is the red word in the message in the NewGRF window. FATAL gives a popup message, but also disables the NewGRF. Now it would make sense to me if ERROR also gave a pop-up, making it distinct from the other 4.
With this change, NewGRF developers like Andy can tell players "Hey we have detected that you don't have a compatible Road Vehicle NewGRF. This means you will not be able to move goods via truck. Is this correct, and if so are you sure about that?"
For beginning players who are told to play FIRS for whatever reason, this helps by telling them what is wrong. They might not know to look in the NewGRF window, and might miss the message. They might not know that they need a FIRS compatible NewGRF to use it.
For veteran players, this is nice because they might decide that they don't want to use ships anyway, so they can just press [x] and continue playing. Other veteran players might forgot to load a ship newgrf, when they meant to. It's nice to be told this upfront, and to not discover an hour into the game.
The direct motivation for this was a perceived issue with FIRS, but I believe this can be widely applicable.
Description
In newgrf_gui.cpp there's a line of code that prevents a popup from appearing if the severity is not
FATAL. I changed it so it also shows a popup if the severity isERROR.If the severity is
ERRORinstead ofFATALit will use a different string,STR_NEWGRF_ERROR_POPUPinstead ofSTR_NEWGRF_ERROR_FATAL_POPUP, which omits the word "fatal" from the error message.Example:
Limitations
The feature itself is fairly complete.
Older NewGRFs might have used ERROR when it didn't give a popup, and loading it with this change will cause them to generate a pop-up. I think that's a very minor problem.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.