Add grid-based nav-mesh rendering#14
Conversation
WalkthroughA grid-based spatial partitioning system was implemented for navigation mesh rendering, optimizing visible node selection using chunked lookup. Configuration options for chunk size and render radius were added, with corresponding UI sliders and documentation. The system rebuilds the grid on relevant changes, and documentation and configuration files were updated to reflect these enhancements. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Menu
participant Visuals
participant Navigation
User->>Menu: Adjusts Chunk Size/Render Chunks sliders
Menu->>Visuals: MaybeRebuildGrid()
Visuals->>Visuals: Checks for config changes
Visuals->>Visuals: Rebuilds grid index if needed
User->>Navigation: Triggers Navigation.Setup()
Navigation->>Visuals: BuildGrid() (if available)
Visuals->>Visuals: Builds grid index
Visuals->>Visuals: OnDraw()
Visuals->>Visuals: collectVisible(me)
Visuals->>Visuals: Gather visible node IDs from grid
Visuals->>Visuals: Render visible nodes
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
FAST_NAV_DRAWING.md (1)
1-43: Comprehensive documentation for the grid-based rendering system.The documentation clearly explains the chunked spatial lookup approach with a well-structured table and practical code examples. The mathematical complexity analysis (~2r³) is accurate for the 3D diamond iteration pattern.
Minor suggestion: The code example references "loader.lua" which doesn't match the actual implementation where
Navigation.Setup()is called. Consider using a more generic reference or updating to match the actual file structure.MedBot/Visuals.lua (2)
184-205: Optimize visible node collection algorithm.The
collectVisiblefunction implements a 3D Manhattan distance constraint, but the nested loops and distance calculation could be optimized.Consider this more efficient approach:
local function collectVisible(me) visCount = 0 local px, py, pz = worldToCell(me:GetAbsOrigin()) local r = G.Menu.Visuals.renderChunks or 3 - for dx = -r, r do - local ax = math.abs(dx) - for dy = -(r - ax), (r - ax) do - local dzMax = r - ax - math.abs(dy) - for dz = -dzMax, dzMax do + -- Pre-calculate valid chunk offsets to avoid redundant calculations + for dx = -r, r do + local axDist = math.abs(dx) + local remainingDist = r - axDist + for dy = -remainingDist, remainingDist do + local ayDist = math.abs(dy) + local dzMax = remainingDist - ayDist + for dz = -dzMax, dzMax do local bx = gridIndex[px + dx] local by = bx and bx[py + dy] local bucket = by and by[pz + dz] if bucket then for _, id in ipairs(bucket) do visCount = visCount + 1 visBuf[visCount] = id end end end end end end
221-221: Fix inconsistent indentation.The indentation appears to be inconsistent with the surrounding code style.
- local currentY = 120 + local currentY = 120
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
FAST_NAV_DRAWING.md(1 hunks)MedBot/Menu.lua(2 hunks)MedBot/Navigation.lua(2 hunks)MedBot/Utils/DefaultConfig.lua(1 hunks)MedBot/Visuals.lua(4 hunks)README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
MedBot/Menu.lua (5)
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.494Z
Learning: Applies to Main.lua : Avoid returning modules globally from `Main.lua` unless they are intended for direct user interaction (e.g., triggering a `Fetcher.Start()` from a menu). For accessing shared data, use the `G` module. For accessing another module's functions, use `require` directly.
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.494Z
Learning: Applies to globals.lua : Actively use the custom `G` (globals) module to store frequently accessed, shared runtime data (e.g., `G.DataBase`, `G.PlayerData`). Do not store application state directly in `_G`. Add comments in `globals.lua` indicating which modules use each variable in `G`.
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.495Z
Learning: Applies to **/*.lua : Design modules to be as independent as possible, primarily interacting through shared data in `G` or explicit function calls via `require`.
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.495Z
Learning: Applies to **/*.lua : Follow this standard order in module files: 1. Imports (`require` statements), 2. Module Declaration (`local ModuleName = {}`), 3. Local Variables/Utilities, 4. Helper/Private Functions, 5. Public Module Functions, 6. Self-Initialization, 7. Callback Registration, 8. Module Return (`return ModuleName`).
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.494Z
Learning: Applies to **/{Database,Config,Fetcher,Main}.lua : Modules managing state/resources (e.g., `Database`, `Config`, `Fetcher`) should handle their own initialization internally (ideally once, either on first `require` or via an `Initialize()` function). Avoid complex setup logic in `Main.lua`.
MedBot/Navigation.lua (6)
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.494Z
Learning: Applies to **/*.lua : Place all `require` statements at the top of modules. Below imports, add reverse import comments (e.g., `--[[ Imported by: ModuleA ]]`).
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.495Z
Learning: Applies to **/*.lua : Design modules to be as independent as possible, primarily interacting through shared data in `G` or explicit function calls via `require`.
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.494Z
Learning: Applies to Main.lua : Avoid returning modules globally from `Main.lua` unless they are intended for direct user interaction (e.g., triggering a `Fetcher.Start()` from a menu). For accessing shared data, use the `G` module. For accessing another module's functions, use `require` directly.
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.494Z
Learning: Applies to globals.lua : Actively use the custom `G` (globals) module to store frequently accessed, shared runtime data (e.g., `G.DataBase`, `G.PlayerData`). Do not store application state directly in `_G`. Add comments in `globals.lua` indicating which modules use each variable in `G`.
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.495Z
Learning: Applies to **/*.lua : Follow this standard order in module files: 1. Imports (`require` statements), 2. Module Declaration (`local ModuleName = {}`), 3. Local Variables/Utilities, 4. Helper/Private Functions, 5. Public Module Functions, 6. Self-Initialization, 7. Callback Registration, 8. Module Return (`return ModuleName`).
Learnt from: CR
PR: titaniummachine1/Medbot#0
File: .cursor/rules/mainrules.mdc:0-0
Timestamp: 2025-07-16T18:41:12.494Z
Learning: Applies to **/{Database,Config,Fetcher,Main}.lua : Modules managing state/resources (e.g., `Database`, `Config`, `Fetcher`) should handle their own initialization internally (ideally once, either on first `require` or via an `Initialize()` function). Avoid complex setup logic in `Main.lua`.
🧬 Code Graph Analysis (1)
MedBot/Menu.lua (1)
MedBot/Visuals.lua (1)
Visuals.MaybeRebuildGrid(166-172)
🔇 Additional comments (9)
README.md (1)
9-11: Clean documentation reference addition.The addition of the reference to
FAST_NAV_DRAWING.mdis appropriate and helps users discover the new nav-mesh rendering feature.MedBot/Navigation.lua (2)
20-20: Import follows established patterns.The Visuals module import is correctly placed at the top with other require statements, following the established module structure.
257-264: Proper conditional integration of grid building.The conditional check for both
VisualsandVisuals.BuildGridensures the feature is only used when available, providing good defensive programming. The placement afterNode.Setup()and beforeNavigation.ClearPath()is logical for initializing the grid after nodes are ready.MedBot/Menu.lua (2)
17-17: Import correctly placed.The Visuals module import follows the established pattern of placing require statements at the top of the file.
163-177: Well-structured UI controls for grid configuration.The new sliders for chunk size and render chunks follow established menu patterns. The ranges (64-512 for chunk size, 1-10 for render chunks) appear reasonable for the use case. The call to
Visuals.MaybeRebuildGrid()ensures the grid updates immediately when settings change, providing good user experience.MedBot/Utils/DefaultConfig.lua (1)
24-29: Sensible default values for grid configuration.The new configuration parameters
chunkSize = 256andrenderChunks = 3provide reasonable defaults that balance performance and visual quality. The integration into the existing Visuals configuration section is clean and follows established patterns.MedBot/Visuals.lua (3)
13-20: Good addition of grid-based rendering infrastructure.The initialization of grid data structures and tracking variables looks correct. The use of module-level variables for
lastChunkSizeandlastRenderChunksenables efficient change detection.
165-172: Good change detection pattern for grid rebuilding.The
MaybeRebuildGridfunction efficiently checks for configuration changes before triggering expensive grid rebuilds.
230-243: Consider buffer overflow protection and add validation.The visible node collection and processing logic looks correct, but there's potential for buffer overflow if
visBufbecomes too large, and missing validation for world-to-screen conversion.-- Collect visible nodes using chunk grid Visuals.MaybeRebuildGrid() collectVisible(me) local visibleNodes = {} + -- Protect against excessive visible nodes + local maxVisibleNodes = 10000 -- Configurable limit + local actualCount = math.min(visCount, maxVisibleNodes) - for i = 1, visCount do + for i = 1, actualCount do local id = visBuf[i] local node = G.Navigation.nodes and G.Navigation.nodes[id] if node then local scr = client.WorldToScreen(node.pos) if scr then visibleNodes[id] = { node = node, screen = scr } end end end + -- Log performance warning if we hit the limit + if visCount > maxVisibleNodes then + print(string.format("Warning: Visible node count (%d) exceeds limit (%d)", visCount, maxVisibleNodes)) + endLikely an incorrect or invalid review comment.
| function Visuals.Initialize() | ||
| buildGrid() | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to initialization.
The Initialize function should handle potential errors during grid building.
function Visuals.Initialize()
- buildGrid()
+ local success, err = pcall(buildGrid)
+ if not success then
+ print("Error initializing visuals grid: " .. tostring(err))
+ -- Fallback to empty grid state
+ gridIndex = {}
+ nodeCell = {}
+ visBuf = {}
+ visCount = 0
+ end
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function Visuals.Initialize() | |
| buildGrid() | |
| end | |
| function Visuals.Initialize() | |
| local success, err = pcall(buildGrid) | |
| if not success then | |
| print("Error initializing visuals grid: " .. tostring(err)) | |
| -- Fallback to empty grid state | |
| gridIndex = {} | |
| nodeCell = {} | |
| visBuf = {} | |
| visCount = 0 | |
| end | |
| end |
🤖 Prompt for AI Agents
In MedBot/Visuals.lua around lines 179 to 181, the Initialize function calls
buildGrid without any error handling. Modify Initialize to wrap the buildGrid
call in a pcall or similar error handling construct to catch and handle any
errors that occur during grid building, ensuring the function fails gracefully
or logs the error appropriately.
| -- Convert world position to chunk cell | ||
| local function worldToCell(pos) | ||
| local size = G.Menu.Visuals.chunkSize or 256 | ||
| return math.floor(pos.x / size), math.floor(pos.y / size), math.floor(pos.z / size) | ||
| end |
There was a problem hiding this comment.
Potential division by zero and coordinate system assumptions.
The worldToCell function could fail if chunkSize is 0, and there's an assumption that the coordinate system uses standard X, Y, Z axes.
local function worldToCell(pos)
- local size = G.Menu.Visuals.chunkSize or 256
+ local size = G.Menu.Visuals.chunkSize or 256
+ if size <= 0 then
+ error("chunkSize must be greater than 0")
+ end
return math.floor(pos.x / size), math.floor(pos.y / size), math.floor(pos.z / size)
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -- Convert world position to chunk cell | |
| local function worldToCell(pos) | |
| local size = G.Menu.Visuals.chunkSize or 256 | |
| return math.floor(pos.x / size), math.floor(pos.y / size), math.floor(pos.z / size) | |
| end | |
| -- Convert world position to chunk cell | |
| local function worldToCell(pos) | |
| local size = G.Menu.Visuals.chunkSize or 256 | |
| if size <= 0 then | |
| error("chunkSize must be greater than 0") | |
| end | |
| return math.floor(pos.x / size), | |
| math.floor(pos.y / size), | |
| math.floor(pos.z / size) | |
| end |
🤖 Prompt for AI Agents
In MedBot/Visuals.lua around lines 142 to 146, the worldToCell function risks
division by zero if chunkSize is 0 and assumes standard X, Y, Z axes. To fix
this, add a check to ensure chunkSize is a positive non-zero value before using
it; if it is zero or nil, assign a default positive value like 256. Also, verify
or document the coordinate system assumptions to ensure compatibility or handle
alternative coordinate systems if needed.
| local function buildGrid() | ||
| gridIndex = {} | ||
| nodeCell = {} | ||
| local size = G.Menu.Visuals.chunkSize or 256 | ||
| for id, node in pairs(G.Navigation.nodes or {}) do | ||
| local cx, cy, cz = worldToCell(node.pos) | ||
| gridIndex[cx] = gridIndex[cx] or {} | ||
| gridIndex[cx][cy] = gridIndex[cx][cy] or {} | ||
| gridIndex[cx][cy][cz] = gridIndex[cx][cy][cz] or {} | ||
| table.insert(gridIndex[cx][cy][cz], id) | ||
| nodeCell[id] = { cx, cy, cz } | ||
| end | ||
| Visuals.lastChunkSize = size | ||
| Visuals.lastRenderChunks = G.Menu.Visuals.renderChunks or 3 | ||
| end |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify navigation nodes structure and add error handling.
The buildGrid function assumes G.Navigation.nodes exists and has a specific structure. Consider adding validation to prevent runtime errors.
🏁 Script executed:
#!/bin/bash
# Description: Verify the structure and usage of G.Navigation.nodes across the codebase
# Check how G.Navigation.nodes is defined and used
ast-grep --pattern 'G.Navigation.nodes = $_'
# Check for null/undefined checks on G.Navigation.nodes
rg -A 3 -B 3 'G\.Navigation\.nodes'
# Look for node structure usage (pos property)
rg -A 2 'node\.pos'Length of output: 14364
Validate node.pos in buildGrid to prevent runtime errors
Before calling worldToCell(node.pos), add a guard to skip any entries where node or node.pos is missing. For example, in MedBot/Visuals.lua (around lines 149–163):
local function buildGrid()
gridIndex = {}
nodeCell = {}
local size = G.Menu.Visuals.chunkSize or 256
- for id, node in pairs(G.Navigation.nodes or {}) do
- local cx, cy, cz = worldToCell(node.pos)
+ for id, node in pairs(G.Navigation.nodes or {}) do
+ if not node or not node.pos then
+ Log:Warn("Visuals.buildGrid: skipping invalid node %s", tostring(id))
+ goto continue
+ end
+ local cx, cy, cz = worldToCell(node.pos)
gridIndex[cx] = gridIndex[cx] or {}
gridIndex[cx][cy] = gridIndex[cx][cy] or {}
gridIndex[cx][cy][cz] = gridIndex[cx][cy][cz] or {}
table.insert(gridIndex[cx][cy][cz], id)
nodeCell[id] = { cx, cy, cz }
+ ::continue::
end
Visuals.lastChunkSize = size
Visuals.lastRenderChunks = G.Menu.Visuals.renderChunks or 3
endThis ensures only valid vectors are passed to worldToCell, avoiding potential nil-field errors in future.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local function buildGrid() | |
| gridIndex = {} | |
| nodeCell = {} | |
| local size = G.Menu.Visuals.chunkSize or 256 | |
| for id, node in pairs(G.Navigation.nodes or {}) do | |
| local cx, cy, cz = worldToCell(node.pos) | |
| gridIndex[cx] = gridIndex[cx] or {} | |
| gridIndex[cx][cy] = gridIndex[cx][cy] or {} | |
| gridIndex[cx][cy][cz] = gridIndex[cx][cy][cz] or {} | |
| table.insert(gridIndex[cx][cy][cz], id) | |
| nodeCell[id] = { cx, cy, cz } | |
| end | |
| Visuals.lastChunkSize = size | |
| Visuals.lastRenderChunks = G.Menu.Visuals.renderChunks or 3 | |
| end | |
| local function buildGrid() | |
| gridIndex = {} | |
| nodeCell = {} | |
| local size = G.Menu.Visuals.chunkSize or 256 | |
| for id, node in pairs(G.Navigation.nodes or {}) do | |
| if not node or not node.pos then | |
| Log:Warn("Visuals.buildGrid: skipping invalid node %s", tostring(id)) | |
| goto continue | |
| end | |
| local cx, cy, cz = worldToCell(node.pos) | |
| gridIndex[cx] = gridIndex[cx] or {} | |
| gridIndex[cx][cy] = gridIndex[cx][cy] or {} | |
| gridIndex[cx][cy][cz] = gridIndex[cx][cy][cz] or {} | |
| table.insert(gridIndex[cx][cy][cz], id) | |
| nodeCell[id] = { cx, cy, cz } | |
| ::continue:: | |
| end | |
| Visuals.lastChunkSize = size | |
| Visuals.lastRenderChunks = G.Menu.Visuals.renderChunks or 3 | |
| end |
🤖 Prompt for AI Agents
In MedBot/Visuals.lua around lines 149 to 163, the function buildGrid calls
worldToCell(node.pos) without checking if node or node.pos exists, which can
cause runtime errors. Add a guard clause inside the loop to skip any node
entries where node or node.pos is nil before calling worldToCell. This will
ensure only valid positions are processed and prevent nil-field errors.
Summary
Testing
git status --shorthttps://chatgpt.com/codex/tasks/task_e_687aad013ce4832c83f7b7cedf7ad758
Summary by CodeRabbit
New Features
Performance Improvements
Documentation