code review

Mods/vcmi/config/english.json

@@ -106,7 +106,7 @@
 	"vcmi.lobby.handicap.resource" : "Gives players appropriate resources to start with in addition to the normal starting resources. Negative values are allowed, but are limited to 0 in total (the player never starts with negative resources).",
 	"vcmi.lobby.handicap.income" : "Changes the player's various incomes by the percentage. Is rounded up.",
 	"vcmi.lobby.handicap.growth" : "Changes the growth rate of creatures in the towns owned by the player. Is rounded up.",
-	"vcmi.lobby.deleteUnsupportedSave" : "Unsupported saves found (e.g. from previous versions).\n\nDelete them?",
+	"vcmi.lobby.deleteUnsupportedSave" : "{Unsupported saves found}\n\nVCMI has found %d saved games that are no longer supported, possibly due to differences in VCMI versions.\n\nDo you want to delete them?",
 	"vcmi.lobby.deleteSaveGameTitle" : "Select a Saved Game to delete",
 	"vcmi.lobby.deleteMapTitle" : "Select a Scenario to delete",
 	"vcmi.lobby.deleteFile" : "Do you want to delete following file?",

client/lobby/SelectionTab.cpp

@@ -282,9 +282,12 @@ void SelectionTab::toggleMode()
 			}
 
 		case ESelectionScreen::loadGame:
+			{
 				inputName->disable();
-			parseSaves(getFiles("Saves/", EResType::SAVEGAME));
+				auto unsupported = parseSaves(getFiles("Saves/", EResType::SAVEGAME));
+				handleUnsupportedSavegames(unsupported);
 				break;
+			}
 
 		case ESelectionScreen::saveGame:
 			parseSaves(getFiles("Saves/", EResType::SAVEGAME));
@@ -876,16 +879,9 @@ void SelectionTab::parseMaps(const std::unordered_set<ResourcePath> & files)
 	}
 }
 
-void SelectionTab::parseSaves(const std::unordered_set<ResourcePath> & files)
+std::vector<ResourcePath> SelectionTab::parseSaves(const std::unordered_set<ResourcePath> & files)
 {
-	enum Choice { NONE, REMAIN, DELETE };
+	std::vector<ResourcePath> unsupported;
-	Choice deleteUnsupported = NONE;
-	auto doDeleteUnsupported = [](std::string file){
-		LobbyDelete ld;
-		ld.type = LobbyDelete::SAVEGAME;
-		ld.name = file;
-		CSH->sendLobbyPack(ld);
-	};
 
 	for(auto & file : files)
 	{
@@ -925,31 +921,35 @@ void SelectionTab::parseSaves(const std::unordered_set<ResourcePath> & files)
 
 			allItems.push_back(mapInfo);
 		}
-		catch(const std::exception & e)
+		catch(const IdentifierResolutionException & e)
 		{
-			// asking for deletion of unsupported saves
-			if(CSH->isHost())
-			{
-				if(deleteUnsupported == DELETE)
-					doDeleteUnsupported(file.getName());
-				else if(deleteUnsupported == NONE)
-				{
-					CInfoWindow::showYesNoDialog(CGI->generaltexth->translate("vcmi.lobby.deleteUnsupportedSave"), std::vector<std::shared_ptr<CComponent>>(), [&deleteUnsupported, doDeleteUnsupported, file](){
-						doDeleteUnsupported(file.getName());
-						deleteUnsupported = DELETE;
-					}, [&deleteUnsupported](){ deleteUnsupported = REMAIN; });
-
-					while(deleteUnsupported == NONE)
-					{
-						auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
-						boost::this_thread::sleep_for(boost::chrono::milliseconds(5));
-					}
-				}
-			}
-
-			if(deleteUnsupported != DELETE)
 			logGlobal->error("Error: Failed to process %s: %s", file.getName(), e.what());
 		}
+		catch(const std::exception & e)
+		{
+			unsupported.push_back(file); // IdentifierResolutionException is not relevant -> not ask to delete, when mods are disabled
+			logGlobal->error("Error: Failed to process %s: %s", file.getName(), e.what());
+		}
+	}
+
+	return unsupported;
+}
+
+void SelectionTab::handleUnsupportedSavegames(const std::vector<ResourcePath> files)
+{
+	if(CSH->isHost() && files.size())
+	{
+		MetaString text = MetaString::createFromTextID("vcmi.lobby.deleteUnsupportedSave");
+		text.replaceNumber(files.size());
+		CInfoWindow::showYesNoDialog(text.toString(), std::vector<std::shared_ptr<CComponent>>(), [files](){
+			for(auto & file : files)
+			{
+				LobbyDelete ld;
+				ld.type = LobbyDelete::SAVEGAME;
+				ld.name = file.getName();
+				CSH->sendLobbyPack(ld);
+			}
+		}, nullptr);
 	}
 }
 

client/lobby/SelectionTab.h

@@ -72,6 +72,8 @@ class SelectionTab : public CIntObject
 	// FIXME: CSelectionBase use them too!
 	std::shared_ptr<CAnimation> iconsVictoryCondition;
 	std::shared_ptr<CAnimation> iconsLossCondition;
+
+	std::vector<std::shared_ptr<ListItem>> unSupportedSaves;
 public:
 	std::vector<std::shared_ptr<ElementInfo>> allItems;
 	std::vector<std::shared_ptr<ElementInfo>> curItems;
@@ -127,7 +129,9 @@ private:
 
 	bool isMapSupported(const CMapInfo & info);
 	void parseMaps(const std::unordered_set<ResourcePath> & files);
-	void parseSaves(const std::unordered_set<ResourcePath> & files);
+	std::vector<ResourcePath> parseSaves(const std::unordered_set<ResourcePath> & files);
 	void parseCampaigns(const std::unordered_set<ResourcePath> & files);
 	std::unordered_set<ResourcePath> getFiles(std::string dirURI, EResType resType);
+
+	void handleUnsupportedSavegames(const std::vector<ResourcePath> files);
 };

lib/networkPacks/PacksForLobby.h

@@ -177,7 +177,6 @@ struct DLL_LINKAGE LobbyUpdateState : public CLobbyPackToPropagate
 	template <typename Handler> void serialize(Handler &h)
 	{
 		h & state;
-		if (h.version >= Handler::Version::LOBBY_DELETE)
 		h & refreshList;
 	}
 };

lib/serializer/ESerializationVersion.h

@@ -67,7 +67,6 @@ enum class ESerializationVersion : int32_t
 	REMOVE_OBJECT_TYPENAME, // 868 - remove typename from CGObjectInstance
 	REMOVE_VLC_POINTERS, // 869 removed remaining pointers to VLC entities
 	FOLDER_NAME_REWORK, // 870 - rework foldername
-	LOBBY_DELETE, // 871 - possibility to delete savegames and random maps
 	
-	CURRENT = LOBBY_DELETE
+	CURRENT = FOLDER_NAME_REWORK
 };