Project destinations to NavMesh height

[HazDS]

Sample NavMesh height for destination markers in LocationDialogueSpec and WalkToSpec so destination markers use the navmesh Y coordinate when creating NPC markers. This makes distance checks and walk completion logic consistent with where NPCs actually walk (fixes WalkResult.Partial / unreachable ReachedDestination and FaceDirection not being called).

Release Notes

• Project destination markers to NavMesh height in LocationDialogueSpec to align marker positioning with actual walking surfaces
• Project destination markers to NavMesh height in WalkToSpec to ensure distance checks and walk completion logic reflect actual NPC walking height
• Add UnityEngine.AIModule.dll references to project dependencies for NavMesh sampling functionality

Contributor Statistics

Author	Insertions	Deletions
HazDS	35	4

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes add NavMesh height alignment to destination markers in location dialogue and walk-to action specifications. A NavMesh.SamplePosition call projects destination coordinates onto the navmesh height, adjusting the y-coordinate for marker placement. Corresponding UnityEngine.AIModule.dll references are added to the project file for both IL2CPP and Mono compilation targets.

Changes

Cohort / File(s)	Summary
NavMesh Height Alignment S1API/Entities/Schedule/ActionSpecs/LocationDialogueSpec.cs, S1API/Entities/Schedule/ActionSpecs/WalkToSpec.cs	Added NavMesh.SamplePosition logic to compute markerPosition with navmesh-aligned y-coordinate. Adds using UnityEngine.AI; directive and passes adjusted position to CreateDestinationMarker instead of original Destination.
Project Dependencies S1API/S1API.csproj	Added UnityEngine.AIModule.dll references across IL2CPP and Mono dependency blocks, including Melon/BepInEx variants, to support NavMesh API access.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 NavMeshes aligned, height adjusted true,
No more floating markers in the ground's view,
Paths now follow where rabbits should tread,
Destinations grounded, precisely ahead! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 70.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Project destinations to NavMesh height' directly and concisely describes the main change across the modified files: projecting destination positions onto the NavMesh height.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

S1API/Entities/Schedule/ActionSpecs/LocationDialogueSpec.cs (1)

116-130: Consider extracting the shared NavMesh projection into a utility helper.

The exact same six-line projection block (lines 118–125 here) is duplicated verbatim in WalkToSpec.cs (lines 120–125). Extracting it keeps future radius or area-mask changes in one place.

♻️ Suggested helper extraction in NPCDestinationContainer.cs

+// In NPCDestinationContainer.cs
+using UnityEngine.AI;
+
+internal static class NPCDestinationContainer
+{
+    // ...existing members...
+
+    /// <summary>
+    /// Projects a world position onto the NavMesh height, preserving X and Z.
+    /// Returns the adjusted position, or the original position if no NavMesh is found.
+    /// </summary>
+    public static Vector3 ProjectToNavMeshHeight(Vector3 position, float maxDistance = 5f)
+    {
+        if (NavMesh.SamplePosition(position, out NavMeshHit navHit, maxDistance, NavMesh.AllAreas))
+            return new Vector3(position.x, navHit.position.y, position.z);
+        return position;
+    }
+}

Then in both specs:

-            Vector3 markerPosition = Destination;
-            NavMeshHit navHit;
-            if (NavMesh.SamplePosition(Destination, out navHit, 5f, NavMesh.AllAreas))
-            {
-                markerPosition = new Vector3(Destination.x, navHit.position.y, Destination.z);
-            }
+            Vector3 markerPosition = NPCDestinationContainer.ProjectToNavMeshHeight(Destination);

Also, since <LangVersion>latest</LangVersion> is set, the pre-C#7 two-step out declaration can be collapsed to inline out var:

-            NavMeshHit navHit;
-            if (NavMesh.SamplePosition(Destination, out navHit, 5f, NavMesh.AllAreas))
+            if (NavMesh.SamplePosition(Destination, out NavMeshHit navHit, 5f, NavMesh.AllAreas))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@S1API/Entities/Schedule/ActionSpecs/LocationDialogueSpec.cs` around lines 116
- 130, Extract the duplicated NavMesh projection logic into a single helper (for
example add a method like ProjectToNavMesh(Vector3 destination, float
maxDistance = 5f, int areaMask = NavMesh.AllAreas) that returns the projected
Vector3) on NPCDestinationContainer or another small utility class, then replace
the six-line block in LocationDialogueSpec (the
markerPosition/NavMesh.SamplePosition code) and the identical block in
WalkToSpec with a call to that helper; while updating call sites, use inline out
declaration (out var navHit) if you still call NavMesh.SamplePosition directly
inside the helper or when testing, and pass Forward and markerPosition into
NPCDestinationContainer.CreateDestinationMarker as before.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@S1API/Entities/Schedule/ActionSpecs/LocationDialogueSpec.cs`:
- Around line 120-123: NavMesh.SamplePosition call using a radius of 5f can snap
to the wrong floor on stacked NavMesh surfaces; update the SamplePosition
invocation in LocationDialogueSpec (the call using Destination and out navHit)
to use a smaller radius (e.g., 2f) to reduce cross-floor hits, and keep the
existing assignment to markerPosition (new Vector3(Destination.x,
navHit.position.y, Destination.z)) unchanged so vertical placement still uses
the sampled navHit.y when a hit occurs.

---

Nitpick comments:
In `@S1API/Entities/Schedule/ActionSpecs/LocationDialogueSpec.cs`:
- Around line 116-130: Extract the duplicated NavMesh projection logic into a
single helper (for example add a method like ProjectToNavMesh(Vector3
destination, float maxDistance = 5f, int areaMask = NavMesh.AllAreas) that
returns the projected Vector3) on NPCDestinationContainer or another small
utility class, then replace the six-line block in LocationDialogueSpec (the
markerPosition/NavMesh.SamplePosition code) and the identical block in
WalkToSpec with a call to that helper; while updating call sites, use inline out
declaration (out var navHit) if you still call NavMesh.SamplePosition directly
inside the helper or when testing, and pass Forward and markerPosition into
NPCDestinationContainer.CreateDestinationMarker as before.