widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Needs Fixing



Diff comments:

> === modified file 'src/logic/save_handler.cc'
> --- src/logic/save_handler.cc	2017-01-25 18:55:59 +0000
> +++ src/logic/save_handler.cc	2017-04-21 16:12:49 +0000
> @@ -37,25 +37,119 @@
>  #include "wlapplication.h"
>  #include "wui/interactive_base.h"
>  
> +// The actual work of saving is done by the GameSaver
>  using Widelands::GameSaver;
>  
> -/**
> -* Check if autosave is not needed.
> +SaveHandler::SaveHandler()
> +       : next_save_realtime_(0),
> +        initialized_(false),
> +        allow_saving_(true),
> +        save_requested_(false),
> +        saving_next_tick_(false),
> +        save_filename_(""),
> +        autosave_filename_("wl_autosave"),
> +        fs_type_(FileSystem::ZIP),
> +        autosave_interval_in_ms_(DEFAULT_AUTOSAVE_INTERVAL * 60 * 1000),
> +		number_of_rolls_(5)
> +{
> +}
> +
> +void SaveHandler::rollSaveFiles(const std::string& filename) {

Widelands style uses snake_case_for_functions. You have some Javaisms here :)

> +
> +    int32_t rolls = number_of_rolls_;
> +	log("Autosave: Rolling savefiles (count): %d\n", rolls);
> +    rolls--;
> +	std::string filename_previous = create_file_name(
> +	   get_base_dir(), (boost::format("%s_%02d") % filename % rolls).str());
> +	if (rolls > 0 && g_fs->file_exists(filename_previous)) {
> +		g_fs->fs_unlink(filename_previous); // Delete last of the rolling files
> +		log("Autosave: Deleted %s\n", filename_previous.c_str());
> +	}
> +	rolls--;
> +	while (rolls >= 0) {
> +		const std::string filename_next = create_file_name(
> +		   get_base_dir(), (boost::format("%s_%02d") % filename % rolls).str());
> +		if (g_fs->file_exists(filename_next)) {
> +			g_fs->fs_rename(filename_next, filename_previous); // e.g. wl_autosave_08 -> wl_autosave_09
> +			log("Autosave: Rolled %s to %s\n", filename_next.c_str(), filename_previous.c_str());
> +		}
> +		filename_previous = filename_next;
> +		rolls--;
> +	}
> +}
> +
> +/**
> + * Check if game should be saved at next tick / think.
> + *
> + * @return true if game should be saved ad next think().
> + */
> +bool SaveHandler::checkNextTick(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;
> +	}
> +
> +	log("Autosave: %d ms interval elapsed, current gametime: %s, saving...\n", autosave_interval_in_ms_,
> +	    gametimestring(game.get_gametime(), true).c_str());
> +
> +	game.get_ibase()->log_message(_("Saving game…"));
> +	return true;
> +}
> +
> +/**
> + * If saving fails restore the backup file.
> + *
> + * @return true when save was a success.
> + */
> +bool SaveHandler::saveAndHandleError(Widelands::Game& game,
> +						const std::string& complete_filename,
> +						const std::string& backup_filename) {
> +	std::string error;
> +    bool result = save_game(game, complete_filename, &error);
> +	if (!result) {
> +		log("Autosave: ERROR! - %s\n", error.c_str());
> +		game.get_ibase()->log_message(_("Saving failed!"));
> +
> +		// if backup file was created, move it back
> +		if (backup_filename.length() > 0) {
> +			if (g_fs->file_exists(complete_filename)) {
> +				g_fs->fs_unlink(complete_filename);
> +			}
> +			g_fs->fs_rename(backup_filename, complete_filename);
> +		}
> +		// Wait 30 seconds until next save try
> +		next_save_realtime_ += 30000;
> +	} else {
> +		// if backup file was created, time to remove it
> +		if (backup_filename.length() > 0 && g_fs->file_exists(backup_filename))
> +			g_fs->fs_unlink(backup_filename);
> +	}
> +    return result;
> +}
> +
> +/**
> + * 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();
>  	initialize(realtime);
> -	std::string filename = autosave_filename_;
> -
> -	if (!allow_saving_) {
> -		return;
> -	}
> -	if (game.is_replay()) {
> -		return;
> -	}
>  
>  	// Are we saving now?
>  	if (saving_next_tick_ || save_requested_) {
> +        std::string filename = autosave_filename_;
>  		if (save_requested_) {
>  			// Requested by user
>  			if (!save_filename_.empty()) {


-- 
https://code.launchpad.net/~widelands-dev/widelands/save_refactor/+merge/322622
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/save_refactor.