diff --git a/DXMainClient/DXGUI/Multiplayer/GameLobby/CnCNetGameLobby.cs b/DXMainClient/DXGUI/Multiplayer/GameLobby/CnCNetGameLobby.cs index 92d550c83..4311d11a9 100644 --- a/DXMainClient/DXGUI/Multiplayer/GameLobby/CnCNetGameLobby.cs +++ b/DXMainClient/DXGUI/Multiplayer/GameLobby/CnCNetGameLobby.cs @@ -95,7 +95,7 @@ public CnCNetGameLobby(WindowManager windowManager, string iniName, "Change the used CnCNet tunnel server (game host only)".L10N("UI:Main:ChangeTunnel"), true, (s) => ShowTunnelSelectionWindow("Select tunnel server:".L10N("UI:Main:SelectTunnelServer")))); AddChatBoxCommand(new ChatBoxCommand("DOWNLOADMAP", - "Download a map from CNCNet's map server using a map ID and an optional filename.\nExample: \"/downloadmap MAPID [2] My Battle Map\"".L10N("UI:Main:DownloadMapCommandDescription"), + "Download a map from CNCNet's map server using a map ID and an optional filename.\nYou can find maps at https://mapdb.cncnet.org/search/\nExample: \"/downloadmap MAPID [2] My Battle Map\"".L10N("UI:Main:DownloadMapCommandDescription"), false, DownloadMapByIdCommand)); } @@ -1596,31 +1596,22 @@ private void MapSharer_HandleMapDownloadComplete(SHA1EventArgs e) string mapFileName = MapSharer.GetMapFileName(e.SHA1, e.MapName); Logger.Log("Map " + mapFileName + " downloaded, parsing."); string mapPath = MapLoader.CustomMapsDirectory + mapFileName; - Map map = MapLoader.LoadCustomMap(mapPath, out string returnMessage); - if (map != null) - { - AddNotice(returnMessage); - if (lastMapSHA1 == e.SHA1) - { - GameModeMap = MapLoader.GetLoadedMapBySha1(lastMapSHA1); - ChangeMap(GameModeMap); - } - } - else if (chatCommandDownloadedMaps.Contains(e.SHA1)) - { - // Somehow the user has managed to download an already existing sha1 hash. - // This special case prevents user confusion from the file successfully downloading but showing an error anyway. - AddNotice(returnMessage, Color.Yellow); - AddNotice("Map was downloaded, but a duplicate is already loaded from a different filename. This may cause strange behavior.".L10N("UI:Main:DownloadMapCommandDuplicateMapFileLoaded"), - Color.Yellow); - } - else + GameModeMap gameModeMap = MapLoader.GetLoadedMapBySha1(e.SHA1); + + if (gameModeMap == null) { - AddNotice(returnMessage, Color.Red); + AddNotice($"Failed to download map {e.SHA1}", Color.Red); AddNotice("Transfer of the custom map failed. The host needs to change the map or you will be unable to participate in this match.".L10N("UI:Main:MapTransferFailed")); mapSharingConfirmationPanel.SetFailedStatus(); channel.SendCTCPMessage(MAP_SHARING_FAIL_MESSAGE + " " + e.SHA1, QueuedMessageType.SYSTEM_MESSAGE, 9); } + + AddNotice($"Map {gameModeMap.Map.Name} loaded succesfully."); + if (lastMapSHA1 == e.SHA1) + { + GameModeMap = MapLoader.GetLoadedMapBySha1(lastMapSHA1); + ChangeMap(GameModeMap); + } } private void MapSharer_MapUploadFailed(object sender, MapEventArgs e) => @@ -1789,7 +1780,7 @@ private void DownloadMapByIdCommand(string parameters) { // The user did not supply a map name. sha1 = parameters; - mapName = "user_chat_command_download"; + mapName = MapLoader.MAP_CHAT_COMMAND_FILENAME_PREFIX; } else { diff --git a/DXMainClient/DXGUI/Multiplayer/GameLobby/GameLobbyBase.cs b/DXMainClient/DXGUI/Multiplayer/GameLobby/GameLobbyBase.cs index 7bb737f5b..77f7ba203 100644 --- a/DXMainClient/DXGUI/Multiplayer/GameLobby/GameLobbyBase.cs +++ b/DXMainClient/DXGUI/Multiplayer/GameLobby/GameLobbyBase.cs @@ -2325,14 +2325,11 @@ public bool LoadGameOptionPreset(string name) /// /// Handle the GameModeMapsUpdated event from the MapLoader. /// - /// Updates the gamemode dropdown for new maps being added while the client is running + /// Updates the gamemode dropdown for new maps being added while the client is running. /// /// /// - private void MapLoader_GameModeMapsUpdated(object sender, MapLoaderEventArgs e) - { - RefreshGameModeDropdown(); - } + private void MapLoader_GameModeMapsUpdated(object sender, MapLoaderEventArgs e) => RefreshGameModeDropdown(); /// /// Update the gamemode dropdown. @@ -2357,7 +2354,7 @@ public void RefreshGameModeDropdown() // Add any new game modes. foreach (GameMode gm in GameModeMaps.GameModes) { - //skip the game mode if it is already in the dropdown. + // Skip the game mode if it is already in the dropdown. if (existingDdGameModes.Contains(gm.UIName)) continue; diff --git a/DXMainClient/Domain/Multiplayer/CnCNet/MapSharer.cs b/DXMainClient/Domain/Multiplayer/CnCNet/MapSharer.cs index 870dee82d..7f07835fc 100644 --- a/DXMainClient/Domain/Multiplayer/CnCNet/MapSharer.cs +++ b/DXMainClient/Domain/Multiplayer/CnCNet/MapSharer.cs @@ -365,18 +365,27 @@ public static string GetMapFileName(string sha1, string mapName) private static string DownloadMain(string sha1, string myGame, string mapName, out bool success) { - string customMapsDirectory = SafePath.CombineDirectoryPath(ProgramConstants.GamePath, "Maps", "Custom"); + string customMapsPath = SafePath.CombineDirectoryPath(ProgramConstants.GamePath, "Maps", "Custom"); + string tempDownloadPath = SafePath.CombineDirectoryPath(ProgramConstants.GamePath, "Maps", "temp"); string mapFileName = GetMapFileName(sha1, mapName); - FileInfo destinationFile = SafePath.GetFile(customMapsDirectory, FormattableString.Invariant($"{mapFileName}.zip")); + // Store the unzipped mapfile in a temp directory while we unzip and rename it to avoid a race condition with the map file watcher + DirectoryInfo tempDownloadDirectory = Directory.CreateDirectory(tempDownloadPath); + FileInfo zipDestinationFile = SafePath.GetFile(tempDownloadDirectory.FullName, FormattableString.Invariant($"{mapFileName}.zip")); + FileInfo tempMapFile = SafePath.GetFile(tempDownloadDirectory.FullName, FormattableString.Invariant($"{mapFileName}{MapLoader.MAP_FILE_EXTENSION}")); - // This string is up here so we can check that there isn't already a .map file for this download. - // This prevents the client from crashing when trying to rename the unzipped file to a duplicate filename. - FileInfo newFile = SafePath.GetFile(customMapsDirectory, FormattableString.Invariant($"{mapFileName}{MapLoader.MAP_FILE_EXTENSION}")); + if (zipDestinationFile.Exists) + { + Logger.Log($"DownloadMain: zipDestinationFile already exists, deleting: zipDestinationFile={zipDestinationFile.FullName}"); + zipDestinationFile.Delete(); + } - destinationFile.Delete(); - newFile.Delete(); + if (tempMapFile.Exists) + { + Logger.Log($"DownloadMain: tempMapFile already exists, deleting: tempMapFile={zipDestinationFile.FullName}"); + tempMapFile.Delete(); + } using (TWebClient webClient = new TWebClient()) { @@ -385,7 +394,7 @@ private static string DownloadMain(string sha1, string myGame, string mapName, o try { Logger.Log("MapSharer: Downloading URL: " + "http://mapdb.cncnet.org/" + myGame + "/" + sha1 + ".zip"); - webClient.DownloadFile("http://mapdb.cncnet.org/" + myGame + "/" + sha1 + ".zip", destinationFile.FullName); + webClient.DownloadFile("http://mapdb.cncnet.org/" + myGame + "/" + sha1 + ".zip", zipDestinationFile.FullName); } catch (Exception ex) { @@ -404,15 +413,15 @@ private static string DownloadMain(string sha1, string myGame, string mapName, o } } - destinationFile.Refresh(); + zipDestinationFile.Refresh(); - if (!destinationFile.Exists) + if (!zipDestinationFile.Exists) { success = false; return null; } - string extractedFile = ExtractZipFile(destinationFile.FullName, customMapsDirectory); + string extractedFile = ExtractZipFile(zipDestinationFile.FullName, tempDownloadDirectory.FullName); if (String.IsNullOrEmpty(extractedFile)) { @@ -420,11 +429,19 @@ private static string DownloadMain(string sha1, string myGame, string mapName, o return null; } - // We can safely assume that there will not be a duplicate file due to deleting it - // earlier if one already existed. - File.Move(SafePath.CombineFilePath(customMapsDirectory, extractedFile), newFile.FullName); + FileInfo newMapFile = SafePath.GetFile(customMapsPath, FormattableString.Invariant($"{mapFileName}{MapLoader.MAP_FILE_EXTENSION}")); + + // We need to delete potentially conflicting map files because .Move won't overwrite. + if (newMapFile.Exists) + { + Logger.Log($"DownloadMain: newMapFile already exists, deleting: newMapFile={newMapFile.FullName}"); + newMapFile.Delete(); + } + + File.Move(SafePath.CombineFilePath(tempDownloadDirectory.FullName, extractedFile), newMapFile.FullName); - destinationFile.Delete(); + zipDestinationFile.Delete(); + Directory.Delete(tempDownloadDirectory.FullName, true); success = true; return extractedFile; diff --git a/DXMainClient/Domain/Multiplayer/MapLoader.cs b/DXMainClient/Domain/Multiplayer/MapLoader.cs index b826ede6f..51fc29e75 100644 --- a/DXMainClient/Domain/Multiplayer/MapLoader.cs +++ b/DXMainClient/Domain/Multiplayer/MapLoader.cs @@ -14,6 +14,7 @@ namespace DTAClient.Domain.Multiplayer public class MapLoader { public const string MAP_FILE_EXTENSION = ".map"; + public const string MAP_CHAT_COMMAND_FILENAME_PREFIX = "user_chat_command_download"; private const string CUSTOM_MAPS_DIRECTORY = "Maps/Custom"; private static readonly string CUSTOM_MAPS_CACHE = SafePath.CombineFilePath(ProgramConstants.ClientUserFilesPath, "custom_map_cache"); private const string MultiMapsSection = "MultiMaps"; @@ -64,20 +65,14 @@ public class MapLoader /// /// The map ID to search the loaded maps for. /// - public bool IsMapAlreadyLoaded(string sha1) - { - return GetLoadedMapBySha1(sha1) != null; - } + public bool IsMapAlreadyLoaded(string sha1) => GetLoadedMapBySha1(sha1) != null; /// /// Search the loaded maps for the SHA-1, return the map if a match is found. /// /// The map ID to search the loaded maps for. /// The map matching the SHA-1 if one was found. - public GameModeMap GetLoadedMapBySha1(string sha1) - { - return GameModeMaps.Find(gmm => gmm.Map.SHA1 == sha1); - } + public GameModeMap GetLoadedMapBySha1(string sha1) => GameModeMaps.Find(gmm => gmm.Map.SHA1 == sha1); /// /// Loads multiplayer map info asynchonously. @@ -115,16 +110,16 @@ public void StartCustomMapFileWatcher() /// /// Sent by the file system watcher /// Sent by the file system watcher - public void HandleCustomMapFolder_Created(object sender, FileSystemEventArgs e) + private void HandleCustomMapFolder_Created(object sender, FileSystemEventArgs e) { // Get the map filename without the extension. // The extension gets added in LoadCustomMap so we need to excise it to avoid "file.map.map". string name = Path.GetFileNameWithoutExtension(e.Name); - if (name.StartsWith()) - string relativeMapPath = SafePath.CombineFilePath(CustomMapsDirectory, name); + Logger.Log($"HandleCustomMapFolder_Created: Calling LoadCustomMap: mapPath={relativeMapPath}"); Map map = LoadCustomMap(relativeMapPath, out string result); + Logger.Log($"HandleCustomMapFolder_Created: Ended LoadCustomMap: mapPath={relativeMapPath}"); if (map == null) { @@ -135,11 +130,11 @@ public void HandleCustomMapFolder_Created(object sender, FileSystemEventArgs e) /// /// Handle a .map file being removed from the custom map directory. /// - /// This function will attempt to remove the map from the client if it was deleted from the folder + /// This function will attempt to remove the map from the client if it was deleted from the folder. /// - /// Sent by the file system watcher + /// Sent by the file system watcher. /// Sent by the file system watcher. - public void HandleCustomMapFolder_Deleted(object sender, FileSystemEventArgs e) + private void HandleCustomMapFolder_Deleted(object sender, FileSystemEventArgs e) { Logger.Log($"Map was deleted: map={e.Name}"); // Use the filename without the extension so we can remove maps that had their extension changed. @@ -165,7 +160,7 @@ public void HandleCustomMapFolder_Deleted(object sender, FileSystemEventArgs e) /// /// /// - public void HandleCustomMapFolder_Renamed(object sender, RenamedEventArgs e) + private void HandleCustomMapFolder_Renamed(object sender, RenamedEventArgs e) { string name = Path.GetFileNameWithoutExtension(e.Name); string relativeMapPath = SafePath.CombineFilePath(CustomMapsDirectory, name); @@ -176,27 +171,30 @@ public void HandleCustomMapFolder_Renamed(object sender, RenamedEventArgs e) // This is just for logging to help debug. if (!oldPathIsMap && newPathIsMap) { - Logger.Log($"Renaming file changed the file extension. User is likely renaming a '.yrm' from Final Alert 2: old={e.OldName}, new={e.Name}"); + Logger.Log($"HandleCustomMapFolder_Renamed: Changed the file extension. User is likely renaming a '.yrm' from Final Alert 2: old={e.OldName}, new={e.Name}"); } else if (oldPathIsMap && !newPathIsMap) { // A bit hacky, but this is a rare case. - Logger.Log($"Renaming file changed the file extension to no longer be '.map' for some reason, removing from map list: old={e.OldName}, new={e.Name}"); + Logger.Log($"HandleCustomMapFolder_Renamed: Changed the file extension to no longer be '.map' for some reason, removing from map list: old={e.OldName}, new={e.Name}"); HandleCustomMapFolder_Deleted(sender, e); + return; } - - if (!newPathIsMap) + else if (!newPathIsMap) { - Logger.Log($"Renaming file. New extension is not '{MAP_FILE_EXTENSION}', moving on: file={e.Name}"); + Logger.Log($"HandleCustomMapFolder_Renamed: New extension is not '{MAP_FILE_EXTENSION}', moving on: file={e.Name}"); return; } Map map = LoadCustomMap(relativeMapPath, out string result); - if (map == null) + if (map != null) { - Logger.Log($"Failed to load renamed map file. Map is likely already loaded: original={e.OldName}, new={e.Name}, reason={result}"); + Logger.Log($"HandleCustomMapFolder_Renamed: Loaded renamed file as map: file={e.Name}, mapName={map.Name}"); + return; } + + Logger.Log($"Failed to load renamed map file. Map is likely already loaded, filepath: original={e.OldName}, new={e.Name}, reason={result}"); } /// @@ -206,7 +204,7 @@ public void HandleCustomMapFolder_Renamed(object sender, RenamedEventArgs e) /// /// /// - public void HandleCustomMapFolder_Error(object sender, ErrorEventArgs e) + private void HandleCustomMapFolder_Error(object sender, ErrorEventArgs e) { Exception exc = e.GetException(); Logger.Log($"The custom map folder file watcher crashed: error={exc.Message}"); @@ -445,7 +443,7 @@ public Map LoadCustomMap(string mapPath, out string resultMessage) // This checks the SHA-1, so duplicate maps in two .map files with different filenames can still be detected. if (IsMapAlreadyLoaded(map.SHA1)) { - Logger.Log("LoadCustomMap: Custom map " + customMapFile.FullName + " is already loaded!"); + Logger.Log($"LoadCustomMap: Custom map {customMapFile.FullName} is already loaded! SHA1={map.SHA1}"); resultMessage = $"Map {customMapFile.FullName} is already loaded."; return null; diff --git a/DXMainClient/Domain/Multiplayer/MapLoaderEventArgs.cs b/DXMainClient/Domain/Multiplayer/MapLoaderEventArgs.cs index 6dbdef636..822d9886f 100644 --- a/DXMainClient/Domain/Multiplayer/MapLoaderEventArgs.cs +++ b/DXMainClient/Domain/Multiplayer/MapLoaderEventArgs.cs @@ -13,6 +13,5 @@ public MapLoaderEventArgs(Map map) } public Map Map { get; private set; } - } } \ No newline at end of file