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

Increment save counts when writing LDB, LMU, and LSD #266

Merged
merged 1 commit into from Nov 1, 2018

Conversation

Projects
None yet
4 participants
@fmatthew5876
Copy link
Contributor

fmatthew5876 commented Oct 17, 2018

Depends on: #264

This patch changes *_Reader::Save() to automatically increment save_count before writing the file.

There is a new SaveOpt::eNoIncSaveCount you can pass to suppress this behavior.

I also added SaveOpt::eNoUdate value which is intended to combine all of the SaveOpt's required for copying lcf files without modifications. We should use this SaveOpt for our testing liblcf.

This will fix bugs in Player and Editor related to save counts.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_savecount branch from c7ee345 to a7ccf9a Oct 18, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_savecount branch from a7ccf9a to e926d49 Oct 18, 2018

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Oct 22, 2018

The question is if incrementing the save count should be the default option or not.
One will need changes to Player and editor, the other to all other tools working with LCF files.

btw for save games there is also a timestamp. The Player currently handles this by setting the timestamp but this could be also done via an option.

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Oct 23, 2018

The question is if incrementing the save count should be the default option or not.
One will need changes to Player and editor, the other to all other tools working with LCF files.

Good question. It's also wierd that now the Save_* functions now take the object by non-const reference.

The only thing I feel strongly about is that the increment logic should be in liblcf so it can be written once and shared. Maybe a separate function that needs to be called explicitly by Player and Editor before saving?

@fdelapena fdelapena added this to the 0.6.0 milestone Oct 25, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_savecount branch from e926d49 to 2d4ac3d Oct 27, 2018

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Oct 29, 2018

Doesn't really matter for me if

  • individual function in liblcf or
  • invert the logic of the enums because then we only have to update Player and Editor and keep anything else untouched.

SaveOpt::Update, SaveOpt:IncSaveCount and SaveOpt::UpdateTimestamp (this is for the timestamp in the LSD, currently done in Player but the timestamp function is in liblcf).

Originally liblcf always updated the timestamp when saving but this was obviously bad when comparing files :)

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_savecount branch 2 times, most recently from 1a294cc to c98f726 Oct 29, 2018

Add PrepareSave() method for LDB, LMU, LSD
- Should be called by Player and Editor before saving.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_savecount branch from c98f726 to 477cbf2 Oct 29, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Oct 29, 2018

New implementation. IMO this is much cleaner.

@fdelapena fdelapena added the Savegames label Oct 30, 2018

@Ghabry

Ghabry approved these changes Oct 30, 2018

@carstene1ns carstene1ns merged commit b553a21 into EasyRPG:master Nov 1, 2018

4 checks passed

GNU/Linux Build finished.
Details
OSX Build finished.
Details
Windows Build finished.
Details
web Build finished.
Details

@fmatthew5876 fmatthew5876 deleted the fmatthew5876:ldb_savecount branch Nov 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.