From 568cf1a02ff415b020bb1f17c3ca14b730d3a146 Mon Sep 17 00:00:00 2001 From: Joonas Rikkonen Date: Wed, 18 Jul 2018 14:23:43 +0300 Subject: [PATCH] Fixed memory leak caused by submarine preview images, changed Submarine.SavedSubmarines to a property that prevents removing submarines from outside the class without disposing the preview image. Closes #498 --- .../Source/Networking/GameClient.cs | 10 +- .../Source/Networking/Voting.cs | 3 +- .../Source/Screens/CampaignSetupUI.cs | 2 +- .../Source/Screens/NetLobbyScreen.cs | 8 +- .../BarotraumaClient/Source/Sprite/Sprite.cs | 10 +- .../Source/Screens/NetLobbyScreen.cs | 4 +- .../BarotraumaShared/Source/Map/Submarine.cs | 92 ++++++++++++------- .../Networking/FileTransfer/FileSender.cs | 2 +- .../Source/Networking/Voting.cs | 2 +- 9 files changed, 81 insertions(+), 52 deletions(-) diff --git a/Barotrauma/BarotraumaClient/Source/Networking/GameClient.cs b/Barotrauma/BarotraumaClient/Source/Networking/GameClient.cs index 47c0a4e7a..5e097dffd 100644 --- a/Barotrauma/BarotraumaClient/Source/Networking/GameClient.cs +++ b/Barotrauma/BarotraumaClient/Source/Networking/GameClient.cs @@ -815,7 +815,7 @@ namespace Barotrauma.Networking string subName = inc.ReadString(); string subHash = inc.ReadString(); - var matchingSub = Submarine.SavedSubmarines.Find(s => s.Name == subName && s.MD5Hash.Hash == subHash); + var matchingSub = Submarine.SavedSubmarines.FirstOrDefault(s => s.Name == subName && s.MD5Hash.Hash == subHash); if (matchingSub != null) { submarines.Add(matchingSub); @@ -1181,8 +1181,12 @@ namespace Barotrauma.Networking case FileTransferType.Submarine: new GUIMessageBox("Download finished", "File \"" + transfer.FileName + "\" was downloaded succesfully."); var newSub = new Submarine(transfer.FilePath); - Submarine.SavedSubmarines.RemoveAll(s => s.Name == newSub.Name && s.MD5Hash.Hash == newSub.MD5Hash.Hash); - Submarine.SavedSubmarines.Add(newSub); + var existingSubs = Submarine.SavedSubmarines.Where(s => s.Name == newSub.Name && s.MD5Hash.Hash == newSub.MD5Hash.Hash).ToList(); + foreach (Submarine existingSub in existingSubs) + { + existingSub.Dispose(); + } + Submarine.AddToSavedSubs(newSub); for (int i = 0; i < 2; i++) { diff --git a/Barotrauma/BarotraumaClient/Source/Networking/Voting.cs b/Barotrauma/BarotraumaClient/Source/Networking/Voting.cs index b0a6294bb..6b41a5dac 100644 --- a/Barotrauma/BarotraumaClient/Source/Networking/Voting.cs +++ b/Barotrauma/BarotraumaClient/Source/Networking/Voting.cs @@ -2,6 +2,7 @@ using Lidgren.Network; using Microsoft.Xna.Framework; using System.Collections.Generic; +using System.Linq; namespace Barotrauma { @@ -151,7 +152,7 @@ namespace Barotrauma { int votes = inc.ReadByte(); string subName = inc.ReadString(); - Submarine sub = Submarine.SavedSubmarines.Find(sm => sm.Name == subName); + Submarine sub = Submarine.SavedSubmarines.FirstOrDefault(sm => sm.Name == subName); SetVoteText(GameMain.NetLobbyScreen.SubList, sub, votes); } } diff --git a/Barotrauma/BarotraumaClient/Source/Screens/CampaignSetupUI.cs b/Barotrauma/BarotraumaClient/Source/Screens/CampaignSetupUI.cs index b477aee46..578ee4bf0 100644 --- a/Barotrauma/BarotraumaClient/Source/Screens/CampaignSetupUI.cs +++ b/Barotrauma/BarotraumaClient/Source/Screens/CampaignSetupUI.cs @@ -144,7 +144,7 @@ namespace Barotrauma return true; }; } - if (Submarine.SavedSubmarines.Count > 0) subList.Select(Submarine.SavedSubmarines[0]); + if (Submarine.SavedSubmarines.Any()) subList.Select(Submarine.SavedSubmarines.First()); } public void UpdateLoadMenu() diff --git a/Barotrauma/BarotraumaClient/Source/Screens/NetLobbyScreen.cs b/Barotrauma/BarotraumaClient/Source/Screens/NetLobbyScreen.cs index 6f5edabac..287edb042 100644 --- a/Barotrauma/BarotraumaClient/Source/Screens/NetLobbyScreen.cs +++ b/Barotrauma/BarotraumaClient/Source/Screens/NetLobbyScreen.cs @@ -782,8 +782,8 @@ namespace Barotrauma CanBeFocused = false }; - var matchingSub = Submarine.SavedSubmarines.Find(s => s.Name == sub.Name && s.MD5Hash.Hash == sub.MD5Hash.Hash); - if (matchingSub == null) matchingSub = Submarine.SavedSubmarines.Find(s => s.Name == sub.Name); + var matchingSub = Submarine.SavedSubmarines.FirstOrDefault(s => s.Name == sub.Name && s.MD5Hash.Hash == sub.MD5Hash.Hash); + if (matchingSub == null) matchingSub = Submarine.SavedSubmarines.FirstOrDefault(s => s.Name == sub.Name); if (matchingSub == null) { @@ -1467,8 +1467,8 @@ namespace Barotrauma return false; } - Submarine sub = Submarine.SavedSubmarines.Find(m => m.Name == subName && m.MD5Hash.Hash == md5Hash); - if (sub == null) sub = Submarine.SavedSubmarines.Find(m => m.Name == subName); + Submarine sub = Submarine.SavedSubmarines.FirstOrDefault(m => m.Name == subName && m.MD5Hash.Hash == md5Hash); + if (sub == null) sub = Submarine.SavedSubmarines.FirstOrDefault(m => m.Name == subName); var matchingListSub = subList.children.Find(c => c.UserData == sub); if (matchingListSub != null) diff --git a/Barotrauma/BarotraumaClient/Source/Sprite/Sprite.cs b/Barotrauma/BarotraumaClient/Source/Sprite/Sprite.cs index c0e65a76f..d09deb6b0 100644 --- a/Barotrauma/BarotraumaClient/Source/Sprite/Sprite.cs +++ b/Barotrauma/BarotraumaClient/Source/Sprite/Sprite.cs @@ -254,11 +254,14 @@ namespace Barotrauma partial void DisposeTexture() { //check if another sprite is using the same texture - foreach (Sprite s in list) + if (!string.IsNullOrEmpty(file)) //file can be empty if the sprite is created directly from a Texture2D instance { - if (s.file == file) return; + foreach (Sprite s in list) + { + if (s.file == file) return; + } } - + //if not, free the texture if (texture != null) { @@ -267,6 +270,5 @@ namespace Barotrauma } } } - } diff --git a/Barotrauma/BarotraumaServer/Source/Screens/NetLobbyScreen.cs b/Barotrauma/BarotraumaServer/Source/Screens/NetLobbyScreen.cs index 959d85b2c..79fffa92f 100644 --- a/Barotrauma/BarotraumaServer/Source/Screens/NetLobbyScreen.cs +++ b/Barotrauma/BarotraumaServer/Source/Screens/NetLobbyScreen.cs @@ -119,7 +119,7 @@ namespace Barotrauma subs = Submarine.SavedSubmarines.Where(s => !s.HasTag(SubmarineTag.HideInMenus)).ToList(); - if (subs == null || subs.Count()==0) + if (subs == null || subs.Count() == 0) { throw new Exception("No submarines are available."); } @@ -187,7 +187,7 @@ namespace Barotrauma if (GameMain.Server.SubSelectionMode == SelectionMode.Random) { - var nonShuttles = Submarine.SavedSubmarines.FindAll(c => !c.HasTag(SubmarineTag.Shuttle) && !c.HasTag(SubmarineTag.HideInMenus)); + var nonShuttles = Submarine.SavedSubmarines.Where(c => !c.HasTag(SubmarineTag.Shuttle) && !c.HasTag(SubmarineTag.HideInMenus)).ToList(); SelectedSub = nonShuttles[Rand.Range(0, nonShuttles.Count)]; } if (GameMain.Server.ModeSelectionMode == SelectionMode.Random) diff --git a/Barotrauma/BarotraumaShared/Source/Map/Submarine.cs b/Barotrauma/BarotraumaShared/Source/Map/Submarine.cs index 63277d538..e5ade2ae9 100644 --- a/Barotrauma/BarotraumaShared/Source/Map/Submarine.cs +++ b/Barotrauma/BarotraumaShared/Source/Map/Submarine.cs @@ -51,8 +51,12 @@ namespace Barotrauma public static bool LockX, LockY; - public static List SavedSubmarines = new List(); - + private static List savedSubmarines = new List(); + public static IEnumerable SavedSubmarines + { + get { return savedSubmarines; } + } + public static readonly Vector2 GridSize = new Vector2(16.0f, 16.0f); public static Submarine[] MainSubs = new Submarine[2]; @@ -214,10 +218,10 @@ namespace Barotrauma return ConvertUnits.ToSimUnits(Position); } } - + public Vector2 Velocity { - get { return subBody==null ? Vector2.Zero : subBody.Velocity; } + get { return subBody == null ? Vector2.Zero : subBody.Velocity; } set { if (subBody == null) return; @@ -244,7 +248,7 @@ namespace Barotrauma public override string ToString() { - return "Barotrauma.Submarine ("+name+")"; + return "Barotrauma.Submarine (" + name + ")"; } public override bool Removed @@ -344,7 +348,7 @@ namespace Barotrauma { if (dockedSub == this) continue; - Vector2 diff = dockedSub.Submarine == this ? dockedSub.WorldPosition : dockedSub.WorldPosition - WorldPosition; + Vector2 diff = dockedSub.Submarine == this ? dockedSub.WorldPosition : dockedSub.WorldPosition - WorldPosition; Rectangle dockedSubBorders = dockedSub.Borders; dockedSubBorders.Y -= dockedSubBorders.Height; @@ -383,15 +387,15 @@ namespace Barotrauma public Vector2 FindSpawnPos(Vector2 spawnPos) { Rectangle dockedBorders = GetDockedBorders(); - + int iterations = 0; bool wallTooClose = false; do { Rectangle worldBorders = new Rectangle( dockedBorders.X + (int)spawnPos.X, - dockedBorders.Y + (int)spawnPos.Y, - dockedBorders.Width, + dockedBorders.Y + (int)spawnPos.Y, + dockedBorders.Width, dockedBorders.Height); wallTooClose = false; @@ -507,8 +511,8 @@ namespace Barotrauma public Rectangle CalculateDimensions(bool onlyHulls = true) { - List entities = onlyHulls ? - Hull.hullList.FindAll(h => h.Submarine == this).Cast().ToList() : + List entities = onlyHulls ? + Hull.hullList.FindAll(h => h.Submarine == this).Cast().ToList() : MapEntity.mapEntityList.FindAll(me => me.Submarine == this); if (entities.Count == 0) return Rectangle.Empty; @@ -557,7 +561,7 @@ namespace Barotrauma } } - public static bool RectsOverlap(Rectangle rect1, Rectangle rect2, bool inclusive=true) + public static bool RectsOverlap(Rectangle rect1, Rectangle rect2, bool inclusive = true) { if (inclusive) { @@ -584,25 +588,25 @@ namespace Barotrauma { if (fixture == null || (ignoreSensors && fixture.IsSensor) || - fixture.CollisionCategories == Category.None || + fixture.CollisionCategories == Category.None || fixture.CollisionCategories == Physics.CollisionItem) return -1; - - if (collisionCategory != null && + + if (collisionCategory != null && !fixture.CollisionCategories.HasFlag((Category)collisionCategory) && !((Category)collisionCategory).HasFlag(fixture.CollisionCategories)) return -1; - + if (ignoredBodies != null && ignoredBodies.Contains(fixture.Body)) return -1; - - Structure structure = fixture.Body.UserData as Structure; + + Structure structure = fixture.Body.UserData as Structure; if (structure != null) { if (structure.IsPlatform && collisionCategory != null && !((Category)collisionCategory).HasFlag(Physics.CollisionPlatform)) return -1; - } + } if (fraction < closestFraction) { closestFraction = fraction; - if (fixture.Body!=null) closestBody = fixture.Body; + if (fixture.Body != null) closestBody = fixture.Body; } return fraction; } @@ -669,7 +673,7 @@ namespace Barotrauma get { return flippedX; } } - public void FlipX(List parents=null) + public void FlipX(List parents = null) { if (parents == null) parents = new List(); parents.Add(this); @@ -729,7 +733,7 @@ namespace Barotrauma { if (bodyItems.Contains(item)) { - item.Submarine = this; + item.Submarine = this; if (Position == Vector2.Zero) item.Move(-HiddenSubPosition); } else if (item.Submarine != this) @@ -751,7 +755,7 @@ namespace Barotrauma if (Level.Loaded == null || subBody == null) return; if (WorldPosition.Y < Level.MaxEntityDepth && - subBody.Body.Enabled && + subBody.Body.Enabled && (GameMain.NetworkMember?.RespawnManager == null || this != GameMain.NetworkMember.RespawnManager.RespawnShuttle)) { subBody.Body.ResetDynamics(); @@ -778,26 +782,26 @@ namespace Barotrauma } subBody.Body.LinearVelocity = new Vector2( - LockX ? 0.0f : subBody.Body.LinearVelocity.X, + LockX ? 0.0f : subBody.Body.LinearVelocity.X, LockY ? 0.0f : subBody.Body.LinearVelocity.Y); - - + + subBody.Update(deltaTime); - for (int i = 0; i < 2; i++ ) + for (int i = 0; i < 2; i++) { if (MainSubs[i] == null) continue; if (this != MainSubs[i] && MainSubs[i].DockedTo.Contains(this)) return; } //send updates more frequently if moving fast - networkUpdateTimer -= MathHelper.Clamp(Velocity.Length()*10.0f, 0.1f, 5.0f) * deltaTime; + networkUpdateTimer -= MathHelper.Clamp(Velocity.Length() * 10.0f, 0.1f, 5.0f) * deltaTime; if (networkUpdateTimer < 0.0f) { networkUpdateTimer = 1.0f; } - + } public void ApplyForce(Vector2 force) @@ -867,7 +871,7 @@ namespace Barotrauma subBorders.Inflate(500.0f, 500.0f); - if (subBorders.Contains(position)) return sub; + if (subBorders.Contains(position)) return sub; } return null; @@ -875,9 +879,18 @@ namespace Barotrauma //saving/loading ---------------------------------------------------- + public static void AddToSavedSubs(Submarine sub) + { + savedSubmarines.Add(sub); + } + public static void RefreshSavedSubs() { - SavedSubmarines.Clear(); + for (int i = savedSubmarines.Count - 1; i>= 0; i--) + { + savedSubmarines[i].Dispose(); + } + System.Diagnostics.Debug.Assert(savedSubmarines.Count == 0); if (!Directory.Exists(SavePath)) { @@ -921,7 +934,7 @@ namespace Barotrauma foreach (string path in filePaths) { - SavedSubmarines.Add(new Submarine(path)); + savedSubmarines.Add(new Submarine(path)); } } @@ -971,7 +984,7 @@ namespace Barotrauma catch (Exception e) { - DebugConsole.ThrowError("Loading submarine \"" + file + "\" failed! ("+e.Message+")"); + DebugConsole.ThrowError("Loading submarine \"" + file + "\" failed! (" + e.Message + ")"); return null; } } @@ -1147,12 +1160,12 @@ namespace Barotrauma Submarine sub = new Submarine(element.GetAttributeString("name", ""), "", false); sub.Load(unloadPrevious, element); - return sub; + return sub; } public static Submarine Load(string fileName, bool unloadPrevious) { - return Load(fileName, SavePath, unloadPrevious); + return Load(fileName, SavePath, unloadPrevious); } public static Submarine Load(string fileName, string folder, bool unloadPrevious) @@ -1285,6 +1298,15 @@ namespace Barotrauma DockedTo.Clear(); } + public void Dispose() + { + savedSubmarines.Remove(this); +#if CLIENT + PreviewImage.Remove(); + PreviewImage = null; +#endif + } + public void ServerWrite(NetBuffer msg, Client c, object[] extraData = null) { msg.Write(ID); diff --git a/Barotrauma/BarotraumaShared/Source/Networking/FileTransfer/FileSender.cs b/Barotrauma/BarotraumaShared/Source/Networking/FileTransfer/FileSender.cs index f119d3f26..3ce6c9153 100644 --- a/Barotrauma/BarotraumaShared/Source/Networking/FileTransfer/FileSender.cs +++ b/Barotrauma/BarotraumaShared/Source/Networking/FileTransfer/FileSender.cs @@ -271,7 +271,7 @@ namespace Barotrauma.Networking case (byte)FileTransferType.Submarine: string fileName = inc.ReadString(); string fileHash = inc.ReadString(); - var requestedSubmarine = Submarine.SavedSubmarines.Find(s => s.Name == fileName && s.MD5Hash.Hash == fileHash); + var requestedSubmarine = Submarine.SavedSubmarines.FirstOrDefault(s => s.Name == fileName && s.MD5Hash.Hash == fileHash); if (requestedSubmarine != null) { diff --git a/Barotrauma/BarotraumaShared/Source/Networking/Voting.cs b/Barotrauma/BarotraumaShared/Source/Networking/Voting.cs index c78a1bede..300198353 100644 --- a/Barotrauma/BarotraumaShared/Source/Networking/Voting.cs +++ b/Barotrauma/BarotraumaShared/Source/Networking/Voting.cs @@ -93,7 +93,7 @@ namespace Barotrauma { case VoteType.Sub: string subName = inc.ReadString(); - Submarine sub = Submarine.SavedSubmarines.Find(s => s.Name == subName); + Submarine sub = Submarine.SavedSubmarines.FirstOrDefault(s => s.Name == subName); sender.SetVote(voteType, sub); #if CLIENT UpdateVoteTexts(GameMain.Server.ConnectedClients, voteType);