Merge pull request 'Probes never write item.Rotation; ItemFits reports the seat via out' (#25) from eureka/aetheria-release-itemfits-rotation-1 into master

Assets/Scripts/ServerShared/Entity.cs

@@ -1547,7 +1547,6 @@ public abstract class Entity
         {
             hardpoint = HardpointAt(itemData, hullData, hullCoord, item.Rotation, moving);
             if (hardpoint == null) return false;
-            item.Rotation = hardpoint.Rotation;
         }
 
         return true;
@@ -1582,14 +1581,22 @@ public abstract class Entity
     // Check whether the given item will fit when its origin is placed at the given coordinate on the hull. An item that
     // is already equipped here is judged without its own cells, so a gun can be moved within its own hardpoint;
     // TryEquip stays strict (it never sees an item as its own occupant), so equipping an equipped item twice is refused.
-    public bool ItemFits(EquippableItem item, int2 hullCoord)
+    public bool ItemFits(EquippableItem item, int2 hullCoord) => ItemFits(item, hullCoord, out _);
+
+    // As above, and reports the rotation TryEquip would seat the item at: its hardpoint's for hardpoint gear, otherwise
+    // its own. A probe never writes item.Rotation; TryEquip is the only seat.
+    public bool ItemFits(EquippableItem item, int2 hullCoord, out ItemRotation seated)
     {
+        seated = item.Rotation;
         // Don't allow equipping while deployed
         if (_active) return false;
 
         var itemData = ItemManager.GetData(item);
         var hullData = ItemManager.GetData(Hull) as HullData;
-        return ItemFits(itemData, hullData, item, hullCoord, out _, Equipment.FirstOrDefault(e => e.EquippableItem == item));
+        if (!ItemFits(itemData, hullData, item, hullCoord, out var hardpoint, Equipment.FirstOrDefault(e => e.EquippableItem == item)))
+            return false;
+        seated = hardpoint?.Rotation ?? item.Rotation;
+        return true;
     }
 
     public bool TryFindSpace(EquippableItem item, out int2 hullCoord)
@@ -1662,7 +1669,9 @@ public abstract class Entity
         var hullData = ItemManager.GetData(Hull) as HullData;
 
         if (!ItemFits(itemData, hullData, item, hullCoord, out var hardpoint)) return false;
-        
+        // The one seat of hardpoint gear's rotation, before EquippedItem's ctor reads it.
+        if (hardpoint != null) item.Rotation = hardpoint.Rotation;
+
         EquippedItem equippedItem;
         if (itemData.HardpointType == HardpointType.Tool)
         {

Assets/Scripts/UI/Menu/InventoryPanel.cs

@@ -386,11 +386,11 @@ public class InventoryPanel : MonoBehaviour, IPointerClickHandler
                                 var itemData = GameManager.ItemManager.GetData(item);
                                 if (!(item is EquippableItem equippableItem)) return;
                                 var placementPosition = v + itemDragObject.OriginCellOffset;
-                                if (entity.ItemFits(equippableItem, placementPosition))
+                                if (entity.ItemFits(equippableItem, placementPosition, out var seated))
                                 {
                                     //foreach (var cell in _dragCells) cell.gameObject.SetActive(false);
                                     FakeItem = item;
-                                    FakeOccupancy = hullData.Shape.Inset(itemData.Shape, placementPosition, item.Rotation);
+                                    FakeOccupancy = hullData.Shape.Inset(itemData.Shape, placementPosition, seated);
                                     RefreshCells();
                                     GameManager.RegisterDragTarget(drag =>
                                     {

tests/Aetheria.Shared.Tests/HardpointOriginTests.cs

@@ -1,3 +1,4 @@
+using System.Collections.Generic;
 using System.Linq;
 using CultMath;
 using Xunit;
@@ -64,4 +65,78 @@ public sealed partial class RunStartTests
         Assert.Same(a, held.Hardpoint);
         Assert.Equal(ItemRotation.None, held.EquippableItem.Rotation);
     }
+
+    // A probe reports; only TryEquip seats. The probe used to turn the item to the hardpoint it found, so the same item
+    // probed twice could resolve differently, and a failed or merely previewed placement left it turned.
+    [Fact]
+    public void AProbeLeavesTheItemsRotationAlone()
+    {
+        var (hull, a, b) = TwinMountHull("Origin Twin Mounts Probe");
+        var edge = ScratchGear("Origin Edge Gun Probe", HardpointType.Ballistic, EdgeShape());
+        var entity = BareHull(hull);
+        var item = Instance(edge);
+        item.Rotation = ItemRotation.Clockwise;
+
+        Assert.True(entity.ItemFits(item, int2(5, 5)));
+        Assert.Equal(ItemRotation.Clockwise, item.Rotation);
+        Assert.True(entity.ItemFits(item, int2(5, 5), out var seated));
+        Assert.Equal(ItemRotation.None, seated);
+        Assert.Equal(ItemRotation.Clockwise, item.Rotation);
+        Assert.True(entity.TryFindSpace(item, out _));
+        Assert.Equal(ItemRotation.Clockwise, item.Rotation);
+        Assert.False(entity.ItemFits(item, int2(0, 0), out var unseated));
+        Assert.Equal(ItemRotation.Clockwise, unseated);
+        Assert.Equal(ItemRotation.Clockwise, item.Rotation);
+
+        Assert.True(entity.TryEquip(item, int2(5, 5)));
+        var held = entity.Equipment.Single(i => i.Data == edge);
+        Assert.Same(a, held.Hardpoint);
+        Assert.Equal(ItemRotation.None, held.EquippableItem.Rotation);
+
+        var reversed = Instance(edge);
+        reversed.Rotation = ItemRotation.Reversed;
+        var fresh = BareHull(hull);
+        Assert.True(fresh.ItemFits(reversed, int2(5, 5), out var first));
+        Assert.True(fresh.ItemFits(reversed, int2(5, 5), out var second));
+        Assert.Equal(ItemRotation.Reversed, first);
+        Assert.Equal(first, second);
+        Assert.Equal(b.Rotation, first);
+    }
+}
+
+// The shipped catalog authors no origin that two differently turned hardpoints of one type both take. The fit rule alone
+// over-approximates what Entity.HardpointAt sees (occupancy only removes candidates), so zero here is stricter than needed.
+public sealed class ShippedHardpointOriginTests
+{
+    [Fact]
+    public void TheShippedCatalogHasNoAmbiguousOrigin()
+    {
+        using var cache = RestoredHullsTests.OpenCatalog();
+        var designs = cache.GetAll<EquippableItemData>().Where(d => !(d is HullData) && d.HardpointType != HardpointType.Tool).ToArray();
+        var ambiguous = new List<string>();
+        var pairs = 0;
+        var probes = 0;
+        foreach (var hull in cache.GetAll<HullData>())
+            for (var i = 0; i < hull.Hardpoints.Count; i++)
+                for (var j = i + 1; j < hull.Hardpoints.Count; j++)
+                {
+                    var a = hull.Hardpoints[i];
+                    var b = hull.Hardpoints[j];
+                    if (a.Type != b.Type || a.Rotation == b.Rotation) continue;
+                    pairs++;
+                    foreach (var design in designs.Where(d => d.HardpointType == a.Type))
+                        for (var x = -20; x < hull.Shape.Width; x++)
+                            for (var y = -20; y < hull.Shape.Height; y++)
+                            {
+                                var origin = int2(x, y);
+                                probes++;
+                                if (a.TakesAt(design, origin - a.Position) && b.TakesAt(design, origin - b.Position))
+                                    ambiguous.Add($"{hull.Name} {design.Name} at ({x},{y}): hardpoints {i} and {j}");
+                            }
+                }
+        System.Console.WriteLine($"ShippedHardpointOriginTests visited {pairs} same-type different-rotation pairs");
+        // A scan that visits nothing would pass; the catalog has such pairs today, so losing them must fail loudly.
+        Assert.True(pairs > 0 && probes > 0, $"the scan visited {pairs} pairs and {probes} origins");
+        Assert.True(ambiguous.Count == 0, $"{ambiguous.Count} ambiguous origins: " + string.Join("; ", ambiguous.Take(10)));
+    }
 }