Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughIntroduces Lua code formatting standardization via StyLua with a GitHub Actions workflow job and configuration file. Updates multiple Lua files in the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
data/monstergen.lua (1)
40-50:⚠️ Potential issue | 🟠 MajorAdd missing
behaviour.passiveconstant (currently nil).Line 280–286 use
behaviour.passive, but it is not defined in Line 40–50. This resolves tonil, and those actors can fall back to unintended behavior defaults.Proposed fix
local behaviour = { pacifist = 0, + passive = 0, normal = 1, hostile = 2, guerilla = 3, coward = 4, sentinel = 5, fire_demon = 6, sorcerer = 7, assassin = 8, }Also applies to: 280-286
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/monstergen.lua` around lines 40 - 50, The behaviour table is missing a passive key used elsewhere (behaviour.passive), causing nil fallbacks; update the local behaviour table (the table literal named behaviour) to include a passive entry (e.g., behaviour.passive = <appropriate integer>) alongside the existing keys like pacifist, normal, hostile, etc., so any code referencing behaviour.passive (lines around the actor setup at 280–286) uses the intended constant instead of nil.
🧹 Nitpick comments (1)
.github/workflows/cmake-multi-platform.yml (1)
18-23: Pin StyLua version instead of usinglatest.Line 22 uses
version: latest, which can cause non-reproducible CI builds if a new StyLua version is released between runs. Pin to a specific version likev2.4.0instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cmake-multi-platform.yml around lines 18 - 23, The CI step "Install StyLua" currently sets uses: JohnnyMorganz/stylua-action@v4 with version: latest; change the version field to a fixed tag (e.g. v2.4.0) instead of "latest" to ensure reproducible builds — locate the "Install StyLua" step (the uses: JohnnyMorganz/stylua-action@v4 line) and replace the dynamic version value with the specific pinned release string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data/layoutparser.lua`:
- Around line 14-15: The for-loop over layoutfile uses Lua's 1-based indexing
but calls layoutfile:sub(i + 1, i + 1), skipping the first character and reading
past the end on the last iteration; fix this in layoutparser.lua by changing the
substring access to layoutfile:sub(i, i) (or alternatively adjust the loop
bounds if you prefer) so that the loop variable i correctly selects the i-th
character from layoutfile and avoids out-of-range accesses.
- Around line 22-23: The for-loop iterating "for i = 0, `#cleanData` - 1 do" uses
zero-based indices while Lua strings are one-based, causing cleanData:sub(i, i)
to return an empty string at i=0 and skip the first character; change the loop
to iterate from 1 to `#cleanData` (e.g., "for i = 1, `#cleanData` do") and update
any downstream logic that assumed zero-based indexing (adjust offsets or index
arithmetic in this file for variables that reference i or use i-1) so all uses
of cleanData:sub(i, i) and related position calculations remain correct.
In `@data/maproombuilder.lua`:
- Around line 252-255: The second candle placement currently writes to the same
tile as the first (room.decor[6][offset + 2] = lightDecor.candle1), so change
the coordinate used in the random(2) branch to a different tile (e.g.,
room.decor[6][offset + 3] or another adjacent column) so the two branches place
candles on distinct tiles; update the assignment that references
room.decor[6][offset + 2] inside the random(2) == 1 branch to the new target
coordinate while keeping the same value lightDecor.candle1 and leaving
offset/random logic intact.
---
Outside diff comments:
In `@data/monstergen.lua`:
- Around line 40-50: The behaviour table is missing a passive key used elsewhere
(behaviour.passive), causing nil fallbacks; update the local behaviour table
(the table literal named behaviour) to include a passive entry (e.g.,
behaviour.passive = <appropriate integer>) alongside the existing keys like
pacifist, normal, hostile, etc., so any code referencing behaviour.passive
(lines around the actor setup at 280–286) uses the intended constant instead of
nil.
---
Nitpick comments:
In @.github/workflows/cmake-multi-platform.yml:
- Around line 18-23: The CI step "Install StyLua" currently sets uses:
JohnnyMorganz/stylua-action@v4 with version: latest; change the version field to
a fixed tag (e.g. v2.4.0) instead of "latest" to ensure reproducible builds —
locate the "Install StyLua" step (the uses: JohnnyMorganz/stylua-action@v4 line)
and replace the dynamic version value with the specific pinned release string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae5a6e82-ab65-4a86-9f97-d5883f768291
📒 Files selected for processing (10)
.github/workflows/cmake-multi-platform.yml.stylua.toml.vimrcdata/chestgen.luadata/layoutparser.luadata/mapgen.luadata/maproombuilder.luadata/menumapgen.luadata/monstergen.luadata/trapgen.lua
💤 Files with no reviewable changes (1)
- .vimrc
| for i = 1, #layoutfile do | ||
| local c = layoutfile:sub(i + 1, i + 1) |
There was a problem hiding this comment.
Critical: Off-by-one error in string indexing.
The loop iterates i from 1 to #layoutfile, but then accesses layoutfile:sub(i + 1, i + 1), which means:
- Position 1 (first character) is skipped
- Position
#layoutfile + 1(beyond the string) is accessed on the last iteration
This will cause incorrect parsing of the layout file.
🐛 Proposed fix
for i = 1, `#layoutfile` do
- local c = layoutfile:sub(i + 1, i + 1)
+ local c = layoutfile:sub(i, i)
if c ~= " " and c ~= "\n" and c ~= "\r" then📝 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.
| for i = 1, #layoutfile do | |
| local c = layoutfile:sub(i + 1, i + 1) | |
| for i = 1, `#layoutfile` do | |
| local c = layoutfile:sub(i, i) | |
| if c ~= " " and c ~= "\n" and c ~= "\r" then |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/layoutparser.lua` around lines 14 - 15, The for-loop over layoutfile
uses Lua's 1-based indexing but calls layoutfile:sub(i + 1, i + 1), skipping the
first character and reading past the end on the last iteration; fix this in
layoutparser.lua by changing the substring access to layoutfile:sub(i, i) (or
alternatively adjust the loop bounds if you prefer) so that the loop variable i
correctly selects the i-th character from layoutfile and avoids out-of-range
accesses.
There was a problem hiding this comment.
The logic hasn't changed at all. Only the formatting.
There was a problem hiding this comment.
@LiquidityC, you're right — the logic is identical between layoutfile:sub(i+1, i+1) and layoutfile:sub(i + 1, i + 1). This PR only adds whitespace around the operators as part of the StyLua formatting enforcement. I apologize for the noise.
That said, the off-by-one behavior (skipping the first character and accessing index #layoutfile + 1 on the last iteration) does appear to be a pre-existing issue in the original code, unrelated to this PR. Would you like me to open a separate GitHub issue to track that?
| room.decor[6][offset + 2] = lightDecor.candle1 | ||
| end | ||
| if random(2) == 1 then | ||
| room.decor[6][offset+2] = lightDecor.candle1 | ||
| room.decor[6][offset + 2] = lightDecor.candle1 |
There was a problem hiding this comment.
Second candle placement targets the same tile twice.
Line 255 repeats Line 252 (room.decor[6][offset + 2]), so the second random branch has no distinct effect.
Proposed fix
if random(2) == 1 then
room.decor[6][offset + 2] = lightDecor.candle1
end
if random(2) == 1 then
- room.decor[6][offset + 2] = lightDecor.candle1
+ room.decor[9][offset + 2] = lightDecor.candle1
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.
| room.decor[6][offset + 2] = lightDecor.candle1 | |
| end | |
| if random(2) == 1 then | |
| room.decor[6][offset+2] = lightDecor.candle1 | |
| room.decor[6][offset + 2] = lightDecor.candle1 | |
| room.decor[6][offset + 2] = lightDecor.candle1 | |
| end | |
| if random(2) == 1 then | |
| room.decor[9][offset + 2] = lightDecor.candle1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/maproombuilder.lua` around lines 252 - 255, The second candle placement
currently writes to the same tile as the first (room.decor[6][offset + 2] =
lightDecor.candle1), so change the coordinate used in the random(2) branch to a
different tile (e.g., room.decor[6][offset + 3] or another adjacent column) so
the two branches place candles on distinct tiles; update the assignment that
references room.decor[6][offset + 2] inside the random(2) == 1 branch to the new
target coordinate while keeping the same value lightDecor.candle1 and leaving
offset/random logic intact.
LiquidityC
force-pushed
the
enforce_lua_formatting
branch
from
2c62730 to
4ae23a7
Compare
LiquidityC
force-pushed
the
enforce_lua_formatting
branch
from
4ae23a7 to
db35f40
Compare
Adds git job to enforce lua formatting.
1 participant
Summary by CodeRabbit