Apply review feedback: rename MsgPack tests, fix Union Version, CI workflow, null check, separate exceptions

Copilot · mrdav30 · Copilot · commit a798ecee3647 · 2026-03-04T05:08:29.000Z
Co-authored-by: mrdav30 &lt;11547347+mrdav30@users.noreply.github.com&gt;
diff --git a/.github/workflows/dotnet.yml b/.github/workflows/dotnet.yml
@@ -13,10 +13,6 @@ on:
 
 jobs:
   build-and-test:
-    if: |
-      (github.event_name != 'pull_request' && !github.event.pull_request.head.repo.fork) 
-      || (github.event_name == 'pull_request' && (github.event.pull_request.head.repo.fork 
-      || startsWith(github.head_ref, 'dependabot/')))
     runs-on: ${{ matrix.os }}
 
     strategy:
diff --git a/src/FixedMathSharp/Bounds/BoundingBox.cs b/src/FixedMathSharp/Bounds/BoundingBox.cs
@@ -392,6 +392,7 @@ public static BoundingBox Union(BoundingBox a, BoundingBox b)
             {
                 Min = Vector3d.Min(a.Min, b.Min),
                 Max = Vector3d.Max(a.Max, b.Max),
+                Version = Math.Max(a.Version, b.Version),
                 _isDirty = true
             };
         }
diff --git a/src/FixedMathSharp/Numerics/FixedQuaternion.cs b/src/FixedMathSharp/Numerics/FixedQuaternion.cs
@@ -408,12 +408,12 @@ public static FixedQuaternion FromEulerAnglesInDegrees(Fixed64 pitch, Fixed64 ya
         public static FixedQuaternion FromEulerAngles(Fixed64 pitch, Fixed64 yaw, Fixed64 roll)
         {
             // Check if the angles are in a valid range (-pi, pi)
-            if (pitch < -FixedMath.PI || pitch > FixedMath.PI ||
-                yaw < -FixedMath.PI || yaw > FixedMath.PI ||
-                roll < -FixedMath.PI || roll > FixedMath.PI)
-            {
-                throw new ArgumentOutOfRangeException(nameof(pitch), $"Euler angles must be in the range ({-FixedMath.PI}, {FixedMath.PI}), but were ({pitch}, {yaw}, {roll})");
-            }
+            if (pitch < -FixedMath.PI || pitch > FixedMath.PI)
+                throw new ArgumentOutOfRangeException(nameof(pitch), pitch, $"Pitch must be in the range ({-FixedMath.PI}, {FixedMath.PI}), but was {pitch}");
+            if (yaw < -FixedMath.PI || yaw > FixedMath.PI)
+                throw new ArgumentOutOfRangeException(nameof(yaw), yaw, $"Yaw must be in the range ({-FixedMath.PI}, {FixedMath.PI}), but was {yaw}");
+            if (roll < -FixedMath.PI || roll > FixedMath.PI)
+                throw new ArgumentOutOfRangeException(nameof(roll), roll, $"Roll must be in the range ({-FixedMath.PI}, {FixedMath.PI}), but was {roll}");
 
             Fixed64 c1 = FixedMath.Cos(yaw / Fixed64.Two);
             Fixed64 s1 = FixedMath.Sin(yaw / Fixed64.Two);
diff --git a/tests/FixedMathSharp.Tests/Bounds/BoundingArea.Tests.cs b/tests/FixedMathSharp.Tests/Bounds/BoundingArea.Tests.cs
@@ -192,7 +192,7 @@ public void BoundingArea_NetSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void BoundingArea_MsgPackSerialization_RoundTripMaintainsData()
+        public void BoundingArea_MemoryPackSerialization_RoundTripMaintainsData()
         {
             BoundingArea originalValue = new(
                 new Vector3d(1, 2, 3),
diff --git a/tests/FixedMathSharp.Tests/Bounds/BoundingBox.Tests.cs b/tests/FixedMathSharp.Tests/Bounds/BoundingBox.Tests.cs
@@ -266,7 +266,7 @@ public void BoundingBox_NetSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void BoundingBox_MsgPackSerialization_RoundTripMaintainsData()
+        public void BoundingBox_MemoryPackSerialization_RoundTripMaintainsData()
         {
             BoundingBox originalValue = new(new Vector3d(0, 0, 0), new Vector3d(4, 4, 4));
 
@@ -278,7 +278,7 @@ public void BoundingBox_MsgPackSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void MsgPack_SerializedBox_RemainsMutable()
+        public void MemoryPack_SerializedBox_RemainsMutable()
         {
             var box = new BoundingBox(new Vector3d(0, 0, 0), new Vector3d(4, 4, 4));
 
diff --git a/tests/FixedMathSharp.Tests/Bounds/BoundingSphere.Tests.cs b/tests/FixedMathSharp.Tests/Bounds/BoundingSphere.Tests.cs
@@ -203,7 +203,7 @@ public void BoundingSphere_NetSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void BoundingSphere_MsgPackSerialization_RoundTripMaintainsData()
+        public void BoundingSphere_MemoryPackSerialization_RoundTripMaintainsData()
         {
             BoundingSphere originalValue = new(new Vector3d(1, 1, 1), new Fixed64(2));
 
diff --git a/tests/FixedMathSharp.Tests/Fixed3x3.Tests.cs b/tests/FixedMathSharp.Tests/Fixed3x3.Tests.cs
@@ -335,7 +335,7 @@ public void Fixed3x3_NetSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void Fixed3x3_MsgPackSerialization_RoundTripMaintainsData()
+        public void Fixed3x3_MemoryPackSerialization_RoundTripMaintainsData()
         {
             Fixed3x3 originalValue = Fixed3x3.CreateRotationX(FixedMath.PiOver2); // 90 degrees
 
diff --git a/tests/FixedMathSharp.Tests/Fixed4x4.Tests.cs b/tests/FixedMathSharp.Tests/Fixed4x4.Tests.cs
@@ -332,7 +332,7 @@ public void Fixed4x4_NetSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void Fixed4x4_MsgPackSerialization_RoundTripMaintainsData()
+        public void Fixed4x4_MemoryPackSerialization_RoundTripMaintainsData()
         {
             var translation = new Vector3d(1, 2, 3);
             var rotation = FixedQuaternion.FromEulerAnglesInDegrees(Fixed64.Zero, FixedMath.PiOver2, Fixed64.Zero);
diff --git a/tests/FixedMathSharp.Tests/Fixed64.Tests.cs b/tests/FixedMathSharp.Tests/Fixed64.Tests.cs
@@ -211,7 +211,7 @@ public void Fixed64_NetSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void Fixed64_MsgPackSerialization_RoundTripMaintainsData()
+        public void Fixed64_MemoryPackSerialization_RoundTripMaintainsData()
         {
             Fixed64 originalValue = FixedMath.PI;
 
diff --git a/tests/FixedMathSharp.Tests/FixedCurveTests.cs b/tests/FixedMathSharp.Tests/FixedCurveTests.cs
@@ -141,13 +141,14 @@ public void FixedCurve_NetSerialization_RoundTripMaintainsData()
             };
             var json = JsonSerializer.SerializeToUtf8Bytes(originalCurve, jsonOptions);
             var deserializedCurve = JsonSerializer.Deserialize<FixedCurve>(json, jsonOptions);
+            Assert.NotNull(deserializedCurve);
 
             // Check that deserialized values match the original
             Assert.Equal(originalCurve, deserializedCurve);
         }
 
         [Fact]
-        public void FixedCurve_MsgPackSerialization_RoundTripMaintainsData()
+        public void FixedCurve_MemoryPackSerialization_RoundTripMaintainsData()
         {
             FixedCurve originalValue = new FixedCurve(
                 new FixedCurveKey(-10, -100),
diff --git a/tests/FixedMathSharp.Tests/FixedQuanternion.Tests.cs b/tests/FixedMathSharp.Tests/FixedQuanternion.Tests.cs
@@ -385,7 +385,7 @@ public void FixedQuanternion_NetSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void FixedQuanternion_MsgPackSerialization_RoundTripMaintainsData()
+        public void FixedQuanternion_MemoryPackSerialization_RoundTripMaintainsData()
         {
             var quaternion = FixedQuaternion.Identity;
             var sin = FixedMath.Sin(FixedMath.PiOver4);  // 45° rotation
diff --git a/tests/FixedMathSharp.Tests/FixedRange.Tests.cs b/tests/FixedMathSharp.Tests/FixedRange.Tests.cs
@@ -180,7 +180,7 @@ public void FixedRange_NetSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void FixedRange_MsgPackSerialization_RoundTripMaintainsData()
+        public void FixedRange_MemoryPackSerialization_RoundTripMaintainsData()
         {
             FixedRange originalValue = new FixedRange(new Fixed64(-10), new Fixed64(10));
 
diff --git a/tests/FixedMathSharp.Tests/Vector2d.Tests.cs b/tests/FixedMathSharp.Tests/Vector2d.Tests.cs
@@ -327,7 +327,7 @@ public void Vector2d_NetSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void Vector2d_MsgPackSerialization_RoundTripMaintainsData()
+        public void Vector2d_MemoryPackSerialization_RoundTripMaintainsData()
         {
             Vector2d originalValue = new Vector2d(FixedMath.PI, FixedMath.PiOver2);
 
diff --git a/tests/FixedMathSharp.Tests/Vector3d.Tests.cs b/tests/FixedMathSharp.Tests/Vector3d.Tests.cs
@@ -603,7 +603,7 @@ public void Vector3d_NetSerialization_RoundTripMaintainsData()
         }
 
         [Fact]
-        public void Vector3d_MsgPackSerialization_RoundTripMaintainsData()
+        public void Vector3d_MemoryPackSerialization_RoundTripMaintainsData()
         {
             Vector3d originalValue = new Vector3d(FixedMath.PI, FixedMath.PiOver2, FixedMath.TwoPI);
 

Original file line number	Diff line number	Diff line change
`@@ -392,6 +392,7 @@ public static BoundingBox Union(BoundingBox a, BoundingBox b)`
`392`	`392`	`{`
`393`	`393`	`Min = Vector3d.Min(a.Min, b.Min),`
`394`	`394`	`Max = Vector3d.Max(a.Max, b.Max),`
	`395`	`+ Version = Math.Max(a.Version, b.Version),`
`395`	`396`	`_isDirty = true`
`396`	`397`	`};`
`397`	`398`	`}`
Original file line number	Diff line number	Diff line change
`@@ -192,7 +192,7 @@ public void BoundingArea_NetSerialization_RoundTripMaintainsData()`
`192`	`192`	`}`
`193`	`193`
`194`	`194`	`[Fact]`
`195`		`- public void BoundingArea_MsgPackSerialization_RoundTripMaintainsData()`
	`195`	`+ public void BoundingArea_MemoryPackSerialization_RoundTripMaintainsData()`
`196`	`196`	`{`
`197`	`197`	`BoundingArea originalValue = new(`
`198`	`198`	`new Vector3d(1, 2, 3),`
Original file line number	Diff line number	Diff line change
`@@ -266,7 +266,7 @@ public void BoundingBox_NetSerialization_RoundTripMaintainsData()`
`266`	`266`	`}`
`267`	`267`
`268`	`268`	`[Fact]`
`269`		`- public void BoundingBox_MsgPackSerialization_RoundTripMaintainsData()`
	`269`	`+ public void BoundingBox_MemoryPackSerialization_RoundTripMaintainsData()`
`270`	`270`	`{`
`271`	`271`	`BoundingBox originalValue = new(new Vector3d(0, 0, 0), new Vector3d(4, 4, 4));`
`272`	`272`
`@@ -278,7 +278,7 @@ public void BoundingBox_MsgPackSerialization_RoundTripMaintainsData()`
`278`	`278`	`}`
`279`	`279`
`280`	`280`	`[Fact]`
`281`		`- public void MsgPack_SerializedBox_RemainsMutable()`
	`281`	`+ public void MemoryPack_SerializedBox_RemainsMutable()`
`282`	`282`	`{`
`283`	`283`	`var box = new BoundingBox(new Vector3d(0, 0, 0), new Vector3d(4, 4, 4));`
`284`	`284`
Original file line number	Diff line number	Diff line change
`@@ -203,7 +203,7 @@ public void BoundingSphere_NetSerialization_RoundTripMaintainsData()`
`203`	`203`	`}`
`204`	`204`
`205`	`205`	`[Fact]`
`206`		`- public void BoundingSphere_MsgPackSerialization_RoundTripMaintainsData()`
	`206`	`+ public void BoundingSphere_MemoryPackSerialization_RoundTripMaintainsData()`
`207`	`207`	`{`
`208`	`208`	`BoundingSphere originalValue = new(new Vector3d(1, 1, 1), new Fixed64(2));`
`209`	`209`
Original file line number	Diff line number	Diff line change
`@@ -335,7 +335,7 @@ public void Fixed3x3_NetSerialization_RoundTripMaintainsData()`
`335`	`335`	`}`
`336`	`336`
`337`	`337`	`[Fact]`
`338`		`- public void Fixed3x3_MsgPackSerialization_RoundTripMaintainsData()`
	`338`	`+ public void Fixed3x3_MemoryPackSerialization_RoundTripMaintainsData()`
`339`	`339`	`{`
`340`	`340`	`Fixed3x3 originalValue = Fixed3x3.CreateRotationX(FixedMath.PiOver2); // 90 degrees`
`341`	`341`
Original file line number	Diff line number	Diff line change
`@@ -332,7 +332,7 @@ public void Fixed4x4_NetSerialization_RoundTripMaintainsData()`
`332`	`332`	`}`
`333`	`333`
`334`	`334`	`[Fact]`
`335`		`- public void Fixed4x4_MsgPackSerialization_RoundTripMaintainsData()`
	`335`	`+ public void Fixed4x4_MemoryPackSerialization_RoundTripMaintainsData()`
`336`	`336`	`{`
`337`	`337`	`var translation = new Vector3d(1, 2, 3);`
`338`	`338`	`var rotation = FixedQuaternion.FromEulerAnglesInDegrees(Fixed64.Zero, FixedMath.PiOver2, Fixed64.Zero);`
Original file line number	Diff line number	Diff line change
`@@ -211,7 +211,7 @@ public void Fixed64_NetSerialization_RoundTripMaintainsData()`
`211`	`211`	`}`
`212`	`212`
`213`	`213`	`[Fact]`
`214`		`- public void Fixed64_MsgPackSerialization_RoundTripMaintainsData()`
	`214`	`+ public void Fixed64_MemoryPackSerialization_RoundTripMaintainsData()`
`215`	`215`	`{`
`216`	`216`	`Fixed64 originalValue = FixedMath.PI;`
`217`	`217`
Original file line number	Diff line number	Diff line change
`@@ -141,13 +141,14 @@ public void FixedCurve_NetSerialization_RoundTripMaintainsData()`
`141`	`141`	`};`
`142`	`142`	`var json = JsonSerializer.SerializeToUtf8Bytes(originalCurve, jsonOptions);`
`143`	`143`	`var deserializedCurve = JsonSerializer.Deserialize<FixedCurve>(json, jsonOptions);`
	`144`	`+ Assert.NotNull(deserializedCurve);`
`144`	`145`
`145`	`146`	`// Check that deserialized values match the original`
`146`	`147`	`Assert.Equal(originalCurve, deserializedCurve);`
`147`	`148`	`}`
`148`	`149`
`149`	`150`	`[Fact]`
`150`		`- public void FixedCurve_MsgPackSerialization_RoundTripMaintainsData()`
	`151`	`+ public void FixedCurve_MemoryPackSerialization_RoundTripMaintainsData()`
`151`	`152`	`{`
`152`	`153`	`FixedCurve originalValue = new FixedCurve(`
`153`	`154`	`new FixedCurveKey(-10, -100),`