Quit pretending that underscores in savefile names are actually spaces

The code was frantically converting spaces to underscores and back to keep
the facade up, and sometimes it failed (#1567).

The solution is obvious: simply use spaces for real in savefile names.
This way the conversion is only needed in one place (when generating the
suggested filename).

Fixes #1567. As a minor bonus, this also makes the "save game" dialog show
the filename with spaces.

changelog

@@ -83,6 +83,7 @@ Version 1.13.11:
    * A missing [event] name= key will now raise a WML error instead of being
      silently ignored.
  * Miscellaneous and bug fixes:
+   * Suggested save file names now use spaces instead of underscores.
    * Fixed crash after canceling add-on download (bug #2203)
    * Fixed ingame help showing units you haven't encountered (bug #2135)
    * Fixed the opacity IPF resetting to 0 if the value given was 100% or

players_changelog

@@ -26,6 +26,7 @@ Version 1.13.11:
    * Miscellaneous low-level optimizations in game rendering code, improving
      performance ingame by up to 50 %.
  * Miscellaneous and bug fixes:
+   * Suggested save file names now use spaces instead of underscores.
    * Fixed crash after canceling add-on download (bug #2203)
    * Fixed ingame help showing units you haven't encountered (bug #2135)
    * Fix recalls updating shroud immediately when "Delay Shroud Updates" is set

src/save_index.cpp

@@ -38,26 +38,13 @@ static lg::log_domain log_engine("engine");
 static lg::log_domain log_enginerefac("enginerefac");
 #define LOG_RG LOG_STREAM(info, log_enginerefac)
 
-void replace_underbar2space(std::string& name)
-{
-	std::replace(name.begin(), name.end(), '_', ' ');
-}
-
-void replace_space2underbar(std::string& name)
-{
-	std::replace(name.begin(), name.end(), ' ', '_');
-}
-
 namespace savegame
 {
 void extract_summary_from_config(config&, config&);
 
 void save_index_class::rebuild(const std::string& name)
 {
-	std::string filename = name;
-	replace_space2underbar(filename);
-
-	time_t modified = filesystem::file_modified_time(filesystem::get_saves_dir() + "/" + filename);
+	time_t modified = filesystem::file_modified_time(filesystem::get_saves_dir() + "/" + name);
 	rebuild(name, modified);
 }
 
@@ -206,12 +193,8 @@ std::vector<save_info> get_saves_list(const std::string* dir, const std::string*
 	filesystem::get_files_in_dir(creator.dir, &filenames);
 
 	if(filter) {
-		// Replace the spaces in the filter
-		std::string filter_replaced(filter->begin(), filter->end());
-		replace_space2underbar(filter_replaced);
-
 		filenames.erase(
-			std::remove_if(filenames.begin(), filenames.end(), filename_filter(filter_replaced)), filenames.end());
+			std::remove_if(filenames.begin(), filenames.end(), filename_filter(*filter)), filenames.end());
 	}
 
 	std::vector<save_info> result;
@@ -267,16 +250,12 @@ bool save_info_less_time::operator()(const save_info& a, const save_info& b) con
 }
 
 static filesystem::scoped_istream find_save_file(
-		const std::string& name, const std::string& alt_name, const std::vector<std::string>& suffixes)
+		const std::string& name, const std::vector<std::string>& suffixes)
 {
 	for(const std::string& suf : suffixes) {
 		filesystem::scoped_istream file_stream =
 			filesystem::istream_file(filesystem::get_saves_dir() + "/" + name + suf);
 
-		if(file_stream->fail()) {
-			file_stream = filesystem::istream_file(filesystem::get_saves_dir() + "/" + alt_name + suf);
-		}
-
 		if(!file_stream->fail()) {
 			return file_stream;
 		}
@@ -288,11 +267,8 @@ static filesystem::scoped_istream find_save_file(
 
 void read_save_file(const std::string& name, config& cfg, std::string* error_log)
 {
-	std::string modified_name = name;
-	replace_space2underbar(modified_name);
-
 	static const std::vector<std::string> suffixes{"", ".gz", ".bz2"};
-	filesystem::scoped_istream file_stream = find_save_file(modified_name, name, suffixes);
+	filesystem::scoped_istream file_stream = find_save_file(name, suffixes);
 
 	cfg.clear();
 	try {
@@ -300,9 +276,9 @@ void read_save_file(const std::string& name, config& cfg, std::string* error_log
 		 * Test the modified name, since it might use a .gz
 		 * file even when not requested.
 		 */
-		if(filesystem::is_gzip_file(modified_name)) {
+		if(filesystem::is_gzip_file(name)) {
 			read_gz(cfg, *file_stream);
-		} else if(filesystem::is_bzip2_file(modified_name)) {
+		} else if(filesystem::is_bzip2_file(name)) {
 			read_bz2(cfg, *file_stream);
 		} else {
 			read(cfg, *file_stream);
@@ -350,11 +326,7 @@ void remove_old_auto_saves(const int autosavemax, const int infinite_auto_saves)
 
 void delete_game(const std::string& name)
 {
-	std::string modified_name = name;
-	replace_space2underbar(modified_name);
-
 	filesystem::delete_file(filesystem::get_saves_dir() + "/" + name);
-	filesystem::delete_file(filesystem::get_saves_dir() + "/" + modified_name);
 
 	save_index_manager.remove(name);
 }
@@ -366,12 +338,9 @@ create_save_info::create_save_info(const std::string* d)
 
 save_info create_save_info::operator()(const std::string& filename) const
 {
-	std::string name = filename;
-	replace_underbar2space(name);
-
 	time_t modified = filesystem::file_modified_time(dir + "/" + filename);
-	save_index_manager.set_modified(name, modified);
-	return save_info(name, modified);
+	save_index_manager.set_modified(filename, modified);
+	return save_info(filename, modified);
 }
 
 void extract_summary_from_config(config& cfg_save, config& cfg_summary)

src/save_index.hpp

@@ -108,6 +108,3 @@ private:
 
 extern save_index_class save_index_manager;
 } // end of namespace savegame
-
-void replace_underbar2space(std::string& name);
-void replace_space2underbar(std::string& name);

src/savegame.cpp

@@ -44,6 +44,8 @@
 #include "version.hpp"
 #include "video.hpp"
 
+#include <algorithm>
+
 static lg::log_domain log_engine("engine");
 #define LOG_SAVE LOG_STREAM(info, log_engine)
 #define ERR_SAVE LOG_STREAM(err, log_engine)
@@ -54,14 +56,10 @@ static lg::log_domain log_enginerefac("enginerefac");
 
 namespace savegame {
 
-bool save_game_exists(const std::string& name, compression::format compressed)
+bool save_game_exists(std::string name, compression::format compressed)
 {
-	std::string fname = name;
-	replace_space2underbar(fname);
-
-	fname += compression::format_extension(compressed);
-
-	return filesystem::file_exists(filesystem::get_saves_dir() + "/" + fname);
+	name += compression::format_extension(compressed);
+	return filesystem::file_exists(filesystem::get_saves_dir() + "/" + name);
 }
 
 void clean_saves(const std::string& label)
@@ -428,6 +426,7 @@ std::string savegame::create_filename(unsigned int turn_number) const
 	std::string filename = create_initial_filename(turn_number);
 	filename.erase(std::remove_if(filename.begin(), filename.end(),
 	            is_illegal_file_char), filename.end());
+	std::replace(filename.begin(), filename.end(), '_', ' ');
 	return filename;
 }
 
@@ -527,11 +526,8 @@ void savegame::finish_save_game(const config_writer &out)
 // Throws game::save_game_failed
 filesystem::scoped_ostream savegame::open_save_game(const std::string &label)
 {
-	std::string name = label;
-	replace_space2underbar(name);
-
 	try {
-		return filesystem::ostream_file(filesystem::get_saves_dir() + "/" + name);
+		return filesystem::ostream_file(filesystem::get_saves_dir() + "/" + label);
 	} catch(filesystem::io_exception& e) {
 		throw game::save_game_failed(e.what());
 	}

src/savegame.hpp

@@ -30,7 +30,7 @@ namespace savegame {
 /** converts saves from older versions of wesnoth*/
 void convert_old_saves(config& cfg);
 /** Returns true if there is already a savegame with that name. */
-bool save_game_exists(const std::string& name, compression::format compressed);
+bool save_game_exists(std::string name, compression::format compressed);
 
 /** Delete all autosaves of a certain scenario. */
 void clean_saves(const std::string& label);