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

Fix a bug in "restart" and introduce "reload" console command #8527

Merged
merged 2 commits into from Jan 11, 2021

Conversation

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jan 8, 2021

Motivation / Problem

#7328 missed a case where a new game doesn't reset _file_to_saveload.abstract_ftype. This could make restart behave rather weird and unpredictable.

#8469, despite the way of wording, does have a point, which was already brought up in a comment in the original PR. Basically, "restart" and "reload" really are two different words, with different meaning. The new "restart" as accepted by #7328 acted in a way many people would not expect, especially as you are already used to the old one.

To top it off, the console command help-text was never changed, making it even more confusing.

Description

In this PR I opted to revert the change to "restart" from #7328, and reintroduce it as "reload". As we haven't had a release since the introduction of that PR, this should be fine. Now people who are used to the way the old command worked, can keep using it that way. Those who fancy the new way, can use that to.

In my world, the naming makes more sense this way:

  • If you want to restart a map, I expect it to be clean of trains, companies, etc.
  • If I want to reload a map, I expect to have exactly that: a reload of what I last loaded the game with.

This was echo'd by several comments in #7328.

I went through these use-case:

  • If I am an AI developer, I would like to use "restart", as that gives me the same map, clean slate, and the AI can go nuts.
  • If I am an NewGRF developer, I would like to use "reload", as I would want to see the buildings I put down, the vehicles, etc.
  • If I am a server owner, I would like to use "restart": a competitive game that repeats on the same map, can use "restart". If he has to upgrade or reboot his machine, he might have loaded the savegame from the reboot, but he still expects "restart" to begin from zero.

But most of all, I find it very weird to so drastically change an existing console command between 2 releases, without having a good argument not to name the new way different. The ones given in #7328 were not convincing to me, at all. But, truth be told, this is a personal itch, as I was mostly annoyed the help text was not updated to reflect reality :)

Limitations

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

Things to not forget after merge

TrueBrain added 2 commits Jan 8, 2021
In the sequence:
- Load a game
- Start a newgame (via console)
- Restart a game (via console)
Gave you the loaded game back, not the new game.
The current "restart" command is now called "reload", as that is
what it does.
The old "restart" command is now called "restart", as that is what
it did.

As this has not been in any official release yet, this shouldn't
harm any kitten.
@TrueBrain TrueBrain added this to the 1.11.0 milestone Jan 8, 2021
Copy link
Member

@LordAro LordAro left a comment

Yeah, this is better. I was never very happy with the old one...

@TrueBrain TrueBrain merged commit 760b0cd into OpenTTD:master Jan 11, 2021
7 checks passed
7 checks passed
Emscripten
Details
Commit checker
Details
Linux (clang, clang++)
Details
Linux (gcc, g++)
Details
Mac OS (x64, x86_64)
Details
Windows (x86)
Details
Windows (x64)
Details
@TrueBrain TrueBrain deleted the TrueBrain:new-game-commands branch Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants