diff --git a/game/scripts/save_store.gd b/game/scripts/save_store.gd index b742c5f..7736ca5 100644 --- a/game/scripts/save_store.gd +++ b/game/scripts/save_store.gd @@ -5,6 +5,8 @@ extends RefCounted const KEY_FILE := "player.key" const SAVE_FILE := "world.grw" +## Holds the previous save while write_save replaces it. +const BACKUP_FILE := SAVE_FILE + ".bak" const RECORDINGS_DIR := "recordings" ## Recordings beyond this number are deleted, oldest first. const KEEP_RECORDINGS := 10 @@ -13,9 +15,13 @@ const KEY_BYTES := 16 var base_dir: String +## Opens the folder and creates it if it is missing. If a crash interrupted a +## save and left only the backup of the previous one (see write_save), the +## backup becomes the save again. func _init(directory: String = "user://") -> void: base_dir = directory DirAccess.make_dir_recursive_absolute(base_dir) + _restore_backup() ## The key that identifies this installation in every world. It is created @@ -41,15 +47,35 @@ func read_save() -> PackedByteArray: ## Writes the save through a temporary file, so that a crash or a failed ## write never destroys the previous save. Returns false if the new save was ## not stored completely; the previous save then stays as it was. +## +## The temporary file replaces the previous save in two moves: the previous +## save becomes the backup, then the temporary file becomes the save. A single +## move onto the previous save would not be safe, because on Windows Godot +## deletes the target first and only then moves the file. If that move failed, +## for example because a virus scanner holds the temporary file, the previous +## save would already be gone. If the second move fails, the backup is moved +## back. A crash between the two moves leaves only the backup; the next +## SaveStore on the folder restores it. func write_save(bytes: PackedByteArray) -> bool: if bytes.is_empty(): return false var path := base_dir.path_join(SAVE_FILE) var temporary := path + ".tmp" + var backup := base_dir.path_join(BACKUP_FILE) if not _write(temporary, bytes) or FileAccess.get_file_as_bytes(temporary) != bytes: DirAccess.remove_absolute(temporary) return false - return DirAccess.rename_absolute(temporary, path) == OK + var has_previous := FileAccess.file_exists(path) + if has_previous and DirAccess.rename_absolute(path, backup) != OK: + DirAccess.remove_absolute(temporary) + return false + if DirAccess.rename_absolute(temporary, path) != OK: + if has_previous: + DirAccess.rename_absolute(backup, path) + return false + if has_previous: + DirAccess.remove_absolute(backup) + return true ## Moves an unusable save out of the way, so that the next save does not @@ -89,6 +115,16 @@ func recordings() -> PackedStringArray: return names +## Moves the backup back to the save if there is no save, see write_save. +func _restore_backup() -> void: + var path := base_dir.path_join(SAVE_FILE) + var backup := base_dir.path_join(BACKUP_FILE) + if FileAccess.file_exists(path) or not FileAccess.file_exists(backup): + return + if DirAccess.rename_absolute(backup, path) != OK: + push_warning("the previous save could not be restored from %s" % backup) + + func _write(path: String, bytes: PackedByteArray) -> bool: var file := FileAccess.open(path, FileAccess.WRITE) if file == null: diff --git a/game/tests/test_save_store.gd b/game/tests/test_save_store.gd index e9dd236..319ff21 100644 --- a/game/tests/test_save_store.gd +++ b/game/tests/test_save_store.gd @@ -19,6 +19,7 @@ func test_a_save_is_written_and_read_back() -> void: check(store.has_save(), "a save exists") check_eq(store.read_save(), PackedByteArray([4, 5]), "the newest save is read") check(not FileAccess.file_exists(directory.path_join(SaveStore.SAVE_FILE + ".tmp")), "no temporary file is left") + check(not FileAccess.file_exists(directory.path_join(SaveStore.BACKUP_FILE)), "no backup is left") TestFiles.remove_dir(directory) @@ -43,6 +44,58 @@ func test_a_save_that_cannot_be_written_keeps_the_previous_one() -> void: TestFiles.remove_dir(directory) +func test_a_save_that_cannot_be_moved_into_place_keeps_the_previous_one() -> void: + var directory := TestFiles.temporary_dir("held") + var store := SaveStore.new(directory) + store.write_save(PackedByteArray([1, 2, 3])) + # The open handle stands for an outside program, for example a virus + # scanner. The new save can be written, but Windows refuses to move a file + # that is held open. + var holder := FileAccess.open(directory.path_join(SaveStore.SAVE_FILE + ".tmp"), FileAccess.WRITE) + check(not store.write_save(PackedByteArray([4, 5])), "the failed save is reported") + check_eq(store.read_save(), PackedByteArray([1, 2, 3]), "the previous save is untouched") + check(not FileAccess.file_exists(directory.path_join(SaveStore.BACKUP_FILE)), "no backup is left") + holder.close() + TestFiles.remove_dir(directory) + + +func test_a_save_that_cannot_move_the_previous_one_aside_keeps_it() -> void: + var directory := TestFiles.temporary_dir("aside-blocked") + var store := SaveStore.new(directory) + store.write_save(PackedByteArray([1, 2, 3])) + # A folder under the backup name keeps the previous save from moving. + DirAccess.make_dir_absolute(directory.path_join(SaveStore.BACKUP_FILE)) + check(not store.write_save(PackedByteArray([4, 5])), "the failed save is reported") + check_eq(store.read_save(), PackedByteArray([1, 2, 3]), "the previous save is untouched") + check(not FileAccess.file_exists(directory.path_join(SaveStore.SAVE_FILE + ".tmp")), "no temporary file is left") + TestFiles.remove_dir(directory) + + +func test_an_interrupted_save_is_recovered_from_the_backup() -> void: + var directory := TestFiles.temporary_dir("recover") + # A crash between the two moves of write_save leaves only the backup. + var backup := directory.path_join(SaveStore.BACKUP_FILE) + var file := FileAccess.open(backup, FileAccess.WRITE) + file.store_buffer(PackedByteArray([7, 8, 9])) + file.close() + var store := SaveStore.new(directory) + check(store.has_save(), "the previous save is back") + check_eq(store.read_save(), PackedByteArray([7, 8, 9]), "with its content") + check(not FileAccess.file_exists(backup), "the backup is moved, not copied") + TestFiles.remove_dir(directory) + + +func test_a_backup_never_replaces_an_existing_save() -> void: + var directory := TestFiles.temporary_dir("keep") + SaveStore.new(directory).write_save(PackedByteArray([1, 2, 3])) + var file := FileAccess.open(directory.path_join(SaveStore.BACKUP_FILE), FileAccess.WRITE) + file.store_buffer(PackedByteArray([9, 9])) + file.close() + var store := SaveStore.new(directory) + check_eq(store.read_save(), PackedByteArray([1, 2, 3]), "the save is not replaced by the older backup") + TestFiles.remove_dir(directory) + + func test_setting_aside_without_a_save_reports_nothing_moved() -> void: var directory := TestFiles.temporary_dir("nothing") check_eq(SaveStore.new(directory).set_aside_save(), "", "no save, nothing moved")