From a726b5736775b4e11b22a4ebd71794a991eae6ad Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Fri, 19 Jan 2018 20:24:52 +0000 Subject: [PATCH] Fix order serialization issues. --- OpenRA.Game/Network/Order.cs | 43 +++++++++----- OpenRA.Game/Network/OrderIO.cs | 8 +++ OpenRA.Game/Traits/Target.cs | 11 +++- OpenRA.Test/OpenRA.Game/OrderTest.cs | 86 +++++++++++++++------------- 4 files changed, 94 insertions(+), 54 deletions(-) diff --git a/OpenRA.Game/Network/Order.cs b/OpenRA.Game/Network/Order.cs index b296233687..18c9982836 100644 --- a/OpenRA.Game/Network/Order.cs +++ b/OpenRA.Game/Network/Order.cs @@ -24,7 +24,8 @@ namespace OpenRA TargetString = 0x04, Queued = 0x08, ExtraLocation = 0x10, - ExtraData = 0x20 + ExtraData = 0x20, + TargetIsCell = 0x40 } static class OrderFieldsExts @@ -45,6 +46,7 @@ namespace OpenRA public CPos ExtraLocation; public uint ExtraData; public bool IsImmediate; + public bool SuppressVisualFeedback; public Actor VisualFeedbackTarget; @@ -60,7 +62,7 @@ namespace OpenRA { get { - return Target.SerializableCell.HasValue ? Target.SerializableCell.Value : CPos.Zero; + return Target.SerializableCell ?? CPos.Zero; } } @@ -68,7 +70,7 @@ namespace OpenRA Order(string orderString, Actor subject, Target target, string targetString, bool queued, CPos extraLocation, uint extraData) { - OrderString = orderString; + OrderString = orderString ?? ""; Subject = subject; Target = target; TargetString = targetString; @@ -127,8 +129,18 @@ namespace OpenRA case TargetType.Terrain: { - if (world != null) - target = Target.FromCell(world, (CPos)r.ReadInt2()); + if (flags.HasField(OrderFields.TargetIsCell)) + { + var cell = new CPos(r.ReadInt32(), r.ReadInt32(), r.ReadByte()); + var subCell = (SubCell)r.ReadInt32(); + if (world != null) + target = Target.FromCell(world, cell, subCell); + } + else + { + var pos = new WPos(r.ReadInt32(), r.ReadInt32(), r.ReadInt32()); + target = Target.FromPos(pos); + } break; } @@ -137,7 +149,7 @@ namespace OpenRA var targetString = flags.HasField(OrderFields.TargetString) ? r.ReadString() : null; var queued = flags.HasField(OrderFields.Queued); - var extraLocation = (CPos)(flags.HasField(OrderFields.ExtraLocation) ? r.ReadInt2() : int2.Zero); + var extraLocation = flags.HasField(OrderFields.ExtraLocation) ? new CPos(r.ReadInt32(), r.ReadInt32(), r.ReadByte()) : CPos.Zero; var extraData = flags.HasField(OrderFields.ExtraData) ? r.ReadUInt32() : 0; if (world == null) @@ -235,10 +247,6 @@ namespace OpenRA public Order(string orderString, Actor subject, Target target, bool queued) : this(orderString, subject, target, null, queued, CPos.Zero, 0) { } - public Order(string orderstring, Order order) - : this(orderstring, order.Subject, order.Target, - order.TargetString, order.Queued, order.ExtraLocation, order.ExtraData) { } - public byte[] Serialize() { var minLength = OrderString.Length + 1 + (IsImmediate ? 1 + TargetString.Length + 1 : 6); @@ -273,6 +281,9 @@ namespace OpenRA if (ExtraData != 0) fields |= OrderFields.ExtraData; + if (Target.SerializableCell != null) + fields |= OrderFields.TargetIsCell; + w.Write((byte)fields); if (fields.HasField(OrderFields.Target)) @@ -288,8 +299,13 @@ namespace OpenRA w.Write(Target.FrozenActor.ID); break; case TargetType.Terrain: - // SerializableCell is guaranteed to be non-null if Type == TargetType.Terrain - w.Write(Target.SerializableCell.Value); + if (fields.HasField(OrderFields.TargetIsCell)) + { + w.Write(Target.SerializableCell.Value); + w.Write((int)Target.SerializableSubCell); + } + else + w.Write(Target.SerializablePos); break; } } @@ -310,7 +326,8 @@ namespace OpenRA { return ("OrderString: \"{0}\" \n\t Subject: \"{1}\". \n\t TargetActor: \"{2}\" \n\t TargetLocation: {3}." + "\n\t TargetString: \"{4}\".\n\t IsImmediate: {5}.\n\t Player(PlayerName): {6}\n").F( - OrderString, Subject, TargetActor != null ? TargetActor.Info.Name : null, TargetLocation, TargetString, IsImmediate, Player != null ? Player.PlayerName : null); + OrderString, Subject, TargetActor != null ? TargetActor.Info.Name : null, TargetLocation, + TargetString, IsImmediate, Player != null ? Player.PlayerName : null); } } } diff --git a/OpenRA.Game/Network/OrderIO.cs b/OpenRA.Game/Network/OrderIO.cs index 2a923a0e31..6fbbe7339a 100644 --- a/OpenRA.Game/Network/OrderIO.cs +++ b/OpenRA.Game/Network/OrderIO.cs @@ -65,6 +65,14 @@ namespace OpenRA.Network { w.Write(cell.X); w.Write(cell.Y); + w.Write(cell.Layer); + } + + public static void Write(this BinaryWriter w, WPos pos) + { + w.Write(pos.X); + w.Write(pos.Y); + w.Write(pos.Z); } } } diff --git a/OpenRA.Game/Traits/Target.cs b/OpenRA.Game/Traits/Target.cs index 781d4f1549..c8660740ba 100644 --- a/OpenRA.Game/Traits/Target.cs +++ b/OpenRA.Game/Traits/Target.cs @@ -26,12 +26,19 @@ namespace OpenRA.Traits FrozenActor frozen; WPos pos; CPos? cell; + SubCell? subCell; int generation; public static Target FromPos(WPos p) { return new Target { pos = p, type = TargetType.Terrain }; } public static Target FromCell(World w, CPos c, SubCell subCell = SubCell.FullCell) { - return new Target { pos = w.Map.CenterOfSubCell(c, subCell), cell = c, type = TargetType.Terrain }; + return new Target + { + pos = w.Map.CenterOfSubCell(c, subCell), + cell = c, + subCell = subCell, + type = TargetType.Terrain + }; } /// @@ -201,5 +208,7 @@ namespace OpenRA.Traits internal TargetType SerializableType { get { return type; } } internal Actor SerializableActor { get { return actor; } } internal CPos? SerializableCell { get { return cell; } } + internal SubCell? SerializableSubCell { get { return subCell; } } + internal WPos SerializablePos { get { return pos; } } } } diff --git a/OpenRA.Test/OpenRA.Game/OrderTest.cs b/OpenRA.Test/OpenRA.Game/OrderTest.cs index afb0a6f9f4..71514eb452 100644 --- a/OpenRA.Test/OpenRA.Game/OrderTest.cs +++ b/OpenRA.Test/OpenRA.Game/OrderTest.cs @@ -18,51 +18,57 @@ namespace OpenRA.Test [TestFixture] public class OrderTest { - Order order; - Order targetInvalid; - Order immediateOrder; - - [SetUp] - public void SetUp() + byte[] RoundTripOrder(byte[] bytes) { - order = new Order("TestOrder", null, false) - { - TargetString = "TestTarget", - ExtraData = 1234, - ExtraLocation = new CPos(555, 555) - }; + return Order.Deserialize(null, new BinaryReader(new MemoryStream(bytes))).Serialize(); + } - targetInvalid = new Order("TestOrder", null, Target.Invalid, false); + [TestCase(TestName = "Order data persists over serialization (empty)")] + public void SerializeEmpty() + { + var o = new Order().Serialize(); + Assert.That(RoundTripOrder(o), Is.EqualTo(o)); + } - immediateOrder = new Order("TestOrderImmediate", null, false) + [TestCase(TestName = "Order data persists over serialization (unqueued)")] + public void SerializeUnqueued() + { + var o = new Order("Test", null, false).Serialize(); + Assert.That(RoundTripOrder(o), Is.EqualTo(o)); + } + + [TestCase(TestName = "Order data persists over serialization (queued)")] + public void SerializeQueued() + { + var o = new Order("Test", null, true).Serialize(); + Assert.That(RoundTripOrder(o), Is.EqualTo(o)); + } + + [TestCase(TestName = "Order data persists over serialization (pos target)")] + public void SerializePos() + { + var o = new Order("Test", null, Target.FromPos(new WPos(int.MinValue, 0, int.MaxValue)), false).Serialize(); + Assert.That(RoundTripOrder(o), Is.EqualTo(o)); + } + + [TestCase(TestName = "Order data persists over serialization (invalid target)")] + public void SerializeInvalid() + { + var o = new Order("Test", null, Target.Invalid, false).Serialize(); + Assert.That(RoundTripOrder(o), Is.EqualTo(o)); + } + + [TestCase(TestName = "Order data persists over serialization (extra fields)")] + public void SerializeExtra() + { + var o = new Order("Test", null, Target.Invalid, true) { + TargetString = "TargetString", + ExtraLocation = new CPos(int.MinValue, int.MaxValue, 128), + ExtraData = uint.MaxValue, IsImmediate = true, - TargetString = "TestTarget" - }; - } - - Order RoundTripOrder(Order o) - { - var serializedData = new MemoryStream(o.Serialize()); - return Order.Deserialize(null, new BinaryReader(serializedData)); - } - - [TestCase(TestName = "Data persists over serialization")] - public void SerializeA() - { - Assert.That(RoundTripOrder(order).ToString(), Is.EqualTo(order.ToString())); - } - - [TestCase(TestName = "Data persists over serialization (Immediate order)")] - public void SerializeB() - { - Assert.That(RoundTripOrder(immediateOrder).ToString(), Is.EqualTo(immediateOrder.ToString())); - } - - [TestCase(TestName = "Data persists over serialization (Invalid target)")] - public void SerializeC() - { - Assert.That(RoundTripOrder(targetInvalid).ToString(), Is.EqualTo(targetInvalid.ToString())); + }.Serialize(); + Assert.That(RoundTripOrder(o), Is.EqualTo(o)); } } }