Merge remote-tracking branch 'origin/feat/inport-props' into feat/craft-discovery

# Conflicts:
#	Assets/External/Animated PBR Chest Demo/Materials/WoodChest.mat
#	Packages/com.distantlands.cozy.core/Content/Integration/Import for BiRP.unitypackage.meta
#	Packages/com.distantlands.cozy.core/Content/Integration/Import for HDRP.unitypackage.meta
#	Packages/com.distantlands.cozy.core/Content/Integration/Import for URP.unitypackage.meta

Assets/GAME/Script/Network/HarvestableRegistry.cs

@@ -60,29 +60,43 @@ namespace Ashwild.Network
         /// <summary>
         /// Owner-side entry: tells the server the local player hit a harvestable. The server applies
         /// the (client-computed) damage, grants loot, broadcasts feedback, and handles depletion.
+        ///
+        /// The two ways this can go wrong are reported back to the requesting player instead of being
+        /// swallowed: an id the server does not track (a scene object with a missing or duplicate id) and
+        /// a connection whose inventory cannot be resolved. Both used to end in a bare `return` — the
+        /// player saw their hit land and gained nothing, with no trace anywhere on their machine. Losing
+        /// the race against another player is the one case left unreported, since the object disappearing
+        /// says it plainly enough.
         /// </summary>
         [ServerRpc(RequireOwnership = false)]
-        public void RequestHitServerRpc(int id, float damage, ushort toolItemId, Vector3 hitDirection, NetworkConnection conn = null)
+        public void RequestHitServerRpc(int id, float damage, ushort toolItemId, NetworkConnection conn = null)
         {
             if (IsInactive(id)) return;
-            if (!TryGetObject(id, out WorldObject obj) || obj is not Harvestable harvestable) return;
+
+            if (!TryGetObject(id, out WorldObject obj) || obj is not Harvestable harvestable)
+            {
+                ReportFailure(conn, id, "no harvestable is registered with this id on the server");
+                return;
+            }
+
+            PlayerInventory inventory = ResolveInventory(conn);
+            if (inventory == null)
+            {
+                ReportFailure(conn, id, "the requesting player's inventory could not be resolved — the loot would be lost");
+                return;
+            }
 
             if (!health.TryGetValue(id, out float hp)) hp = harvestable.MaxHealth;
             hp -= damage;
             health[id] = hp;
 
-            // Grant the rolled loot to the hitting player.
             ItemData tool = ItemDatabase.Instance != null ? ItemDatabase.Instance.GetItem(toolItemId) : null;
-            PlayerInventory inventory = ResolveInventory(conn);
-            if (inventory != null)
-            {
-                foreach ((ItemData item, int quantity) in harvestable.RollDrops(tool))
-                    inventory.GrantItemFromServer(item, quantity);
-            }
+            foreach ((ItemData item, int quantity) in harvestable.RollDrops(tool))
+                inventory.GrantItemFromServer(item, quantity);
 
             // Feedback for everyone except the hitter (who already played it locally for responsiveness).
             int hitterClientId = conn != null ? conn.ClientId : -1;
-            PlayHitObserversRpc(id, hitDirection, hitterClientId);
+            PlayHitObserversRpc(id, hitterClientId);
 
             if (hp <= 0f)
             {
@@ -101,11 +115,11 @@ namespace Ashwild.Network
         /// Plays the hit feedback on every client except the one who threw the hit.
         /// </summary>
         [ObserversRpc]
-        private void PlayHitObserversRpc(int id, Vector3 hitDirection, int hitterClientId)
+        private void PlayHitObserversRpc(int id, int hitterClientId)
         {
             if (LocalConnection != null && LocalConnection.ClientId == hitterClientId) return;
             if (TryGetObject(id, out WorldObject obj) && obj is Harvestable harvestable)
-                harvestable.PlayHitEffect(hitDirection);
+                harvestable.PlayHitEffect();
         }
 
         #endregion

Assets/GAME/Script/Network/NetworkPlayerLookup.cs

@@ -0,0 +1,48 @@
+using FishNet.Connection;
+using FishNet.Object;
+using Ashwild.Inventory;
+
+namespace Ashwild.Network
+{
+    /// <summary>
+    /// Server-side lookup of the player systems belonging to a connection. Every server-authoritative
+    /// interaction (pickup, harvest, chest, cooking station, build refund) needs the requesting player's
+    /// inventory, and they all used to resolve it through <c>conn.FirstObject</c>.
+    ///
+    /// That is unsafe: a connection owns more than one NetworkObject (the build ghost is spawned with the
+    /// player as owner), FishNet stores them in an unordered <c>HashSet</c>, and it re-picks FirstObject
+    /// from that set whenever the current one is despawned. FirstObject can therefore resolve to the ghost
+    /// instead of the player — a null inventory, and a silently lost grant. Scanning the connection's
+    /// objects for the component we actually want removes the guesswork.
+    /// </summary>
+    public static class NetworkPlayerLookup
+    {
+        #region Public API
+
+        /// <summary>
+        /// Returns the inventory of the player owned by this connection, or null when the connection has
+        /// no player object (disconnecting, or not spawned yet).
+        /// </summary>
+        public static PlayerInventory ResolveInventory(NetworkConnection conn) => Resolve<PlayerInventory>(conn);
+
+        /// <summary>
+        /// Returns the first component of the requested type found on any NetworkObject this connection
+        /// owns. Checks FirstObject before scanning, since it is the right answer in the common case.
+        /// </summary>
+        public static T Resolve<T>(NetworkConnection conn) where T : class
+        {
+            if (conn == null) return null;
+
+            NetworkObject first = conn.FirstObject;
+            if (first != null && first.TryGetComponent(out T fromFirst)) return fromFirst;
+
+            foreach (NetworkObject nob in conn.Objects)
+            {
+                if (nob != null && nob.TryGetComponent(out T component)) return component;
+            }
+            return null;
+        }
+
+        #endregion
+    }
+}

Assets/GAME/Script/Network/NetworkPlayerLookup.cs.meta

@@ -0,0 +1,2 @@
+fileFormatVersion: 2
+guid: dfd1e7f1d6a424b4aa0492f8c3f4c502

Assets/GAME/Script/Network/Pickable.cs

@@ -61,6 +61,15 @@ namespace Ashwild.Network
         /// Asks the registry (server) to claim this pickup for the local player. Aborts while the player
         /// is building (the hammer's placement/demolition would otherwise let an interact press snatch an
         /// item mid-build), if already claimed, or if the local inventory is full.
+        ///
+        /// A full inventory is announced rather than ignored: silently doing nothing on a press is
+        /// indistinguishable from a broken pickup, and that ambiguity is what hid the real bugs. A scene
+        /// pickup the registry never accepted (missing or duplicate baked id) is refused here too, with
+        /// the reason spelled out — the server would drop the request anyway.
+        ///
+        /// The grab animation fires here, once every refusal is behind us but before the server has
+        /// answered: the hand must reach out on the press, not a round-trip later. A refused pickup
+        /// therefore never animates, and a granted one animates immediately.
         /// </summary>
         public void Pickup()
         {
@@ -78,10 +87,21 @@ namespace Ashwild.Network
             }
             if (PickableRegistry.Instance.IsClaimed(Id)) return;
 
+            if (Id >= 0 && !PickableRegistry.Instance.IsRegistered(Id))
+            {
+                Debug.LogError($"[Pickable] '{name}' (id {Id}) is not tracked by the registry — the server " +
+                               "would drop this pickup. Check the console for an id error at startup.", this);
+                return;
+            }
+
             // Client-side pre-check so we don't claim something we can't hold.
             if (PlayerInventory.Instance != null && !PlayerInventory.Instance.CanFit(itemData, quantity))
+            {
+                PlayerEvents.RaiseInteractionRefused("Inventory full");
                 return;
+            }
 
+            PlayerEvents.RaiseGrabPerformed();
             PickableRegistry.Instance.RequestPickupServerRpc(Id);
         }
 

Assets/GAME/Script/Network/PickableRegistry.cs

@@ -112,19 +112,38 @@ namespace Ashwild.Network
 
         /// <summary>
         /// Owner-side entry: asks the server to claim a pickup and grant its item.
+        ///
+        /// Nothing leaves the world before the grant is known to be possible: the pickup is only marked
+        /// claimed, and a drop record only removed, once the item and the requesting inventory have both
+        /// been resolved. Resolving them afterwards is how a pickup could disappear while granting
+        /// nothing. Anything the client could not have foreseen is reported back to it; losing the race
+        /// to another player is left silent, since the object visibly vanishes.
         /// </summary>
         [ServerRpc(RequireOwnership = false)]
         public void RequestPickupServerRpc(int id, NetworkConnection conn = null)
         {
             PlayerInventory inventory = ResolveInventory(conn);
-            if (inventory == null) return;
+            if (inventory == null)
+            {
+                ReportFailure(conn, id, "the requesting player's inventory could not be resolved — the item would be lost");
+                return;
+            }
 
             if (id >= 0)
             {
                 // Scene pickup — the server reads the item from its own copy of the object.
                 if (IsInactive(id)) return;
-                if (!TryGetObject(id, out WorldObject obj) || obj is not Pickable pickup) return;
-                if (pickup.ItemData == null) return;
+
+                if (!TryGetObject(id, out WorldObject obj) || obj is not Pickable pickup)
+                {
+                    ReportFailure(conn, id, "no pickup is registered with this id on the server");
+                    return;
+                }
+                if (pickup.ItemData == null)
+                {
+                    ReportFailure(conn, id, $"'{pickup.name}' has no ItemData assigned");
+                    return;
+                }
 
                 MarkInactive(id);
                 inventory.GrantItemFromServer(pickup.ItemData, pickup.Quantity);
@@ -133,10 +152,16 @@ namespace Ashwild.Network
             {
                 // Runtime drop — the server reads the item from the synced record.
                 if (!activeDrops.TryGetValue(id, out DropRecord record)) return;
+
                 ItemData item = ItemDatabase.Instance != null ? ItemDatabase.Instance.GetItem(record.itemId) : null;
+                if (item == null)
+                {
+                    ReportFailure(conn, id, $"item id {record.itemId} is not in the ItemDatabase — run Rebuild Item Database");
+                    return;
+                }
 
                 activeDrops.Remove(id);
-                if (item != null) inventory.GrantItemFromServer(item, record.quantity, record.uses);
+                inventory.GrantItemFromServer(item, record.quantity, record.uses);
             }
         }
 

Assets/GAME/Script/Network/WorldObject.cs

@@ -14,7 +14,7 @@ namespace Ashwild.Network
         #region Serialized Fields
 
         [Header("Identity")]
-        [Tooltip("Baked scene id (>= 0), assigned by Tools ▸ Ashwild ▸ Assign World Object IDs. Runtime objects get a negative id.")]
+        [Tooltip("Baked scene id (>= 0), assigned by Tools ▸ Ashwild ▸ Setup World Object IDs. Runtime objects get a negative id.")]
         [SerializeField] private int id = -1;
 
         #endregion

Assets/GAME/Script/Network/WorldObjectRegistry.cs

@@ -4,6 +4,7 @@ using FishNet.Object;
 using FishNet.Object.Synchronizing;
 using UnityEngine;
 using Ashwild.Inventory;
+using Ashwild.Player;
 
 namespace Ashwild.Network
 {
@@ -74,15 +75,44 @@ namespace Ashwild.Network
 
         /// <summary>
         /// Registers a world object, hiding it immediately if it is already inactive.
+        ///
+        /// Rejects — loudly — an object whose id is unusable, because both failure modes are otherwise
+        /// invisible at runtime and produce the exact same symptom: the interaction plays its local
+        /// feedback and grants nothing. A scene object left at id -1 (the id tool was never run on it, or
+        /// its prefab carries a baked id) would be treated as a runtime drop; two objects sharing an id
+        /// would overwrite each other here, so claiming one silently kills the other for good. The object
+        /// stays unregistered, which is what makes the interaction refuse itself instead of failing later.
         /// </summary>
         public void RegisterObject(WorldObject obj)
         {
             if (obj == null) return;
+
+            if (obj.Id < 0)
+            {
+                Debug.LogError($"[{GetType().Name}] '{obj.name}' has no baked id (id = {obj.Id}) and cannot be " +
+                               "tracked — it will not be interactable. Run Tools ▸ Ashwild ▸ Setup World Object IDs and save the scene.", obj);
+                return;
+            }
+
+            if (registered.TryGetValue(obj.Id, out WorldObject existing) && existing != null && existing != obj)
+            {
+                Debug.LogError($"[{GetType().Name}] Duplicate world object id {obj.Id}: '{obj.name}' collides with " +
+                               $"'{existing.name}'. Claiming one would silently disable the other. Run Tools ▸ Ashwild ▸ Setup World Object IDs.", obj);
+                return;
+            }
+
             registered[obj.Id] = obj;
             if (inactiveIds.Contains(obj.Id))
                 obj.HideAsInactive(false);
         }
 
+        /// <summary>
+        /// Returns whether this id is tracked by the registry on this client. Interactions check it before
+        /// asking the server, so an object the registry never accepted (bad or duplicate id) refuses up
+        /// front rather than playing its feedback and losing the request server-side.
+        /// </summary>
+        public bool IsRegistered(int id) => registered.ContainsKey(id);
+
         /// <summary>
         /// Returns whether the object with this id has been removed from the world.
         /// </summary>
@@ -114,12 +144,35 @@ namespace Ashwild.Network
         }
 
         /// <summary>
-        /// Returns the PlayerInventory on the player object owned by the given connection.
+        /// Returns the PlayerInventory owned by the given connection. Delegates to the shared lookup,
+        /// which scans the connection's objects instead of trusting FirstObject (see NetworkPlayerLookup).
         /// </summary>
-        protected PlayerInventory ResolveInventory(NetworkConnection conn)
+        protected PlayerInventory ResolveInventory(NetworkConnection conn) => NetworkPlayerLookup.ResolveInventory(conn);
+
+        /// <summary>
+        /// Server-side: reports an interaction the server could not carry out for a reason the client had
+        /// no way to foresee (an id it does not track, a player object it cannot resolve). It logs on the
+        /// server and tells the requesting player, because the alternative — the historic behaviour — is a
+        /// bare `return` that leaves the player watching a hit that gave nothing, with the only trace of it
+        /// in the host's console. Normal races (someone else took it first) are not reported here: the
+        /// object visibly disappears, which is explanation enough.
+        /// </summary>
+        protected void ReportFailure(NetworkConnection conn, int id, string reason)
         {
-            NetworkObject playerObject = conn != null ? conn.FirstObject : null;
-            return playerObject != null ? playerObject.GetComponent<PlayerInventory>() : null;
+            Debug.LogWarning($"[{GetType().Name}] Interaction on id {id} refused for client " +
+                             $"{(conn != null ? conn.ClientId : -1)}: {reason}", this);
+            TargetReportFailure(conn, id, reason);
+        }
+
+        /// <summary>
+        /// Runs on the requesting client: surfaces the refusal in the HUD and logs it locally, so the
+        /// player who actually experienced it sees the diagnosis in their own console.
+        /// </summary>
+        [TargetRpc]
+        private void TargetReportFailure(NetworkConnection conn, int id, string reason)
+        {
+            Debug.LogError($"[{GetType().Name}] The server refused the interaction on id {id}: {reason}", this);
+            PlayerEvents.RaiseInteractionRefused("Interaction failed");
         }
 
         #endregion