Skip to content

Savegame filename improvements#267

Merged
nthykier merged 21 commits intosingularity:masterfrom
gunchleoc:default-save-name
Jun 7, 2020
Merged

Savegame filename improvements#267
nthykier merged 21 commits intosingularity:masterfrom
gunchleoc:default-save-name

Conversation

@gunchleoc
Copy link
Contributor

@gunchleoc gunchleoc commented May 28, 2020 •
edited
Loading

  • When player doesn't want to overwrite a savegame, bounce back to filename text input
  • Use difficulty as default savename
  • Make filenames safe for Linux, Windows & Mac

@codecov-commenter
Copy link

codecov-commenter commented May 28, 2020 •
edited
Loading

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 25.00000% with 15 lines in your changes missing coverage. Please review.

Project coverage is 45.85%. Comparing base (c941326) to head (8950143).
Report is 115 commits behind head on master.

Files with missing lines Patch % Lines
singularity/code/savegame.py 22.22% 14 Missing ⚠️
singularity/code/graphics/dialog.py 50.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #267      +/-   ##
==========================================
- Coverage   45.90%   45.85%   -0.06%     
==========================================
  Files          44       44              
  Lines        6916     6931      +15     
==========================================
+ Hits         3175     3178       +3     
- Misses       3741     3753      +12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gunchleoc gunchleoc marked this pull request as draft May 29, 2020 08:18
@gunchleoc
Copy link
Contributor Author

gunchleoc commented May 29, 2020

I have some more ideas for improvements, so setting this to draft for now.

@gunchleoc gunchleoc changed the title Make "Default Save" translatable Savagame filename improvements May 29, 2020
@gunchleoc gunchleoc changed the title Savagame filename improvements Savegame filename improvements May 29, 2020
@gunchleoc gunchleoc marked this pull request as ready for review May 29, 2020 14:53
nthykier
nthykier reviewed May 31, 2020
View reviewed changes
nthykier
nthykier requested changes May 31, 2020
View reviewed changes
@nthykier
Copy link
Member

nthykier commented Jun 2, 2020

I think we need a way to handle existing save games with spaces in them. I have used a space a lot and loading such a game and saving it again later just saves under the new name (no prompt for overrides) and now the save game appears twice with same (visual) name but different content, which is confusing to say the least.

nthykier
nthykier reviewed Jun 5, 2020
View reviewed changes
Copy link
Member

@nthykier nthykier left a comment

Choose a reason for hiding this comment

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

Thanks for improving this, it is looking quite good now and definitely much better than what we had in the previous release! :)

I got a few remarks left and then I think we are ready to merge it. Please take this opportunity to rebase it if you want to squash some of the commits (or let me know if I should do it on merge).

gunchleoc and others added 2 commits June 5, 2020 18:35
@gunchleoc @nthykier
Tweak a text
eba4305 
Co-authored-by: Niels Thykier <[email protected]>
@gunchleoc @nthykier
Tweak a text
869c8a2 
Co-authored-by: Niels Thykier <[email protected]>
@gunchleoc
Copy link
Contributor Author

gunchleoc commented Jun 6, 2020

All done.

I think it will be easiest if you hit the "Squash" button during merging - the original PR comment should do as commit message :)

@nthykier nthykier merged commit d499310 into singularity:master Jun 7, 2020
@nthykier
Copy link
Member

nthykier commented Jun 7, 2020

Thanks :)

@gunchleoc gunchleoc deleted the default-save-name branch June 7, 2020 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nthykier nthykier nthykier approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gunchleoc @codecov-commenter @nthykier