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

This comment has been minimized.

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

This comment has been minimized.

Copy link

RoqueDeicide 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.

Copy link
Member

LordAro left a comment

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 nielsmh force-pushed the nielsmh:save-unique-id branch from 6764e19 to 2f4d11b Jan 28, 2019
@nielsmh

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

nielsmh commented Jan 28, 2019

image

@nielsmh nielsmh force-pushed the nielsmh:save-unique-id branch from 2f4d11b to 43aad3b Jan 28, 2019
@RoqueDeicide

This comment has been minimized.

Copy link

RoqueDeicide commented Jan 28, 2019

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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 label Jan 30, 2019
@nielsmh

This comment has been minimized.

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 nielsmh:save-unique-id branch Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.