Creatures and pathfinding in Lua games: nav.* and ai.* (ENG-F70) #127
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "khyretos/claude/lua-nav-2"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Requested by Kees · project thread
Stacked on #119 (moving-door navmesh): merge that first; this PR then shows only its own two commits.
Replaces #121 (closed with merge conflicts): the same change rebased onto main
ccdddb3, now that #119 is merged; the terrain and nav hooks sit side by side in ScriptModule. All 1295 tests pass locally.What (ENG-F70: pathfinding and creatures for Lua games)
nav.*for scripts:nav.build{radius, height, climb, slope, cell, tile, dynamic}makes a navmesh from the level's collision (every static body: script boxes, scenes, terrain), through the newRigidWorld::staticTriangles. Thenpath(from, to)→ corners andpartial,walkable,nearest,move(slides along walls),random, andobstacle{pos, size, yaw}/{pos, radius, height}→ id,remove(id). A script's obstacles and navmesh go when it unloads (256 obstacles per script).ai.*creatures for scripts:ScriptModulekeeps oneai::AiWorldwhose agents are the script's models.ai.add(model, "dog")gives it a mind; every frame it walks the navmesh, stands on the ground (static raycast), sees only what isn't behind a wall, turns where it goes, and plays the clip thatai.anims(inst, {idle=, walk=, run=, attack=, ...})names for what it does (run falls back to walk, anything else to idle; attacks play once).ai.actor(id, pos [, species, velocity])places someone with no model, such as the player (ids from 1,000,000). Newmodels.position(inst)→ pos, yaw.models.removeand script unload take the mind away too.AiWorld:Arrivedfired every frame between reaching a goTo/fetch goal and the next think tick. It now fires once.nav,aiandmodels.positionin the table), AI.md "Lua", GAPS F70.Tests
RigidWorld.StaticTrianglesAreTheLevelsCollision;AiWorld.OrdersWinAndReportArrivalnow asserts exactly oneArrivedper order.kke_tests: 1250 passed, no warnings at-Wall -Wextra.nav.build()made 28 polygons in 7.8 ms from the template level. Three UAL mannequins givenai.add(inst, "dog")andai.goTowalked 12 m across the level on the navmesh playing Walk, faced their way, spotted each other and theai.actor, firedArrivedonce each, then held their spot playing Idle.🤖 Generated with Claude Code
https://claude.ai/code/session_019rkS6wq7fV9fWdiLwxyyc7
Title
Creatures and pathfinding in Lua games: nav.* and ai.* (ENG-F70)
User description
Requested by Kees · project thread
Stacked on #119 (moving-door navmesh): merge that first; this PR then shows only its own two commits.
Replaces #121 (closed with merge conflicts): the same change rebased onto main
ccdddb3, now that #119 is merged; the terrain and nav hooks sit side by side in ScriptModule. All 1295 tests pass locally.What (ENG-F70: pathfinding and creatures for Lua games)
nav.*for scripts:nav.build{radius, height, climb, slope, cell, tile, dynamic}makes a navmesh from the level's collision (every static body: script boxes, scenes, terrain), through the newRigidWorld::staticTriangles. Thenpath(from, to)→ corners andpartial,walkable,nearest,move(slides along walls),random, andobstacle{pos, size, yaw}/{pos, radius, height}→ id,remove(id). A script's obstacles and navmesh go when it unloads (256 obstacles per script).ai.*creatures for scripts:ScriptModulekeeps oneai::AiWorldwhose agents are the script's models.ai.add(model, "dog")gives it a mind; every frame it walks the navmesh, stands on the ground (static raycast), sees only what isn't behind a wall, turns where it goes, and plays the clip thatai.anims(inst, {idle=, walk=, run=, attack=, ...})names for what it does (run falls back to walk, anything else to idle; attacks play once).ai.actor(id, pos [, species, velocity])places someone with no model, such as the player (ids from 1,000,000). Newmodels.position(inst)→ pos, yaw.models.removeand script unload take the mind away too.AiWorld:Arrivedfired every frame between reaching a goTo/fetch goal and the next think tick. It now fires once.nav,aiandmodels.positionin the table), AI.md "Lua", GAPS F70.Tests
RigidWorld.StaticTrianglesAreTheLevelsCollision;AiWorld.OrdersWinAndReportArrivalnow asserts exactly oneArrivedper order.kke_tests: 1250 passed, no warnings at-Wall -Wextra.nav.build()made 28 polygons in 7.8 ms from the template level. Three UAL mannequins givenai.add(inst, "dog")andai.goTowalked 12 m across the level on the navmesh playing Walk, faced their way, spotted each other and theai.actor, firedArrivedonce each, then held their spot playing Idle.🤖 Generated with Claude Code
https://claude.ai/code/session_019rkS6wq7fV9fWdiLwxyyc7
PR Type
Enhancement
Description
Adds Lua
nav.*functions to build pathfinding from level collisionIntroduces
ai.*system for creatures walking navmeshes with clipsFixes AI arrival event firing only once per order instead of every frame
Exposes static level triangles via new
RigidWorld::staticTrianglesmethodDiagram Walkthrough
File Walkthrough
3 files
Implements staticTriangles to extract level collision as trianglesBinds nav.* and ai.* Lua functions for pathfinding and creaturesIntegrates nav and creature systems into script lifecycle1 files
Fixes Arrived event to fire once per order completion2 files
Asserts Arrived event fires exactly once per orderTests staticTriangles output correctness and limits4 files
Declares staticTriangles method signatureDocuments Lua nav.* and ai.* integration detailsUpdates ENG-F70 status to reflect Lua pathfinding completionAdds nav.* and ai.* function reference sections1 files
Adds navMesh, aiWorld, and creature clip fieldsPR Reviewer Guide 🔍
(Review updated until commit
97d897675d)📋 Focus areas: 🔴 2 open · ✅ 2 handled. Every push re-checks them; a handled area moves to Resolved findings at the bottom.
Missing Error Handling for Raycast
In
RigidWorld::staticTriangles, the code iterates over all bodies and attempts to lock each one. If a body is locked by another thread or in an invalid state,lock.Succeeded()returns false and the loop continues without any diagnostic logging. While this prevents a crash, it silently skips processing potentially valid static bodies that are temporarily locked, leading to missing geometry in the navmesh. Adding a log message when a lock fails would help diagnose race conditions or performance bottlenecks during navmesh generation.Potential Resource Leak on Script Unload
In
ScriptModule::releaseScript, the code removes obstacles from the navmesh (m_nav->removeObstacle(o.id)) but only if the obstacle belongs to the script being released. However, ifm_navis null or invalid at the time of release (e.g., if the navmesh was destroyed earlier), the code will not attempt cleanup. More critically, ifm_navexists butm_nav->removeObstaclethrows an exception or fails silently due to internal state corruption, the obstacle ID remains in the system. The current code does not check the return value or handle exceptions fromremoveObstacle, which could lead to memory leaks or undefined behavior if the navmesh implementation does not guarantee exception safety.✅ Resolved findings
engine/src/RigidWorld.cpp
Missing Error Handling for Raycast
In the
groundHeightlambda (lines 1064-1069), if the raycast fails to hit anything (e.g., the agent is too high or in an empty void), the function returnsfalsewithout settingy. This leavesyuninitialized, which will cause undefined behavior when used later. The code should initializeyto a safe default (like0.0f) before the raycast call.engine/src/RigidWorld.cpp
Potential Memory Leak in Dynamic Navmesh
In
ScriptModule::releaseScript(lines 1214-1224), obstacles are removed from the navmesh and then cleared fromm_navObstacles. However, ifm_navis not valid orm_navSourcedoes not match the current script, the navmesh itself (m_nav) is only reset ifmine(m_navSource)is true. If a script unloads butm_navSourcewas set to a different script (due to a logic error or race condition), the navmesh might persist while its associated obstacles are cleaned up, leading to a mismatch. More critically, ifm_navis non-null butmine(m_navSource)is false, the navmesh is NOT reset, potentially leaving stale data. The logic at line 1219 ensures the navmesh is only cleared if it belongs to the releasing script, which is correct, but the comment at line 1213 implies all obstacles are removed regardless of ownership check before the conditional block. The code correctly checks ownership for obstacle removal (line 1215-1216) and navmesh reset (line 1219). However, there is a subtle issue: ifm_navexists butm_navSourceis empty or doesn't match, the navmesh persists. This is likely intended behavior (don't destroy global navs), but the lack of explicit logging or assertion makes debugging difficult ifm_navSourcegets out of sync. A more robust approach would be to ensurem_navSourceis always updated atomically withm_navcreation and cleared immediately upon destruction, which seems to be happening, but the potential form_navto point to a mesh built by a script that has already been partially unloaded (ifreleaseScriptis called multiple times or in parallel) is a risk. Given the single-threaded nature of Lua scripts, this is less likely, but the lack of validation onm_navSourcebefore accessingm_navin other parts of the code could be problematic.PR Code Suggestions ✨
No code suggestions found for the PR.
🧹 Lint report · ❌ 38 problems in 6 checks · 🧹 3 formatting checks to tidy
Nothing here blocks the merge (report only).
MARKDOWN · markdownlint: first lines of the output
CPP · cppcheck: first lines of the output
YAML · yamllint: first lines of the output
BASH · shellcheck: first lines of the output
DOCKERFILE · hadolint: first lines of the output
REPOSITORY · trivy: first lines of the output
🧹 Formatting only (layout, no behaviour; the nightly lint-fix job applies these as a PR): YAML · prettier (57 files checked), BASH · shfmt (11 files checked), JSON · prettier (147 files checked)
✅ Clean: actionlint, git_diff, gitleaks, grype, jsonlint, secretlint, stylelint, trufflehog, xmllint
MegaLinter on soucouyant · full logs in the Actions run · comment
/improvefor AI fix suggestionsThis PR needs
mainmerged in (a plain merge works), but it was pushed to refs/for/main, so only its author can update it: merge main in and push it again.This PR needs
mainmerged in (a plain merge works), but it was pushed to refs/for/main, so only its author can update it: merge main in and push it again.This PR conflicts with
mainand the bot could not merge it in by itself:engine/include/kke/modules/ScriptModule.hengine/src/modules/ScriptBindings.cppengine/src/modules/ScriptModule.cppReply
unionto keep both sides in every file (CI then checks the result), or merge main in by hand.3ff618a4f797d897675d