widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1678987-save-handler into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1678987 in widelands: "Long game will hang on Windows while saving"
  https://bugs.launchpad.net/widelands/+bug/1678987

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1678987-save-handler/+merge/349096

Just for the AppVeyor build - do not review yet.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1678987-save-handler into lp:widelands.

=== modified file 'src/logic/player.cc'
--- src/logic/player.cc	2018-07-12 04:41:20 +0000
+++ src/logic/player.cc	2018-07-12 20:20:31 +0000
@@ -1439,39 +1439,46 @@
 }
 
 /**
- * Write statistics data to the give file
+ * Write statistics data to the given file
  */
 void Player::write_statistics(FileWrite& fw) const {
+	const Tribes& tribes = egbase().tribes();
+	const std::set<DescriptionIndex>& tribe_wares = tribe().wares();
+	const size_t nr_wares = tribe_wares.size();
+
 	// Write produce statistics
-	fw.unsigned_16(current_produced_statistics_.size());
+	fw.unsigned_16(nr_wares);
 	fw.unsigned_16(ware_productions_[0].size());
 
-	for (uint8_t i = 0; i < current_produced_statistics_.size(); ++i) {
-		fw.c_string(egbase().tribes().get_ware_descr(i)->name());
-		fw.unsigned_32(current_produced_statistics_[i]);
-		for (uint32_t j = 0; j < ware_productions_[i].size(); ++j)
-			fw.unsigned_32(ware_productions_[i][j]);
+	for (const DescriptionIndex ware_index : tribe_wares) {
+		fw.c_string(tribes.get_ware_descr(ware_index)->name());
+		fw.unsigned_32(current_produced_statistics_[ware_index]);
+		for (uint32_t j = 0; j < ware_productions_[ware_index].size(); ++j) {
+			fw.unsigned_32(ware_productions_[ware_index][j]);
+		}
 	}
 
 	// Write consume statistics
-	fw.unsigned_16(current_consumed_statistics_.size());
+	fw.unsigned_16(nr_wares);
 	fw.unsigned_16(ware_consumptions_[0].size());
 
-	for (uint8_t i = 0; i < current_consumed_statistics_.size(); ++i) {
-		fw.c_string(egbase().tribes().get_ware_descr(i)->name());
-		fw.unsigned_32(current_consumed_statistics_[i]);
-		for (uint32_t j = 0; j < ware_consumptions_[i].size(); ++j)
-			fw.unsigned_32(ware_consumptions_[i][j]);
+	for (const DescriptionIndex ware_index : tribe_wares) {
+		fw.c_string(tribes.get_ware_descr(ware_index)->name());
+		fw.unsigned_32(current_consumed_statistics_[ware_index]);
+		for (uint32_t j = 0; j < ware_consumptions_[ware_index].size(); ++j) {
+			fw.unsigned_32(ware_consumptions_[ware_index][j]);
+		}
 	}
 
 	// Write stock statistics
-	fw.unsigned_16(ware_stocks_.size());
+	fw.unsigned_16(nr_wares);
 	fw.unsigned_16(ware_stocks_[0].size());
 
-	for (uint8_t i = 0; i < ware_stocks_.size(); ++i) {
-		fw.c_string(egbase().tribes().get_ware_descr(i)->name());
-		for (uint32_t j = 0; j < ware_stocks_[i].size(); ++j)
-			fw.unsigned_32(ware_stocks_[i][j]);
+	for (const DescriptionIndex ware_index : tribe_wares) {
+		fw.c_string(tribes.get_ware_descr(ware_index)->name());
+		for (uint32_t j = 0; j < ware_stocks_[ware_index].size(); ++j) {
+			fw.unsigned_32(ware_stocks_[ware_index][j]);
+		}
 	}
 }
 }

=== modified file 'src/logic/save_handler.cc'
--- src/logic/save_handler.cc	2018-04-07 16:59:00 +0000
+++ src/logic/save_handler.cc	2018-07-12 20:20:31 +0000
@@ -83,14 +83,11 @@
  * @return true if game should be saved ad next think().
  */
 bool SaveHandler::check_next_tick(Widelands::Game& game, uint32_t realtime) {
-
 	// Perhaps save is due now?
 	if (autosave_interval_in_ms_ <= 0 || next_save_realtime_ > realtime) {
 		return false;  // no autosave or not due, yet
 	}
 
-	next_save_realtime_ = realtime + autosave_interval_in_ms_;
-
 	// check if game is paused (in any way)
 	if (game.game_controller()->is_paused_or_zero_speed()) {
 		return false;
@@ -125,11 +122,16 @@
 			g_fs->fs_rename(backup_filename, complete_filename);
 		}
 		// Wait 30 seconds until next save try
-		next_save_realtime_ += 30000;
+		next_save_realtime_ = SDL_GetTicks() + 30000;
 	} else {
 		// if backup file was created, time to remove it
-		if (backup_filename.length() > 0 && g_fs->file_exists(backup_filename))
+		if (backup_filename.length() > 0 && g_fs->file_exists(backup_filename)) {
 			g_fs->fs_unlink(backup_filename);
+		}
+
+		// Count save interval from end of save.
+		// This prevents us from going into endless autosave cycles if the save should take longer than the autosave interval.
+		next_save_realtime_ = SDL_GetTicks() + autosave_interval_in_ms_;
 	}
 	return result;
 }
@@ -138,12 +140,11 @@
  * Check if autosave is needed and allowed or save was requested by user.
  */
 void SaveHandler::think(Widelands::Game& game) {
-
 	if (!allow_saving_ || game.is_replay()) {
 		return;
 	}
 
-	uint32_t realtime = SDL_GetTicks();
+	const uint32_t realtime = SDL_GetTicks();
 	initialize(realtime);
 
 	// Are we saving now?
@@ -185,7 +186,6 @@
 		log("Autosave: save took %d ms\n", SDL_GetTicks() - realtime);
 		game.get_ibase()->log_message(_("Game saved"));
 		saving_next_tick_ = false;
-
 	} else {
 		saving_next_tick_ = check_next_tick(game, realtime);
 	}