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

Add: Unique ID to savegames to prevent accidental overwriting #7128

Closed
wants to merge 2 commits into from

Conversation

nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Jan 28, 2019

This PR adds a new randomly generated unique ID as a setting to savegames to avoid accidental overwriting of another savegame in the save dialog.
The ID is automatically generated :

  • When a new world is generated
  • When an old savegame (prior to this new version) is loaded

When the user attempts to overwrite a save with a different ID, a warning popup shows and a message is also shown in the details of the save.

This is a "rescue" of #6973.

@nielsmh nielsmh added savegame upgrade enhancement Issue would be a good enhancement; we accept Pull Requests! labels Jan 28, 2019
@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 28, 2019

Okay it looks like there may be an issue with scenarios. When a scenario file is saved, it gets a unique id, and that unique id carries over to all games started from that scenario. So, when starting a new game from a scenario, it needs to have a new unique id generated.

Also may need to think about unique id's for scenarios. If you save a scenario under a new name, should it get a new unique id?

@RoqueDeicide
Copy link

Was it discussed before to have standard "Save/Save As" behavior where the game checks whether the file being saved to is the same as one that's currently loaded? Surely that's an easier option that doesn't require any modifications to savegame files.

Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

does the scenario mode have a different save dialogue? that should do this as well, imo already addressed

src/fios_gui.cpp Outdated Show resolved Hide resolved
src/lang/english.txt Outdated Show resolved Hide resolved
@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 28, 2019

Was it discussed before to have standard "Save/Save As" behavior where the game checks whether the file being saved to is the same as one that's currently loaded? Surely that's an easier option that doesn't require any modifications to savegame files.

I don't think that discussion has been had, no. On the other hand, the unique ID solution has the advantage that keeping e.g. a short rotating history of saves for the same game is less an issue. However historically "save to same file" has never been a thing TT encouraged, it has always suggested a new file by default.

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 28, 2019

image

@RoqueDeicide
Copy link

it has always suggested a new file by default.

That sounds like a rushed feature from early 90-s, akin to Doom not having key-rebind menu. I always kept to 1 file per game and having to open save-game dialog box and select the same file from the list is definitely something I could live without.

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 28, 2019

@RoqueDeicide Changing the defaults for save names probably belongs in a different PR :)
But I don't disagree that the current behavior may be questionable. Also that the scenario editor always suggests UNNAMED for name, instead of giving a blank box.

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 29, 2019

Experimenting with a different way of presenting the "maybe you're overwriting something" in the save window:
image

Yes colours are wrong, going to fix them in a moment.

@nielsmh
Copy link
Contributor Author

nielsmh commented Jan 29, 2019

I think the consensus on IRC is to scrap the unique ID, and just have the Save window always warn when overwriting a file.

@LordAro LordAro added the wip Work in progress. Feature branch that will require feedback during the development process label Jan 30, 2019
@nielsmh
Copy link
Contributor Author

nielsmh commented Feb 1, 2019

Closing in favor of #7156.

@nielsmh nielsmh closed this Feb 1, 2019
@nielsmh nielsmh deleted the save-unique-id branch February 1, 2019 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue would be a good enhancement; we accept Pull Requests! wip Work in progress. Feature branch that will require feedback during the development process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants