From 50603b72f4546bb1ea30c0a5655537aa9a767acb Mon Sep 17 00:00:00 2001 From: Joonas Rikkonen Date: Thu, 26 Jul 2018 12:41:54 +0300 Subject: [PATCH] Fixed another bug in DockingPort that caused entity ID mismatches. Even though the server sent the IDs of the CURRENT hulls and gap of the docking port, they are not necessarily created in the correct order during midround syncing and may end up replacing the ID of another entity or another entity spawned after them may cause their IDs to be replaced. Closes #530 --- .../Source/Items/Components/DockingPort.cs | 59 ++++--------------- .../BarotraumaShared/Source/Map/Entity.cs | 6 +- .../BarotraumaShared/Source/Map/Hull.cs | 2 +- .../NetEntityEvent/NetEntityEventManager.cs | 4 +- 4 files changed, 21 insertions(+), 50 deletions(-) diff --git a/Barotrauma/BarotraumaShared/Source/Items/Components/DockingPort.cs b/Barotrauma/BarotraumaShared/Source/Items/Components/DockingPort.cs index f3137e284..b1244bcf8 100644 --- a/Barotrauma/BarotraumaShared/Source/Items/Components/DockingPort.cs +++ b/Barotrauma/BarotraumaShared/Source/Items/Components/DockingPort.cs @@ -28,17 +28,13 @@ namespace Barotrauma.Items.Components private Joint joint; private readonly Hull[] hulls = new Hull[2]; - private ushort?[] hullIds; + private Gap gap; private Door door; private Body[] bodies; - private Body doorBody; - private Gap gap; - private ushort? gapId; - private bool docked; public int DockingDir @@ -114,9 +110,7 @@ namespace Barotrauma.Items.Components } IsActive = true; - - hullIds = new ushort?[2]; - + list.Add(this); } @@ -407,7 +401,7 @@ namespace Barotrauma.Items.Components hullRects[i].Location -= MathUtils.ToPoint((subs[i].WorldPosition - subs[i].HiddenSubPosition)); hulls[i] = new Hull(MapEntityPrefab.Find("Hull"), hullRects[i], subs[i]); hulls[i].AddToGrid(subs[i]); - if (hullIds[i] != null) hulls[i].ID = (ushort)hullIds[i]; + hulls[i].FreeID(); for (int j = 0; j < 2; j++) { @@ -418,7 +412,6 @@ namespace Barotrauma.Items.Components } gap = new Gap(new Rectangle(hullRects[0].Right - 2, hullRects[0].Y, 4, hullRects[0].Height), true, subs[0]); - if (gapId != null) gap.ID = (ushort)gapId; } else { @@ -436,25 +429,22 @@ namespace Barotrauma.Items.Components hullRects[i].Location -= MathUtils.ToPoint((subs[i].WorldPosition - subs[i].HiddenSubPosition)); hulls[i] = new Hull(MapEntityPrefab.Find("Hull"), hullRects[i], subs[i]); hulls[i].AddToGrid(subs[i]); - if (hullIds[i] != null) hulls[i].ID = (ushort)hullIds[i]; + hulls[i].FreeID(); } gap = new Gap(new Rectangle(hullRects[0].X, hullRects[0].Y+2, hullRects[0].Width, 4), false, subs[0]); - if (gapId != null) gap.ID = (ushort)gapId; } LinkHullsToGap(); - - hullIds[0] = hulls[0].ID; - hullIds[1] = hulls[1].ID; + hulls[0].ShouldBeSaved = false; hulls[1].ShouldBeSaved = false; item.linkedTo.Add(hulls[0]); item.linkedTo.Add(hulls[1]); + gap.FreeID(); gap.DisableHullRechecks = true; gap.ShouldBeSaved = false; - gapId = gap.ID; item.linkedTo.Add(gap); foreach (Body body in bodies) @@ -569,11 +559,7 @@ namespace Barotrauma.Items.Components gap.Remove(); gap = null; } - - hullIds[0] = null; - hullIds[1] = null; - gapId = null; - + if (bodies != null) { foreach (Body body in bodies) @@ -636,6 +622,9 @@ namespace Barotrauma.Items.Components protected override void RemoveComponentSpecific() { list.Remove(this); + hulls[0]?.Remove(); hulls[0] = null; + hulls[1]?.Remove(); hulls[1] = null; + gap?.Remove(); gap = null; } public override void OnMapLoaded() @@ -724,19 +713,8 @@ namespace Barotrauma.Items.Components if (docked) { - msg.Write(dockingTarget.item.ID); - - if (hulls != null && hulls[0] != null && hulls[1] != null && gap != null) - { - msg.Write(true); - msg.Write(hulls[0].ID); - msg.Write(hulls[1].ID); - msg.Write(gap.ID); - } - else - { - msg.Write(false); - } + msg.Write(dockingTarget.item.ID); + msg.Write(hulls != null && hulls[0] != null && hulls[1] != null && gap != null); } } @@ -764,14 +742,7 @@ namespace Barotrauma.Items.Components ushort dockingTargetID = msg.ReadUInt16(); bool isLocked = msg.ReadBoolean(); - - if (isLocked) - { - hullIds[0] = msg.ReadUInt16(); - hullIds[1] = msg.ReadUInt16(); - gapId = msg.ReadUInt16(); - } - + Entity targetEntity = Entity.FindEntityByID(dockingTargetID); if (targetEntity == null || !(targetEntity is Item)) { @@ -791,10 +762,6 @@ namespace Barotrauma.Items.Components if (isLocked) { Lock(true); - - hulls[0].ID = (ushort)hullIds[0]; - hulls[1].ID = (ushort)hullIds[1]; - gap.ID = (ushort)gapId; } } else diff --git a/Barotrauma/BarotraumaShared/Source/Map/Entity.cs b/Barotrauma/BarotraumaShared/Source/Map/Entity.cs index def927532..8490e6113 100644 --- a/Barotrauma/BarotraumaShared/Source/Map/Entity.cs +++ b/Barotrauma/BarotraumaShared/Source/Map/Entity.cs @@ -28,6 +28,11 @@ namespace Barotrauma private set; } + public bool IdFreed + { + get { return idFreed; } + } + public ushort ID { get @@ -230,7 +235,6 @@ namespace Barotrauma } dictionary.Remove(ID); - id = 0; idFreed = true; } diff --git a/Barotrauma/BarotraumaShared/Source/Map/Hull.cs b/Barotrauma/BarotraumaShared/Source/Map/Hull.cs index a74c65fd7..4f2fb775f 100644 --- a/Barotrauma/BarotraumaShared/Source/Map/Hull.cs +++ b/Barotrauma/BarotraumaShared/Source/Map/Hull.cs @@ -406,7 +406,7 @@ namespace Barotrauma if (Math.Abs(lastSentVolume - waterVolume) > Volume * 0.1f || Math.Abs(lastSentOxygen - OxygenPercentage) > 5f) { - if (GameMain.Server != null) + if (GameMain.Server != null && !IdFreed) { sendUpdateTimer -= deltaTime; if (sendUpdateTimer < 0.0f) diff --git a/Barotrauma/BarotraumaShared/Source/Networking/NetEntityEvent/NetEntityEventManager.cs b/Barotrauma/BarotraumaShared/Source/Networking/NetEntityEvent/NetEntityEventManager.cs index eafb6fb1b..65f820231 100644 --- a/Barotrauma/BarotraumaShared/Source/Networking/NetEntityEvent/NetEntityEventManager.cs +++ b/Barotrauma/BarotraumaShared/Source/Networking/NetEntityEvent/NetEntityEventManager.cs @@ -56,9 +56,9 @@ namespace Barotrauma.Networking GameAnalyticsSDK.Net.EGAErrorSeverity.Error, "Too much data in network event for entity \"" + e.Entity.ToString() + "\" (" + tempEventBuffer.LengthBytes + " bytes"); } - + //the ID has been taken by another entity (the original entity has been removed) -> write an empty event - if (Entity.FindEntityByID(e.Entity.ID) != e.Entity) + else if (Entity.FindEntityByID(e.Entity.ID) != e.Entity || e.Entity.IdFreed) { //technically the clients don't have any use for these, but removing events and shifting the IDs of all //consecutive ones is so error-prone that I think this is a safer option