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

Feature: Improve restart command #7328

Open
wants to merge 1 commit into
base: master
from
Open

Feature: Improve restart command #7328

wants to merge 1 commit into from

Conversation

@Berbe
Copy link
Contributor

Berbe commented Mar 5, 2019

When the restart command is issued, a normal map is always spawned.

This improvement takes into account the current state of _file_to_saveload to check if a savegame/scenario/heightmap was previously loaded, and loads the same resource again.

@PeterN
Copy link
Member

PeterN commented Mar 5, 2019

I'm not sure I would expect restart to reload the game.

@planetmaker
Copy link
Contributor

planetmaker commented Mar 5, 2019

I'm not sure I would expect restart to reload the game.

I think such makes sense for competitive servers which re-run the same scenario. Could be argued that 'restart' generates a new map with current newgame settings while 'reload' restarts the currently loaded map / savegame.

@PeterN
Copy link
Member

PeterN commented Mar 5, 2019

Yes, a reload command is what I was thinking of.

@Eddi-z
Copy link
Contributor

Eddi-z commented Mar 5, 2019

Here's what i intuitively would think restart would do:

  1. try to load the scenario (or heightmap) this game was originally started from, or
  2. start a new game with the current game seed and settings

but don't do anything special with savegames.

@Eddi-z
Copy link
Contributor

Eddi-z commented Mar 5, 2019

That would mean, a reference to the original scenario/heightmap must be stored in savegames, and this reference must still work if the file was moved (fallback to 2. if it was not found)

@nielsmh
Copy link
Contributor

nielsmh commented Mar 5, 2019

How about the Quake approach - have a variable/setting containing a console command to execute on game end? That command could be a simple newgame command, or it could be one to execute a longer script. It would probably need some kind of additional check to confirm a new game was in fact started, and if not create a new default game.

@Eddi-z
Copy link
Contributor

Eddi-z commented Mar 5, 2019

i don't see how that has anything to do with the topic being discussed.

this is about what the restart command should do, not about ways it should be triggered

@Berbe
Copy link
Contributor Author

Berbe commented Mar 5, 2019

At the moment, in trunk, if you use restart on a randomly generated map, it generates the same map again, ie using the same seed.
Since the state does not change, I find it appropriate that when loading an external resource (be it savegame/scenario/heightmap), the same behaviour apply, that is applying the same exact starting conditions again.

Hence, I don't understand your restart/reload split as I am merely attempting to mimick the current behaviour.

@Berbe Berbe force-pushed the Berbe:restart branch from bc45ebc to 76bd51c Mar 5, 2019
@Berbe
Copy link
Contributor Author

Berbe commented Mar 5, 2019

Here's what i intuitively would think restart would do:

1. try to load the scenario (or heightmap) this game was originally started from, or
2. start a new game with the current game seed and settings

but don't do anything special with savegames.

Why would you make savegames stand out? Savegames and scenarios are kind of the same resource. To me they are interchangeable.
What if you wanted to host games deriving from a specific savegame? On (automatic - after year N) restart, you would spawn a new randomly generated map, which would miss the point.

@Eddi-z
Copy link
Contributor

Eddi-z commented Mar 5, 2019

Why would you make savegames stand out? Savegames and scenarios are kind of the same resource. To me they are interchangeable.

they might be interchangable on a technical level, but not really at a logical/usage level.

What if you wanted to host games deriving from a specific savegame?

you could make that savegame into a scenario, then start from that scenario.

but what about the reverse? what if you loaded a backup savegame because of some griefers that messed everything up? you'd restart from that instead of the original one in your way.

@Berbe Berbe force-pushed the Berbe:restart branch from 76bd51c to 92a7b68 Mar 5, 2019
@Berbe
Copy link
Contributor Author

Berbe commented Mar 5, 2019

they might be interchangable on a technical level, but not really at a logical/usage level.

It is true I got carried away by the technical level, as I did not have a point of reference for semantics in OpenTTD. Savegames & scenarios are literally the same, except the former did not necessarily come from the editor...

I made some changes to only intercept savegames in editor mode. Would that be satisfactory?

@Berbe Berbe force-pushed the Berbe:restart branch 2 times, most recently from 0e6a350 to e13ac77 Mar 5, 2019
@Berbe Berbe marked this pull request as ready for review Mar 6, 2019
@stale
Copy link

stale bot commented Apr 5, 2019

This pull request has been automatically marked as stale because it has not had any activity in the last month.
Please feel free to give a status update now, ping for review, or re-open when it's ready.
It will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added stale and removed stale labels Apr 5, 2019
@Berbe
Copy link
Contributor Author

Berbe commented Apr 10, 2019

Ping?

src/openttd.cpp Outdated Show resolved Hide resolved
src/openttd.cpp Outdated Show resolved Hide resolved
@Berbe Berbe force-pushed the Berbe:restart branch from e13ac77 to 4d8acc4 Apr 28, 2019
@Berbe Berbe force-pushed the Berbe:restart branch 3 times, most recently from 4b2718f to 1fdd16c Aug 5, 2019
@Berbe Berbe changed the title Improve restart Feature: Improve restart command Aug 8, 2019
@Berbe Berbe force-pushed the Berbe:restart branch 2 times, most recently from 8715f42 to 4279881 Feb 10, 2020
src/openttd.cpp Outdated Show resolved Hide resolved
When the restart command is issued, a normal map is always spawned.

This improvement takes into account the current state of _file_to_saveload to check if a savegame/scenario/heightmap was previously loaded, and loads the same resource again.
@Berbe Berbe force-pushed the Berbe:restart branch from 4279881 to 1935576 Feb 10, 2020
@Berbe Berbe requested a review from LordAro Mar 24, 2020
@@ -1078,7 +1078,22 @@ void SwitchToMode(SwitchMode new_mode)
MakeNewEditorWorld();
break;

case SM_RESTARTGAME: // Restart --> 'Random game' with current settings
case SM_RESTARTGAME: // Restart --> Current settings preserved
if (_file_to_saveload.abstract_ftype == FT_SAVEGAME || _file_to_saveload.abstract_ftype == FT_SCENARIO) {

This comment has been minimized.

Copy link
@LordAro

LordAro Apr 13, 2020

Member

I made some changes to only intercept savegames in editor mode. Would that be satisfactory?

I don't think it would, tbh. Would just be a confusing edgecase.

Besides, when could this actually happen? You can't load savegames into the editor, only scenarios (even if they're just savegames renamed to .scn extension)

This comment has been minimized.

Copy link
@Berbe

Berbe Apr 25, 2020

Author Contributor

You refer to a reply from 13 months ago, on a codebase which has changed since then.
I do not understand your comment in the context on the discussion you quote, applied to unrelated code?
What are you expecting to see?

@LordAro LordAro dismissed their stale review Apr 13, 2020

Old review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.