Add lua event listener for CPU::Interrupt and scheduleInterrupt#1996
Open
johnbaumann wants to merge 4 commits intogrumpycoders:mainfrom
Open
Add lua event listener for CPU::Interrupt and scheduleInterrupt#1996johnbaumann wants to merge 4 commits intogrumpycoders:mainfrom
johnbaumann wants to merge 4 commits intogrumpycoders:mainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds Lua-driven CPU interrupt support: new Changes
Sequence DiagramsequenceDiagram
participant LuaScript as Lua Script
participant LuaFFI as Lua FFI
participant CppBridge as C++ Binding
participant CPU as R3000A CPU
participant EventBus as Event Bus
LuaScript->>LuaFFI: scheduleInterrupt(eCycle)
LuaFFI->>CppBridge: validated eCycle
CppBridge->>CPU: scheduleInterrupt(PSXINT_LUA, eCycle)
CPU->>CPU: record target cycle for PSXINT_LUA
CPU->>EventBus: when firing -> emit Events::CPU::Interrupt {}
EventBus->>LuaScript: dispatch "CPU::Interrupt" to listeners
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/pcsxffi.lua`:
- Around line 176-179: The scheduleInterrupt function currently accepts any Lua
number; change its validation to ensure eCycle is an integer within uint32 range
before calling C.scheduleInterrupt: verify type(eCycle) == 'number', confirm
it's an integer (e.g. eCycle == math.floor(eCycle)), and enforce 0 <= eCycle <=
0xFFFFFFFF, otherwise raise an error with a clear message; then pass the
validated value to C.scheduleInterrupt to avoid undefined FFI conversions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 551567ca-5460-4ec3-8fa0-8a3a424c91f3
📒 Files selected for processing (8)
src/core/eventslua.ccsrc/core/pcsxffi.luasrc/core/pcsxlua.ccsrc/core/r3000a.ccsrc/core/r3000a.hsrc/core/system.hsrc/lua/luawrapper.ccsrc/lua/luawrapper.h
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/core/pcsxffi.lua (1)
176-179:⚠️ Potential issue | 🟠 Major
eCyclestill allows fractional/NaN inputs beforeuint32_tFFI conversion.Line 177-178 checks type and range only. Values like
1.5and0/0can slip through and produce truncation/undefined conversion behavior at the FFI boundary.🔧 Proposed fix
local function scheduleInterrupt(eCycle) if type(eCycle) ~= 'number' then error 'PCSX.scheduleInterrupt requires a numeric cycle count' end - if (eCycle < 0) or (eCycle > 0xffffffff) then - error 'PCSX.scheduleInterrupt cycle count parameter should be an unsigned 32-bit integer' + if (eCycle ~= eCycle) or (eCycle < 0) or (eCycle > 0xffffffff) or (eCycle % 1 ~= 0) then + error 'PCSX.scheduleInterrupt cycle count parameter should be a finite unsigned 32-bit integer' end C.scheduleInterrupt(eCycle) end#!/bin/bash # Verify integer/NaN guards are present (or absent) in scheduleInterrupt validation. rg -n "local function scheduleInterrupt|eCycle ~= eCycle|eCycle % 1|0xffffffff|C.scheduleInterrupt" src/core/pcsxffi.lua -A2 -B2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/pcsxffi.lua` around lines 176 - 179, The scheduleInterrupt function currently only checks type and range, allowing fractional or NaN values to reach the FFI call; update local function scheduleInterrupt(eCycle) to reject non-integers and NaN by adding explicit checks (e.g., ensure eCycle == eCycle to reject NaN and eCycle % 1 == 0 or math.floor(eCycle) == eCycle to enforce integer) before the existing 0..0xffffffff range check, then call C.scheduleInterrupt only after these validations succeed so the FFI receives a valid unsigned 32-bit integer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/pcsxffi.lua`:
- Around line 176-180: The function scheduleInterrupt contains an unclosed if:
after checking (eCycle < 0) or (eCycle > 0xffffffff) you must add a closing end
before calling C.scheduleInterrupt; update the scheduleInterrupt function so the
numeric-range if block is properly terminated (i.e., insert the missing end to
close the if that validates eCycle) so the function reads: validate type,
validate range (with its own end), then call C.scheduleInterrupt(eCycle).
---
Duplicate comments:
In `@src/core/pcsxffi.lua`:
- Around line 176-179: The scheduleInterrupt function currently only checks type
and range, allowing fractional or NaN values to reach the FFI call; update local
function scheduleInterrupt(eCycle) to reject non-integers and NaN by adding
explicit checks (e.g., ensure eCycle == eCycle to reject NaN and eCycle % 1 == 0
or math.floor(eCycle) == eCycle to enforce integer) before the existing
0..0xffffffff range check, then call C.scheduleInterrupt only after these
validations succeed so the FFI receives a valid unsigned 32-bit integer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b7f03e25-8656-42c2-aee9-223d640b75c4
📒 Files selected for processing (1)
src/core/pcsxffi.lua
Comment on lines
+176
to
+180
| local function scheduleInterrupt(eCycle) | ||
| if type(eCycle) ~= 'number' then error 'PCSX.scheduleInterrupt requires a numeric cycle count' end | ||
| if (eCycle < 0) or (eCycle > 0xffffffff) then error 'PCSX.scheduleInterrupt cycle count parameter should be an unsigned 32-bit integer' | ||
| C.scheduleInterrupt(eCycle) | ||
| end |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the exact changed block and confirm missing `end` on the second if.
sed -n '176,181p' src/core/pcsxffi.lua
python - <<'PY'
from pathlib import Path
line178 = Path("src/core/pcsxffi.lua").read_text().splitlines()[177] # 1-based line 178
print("Line 178:", line178)
print("Ends with 'end':", line178.rstrip().endswith("end"))
PYRepository: grumpycoders/pcsx-redux
Length of output: 560
Add missing end to close the if block on line 178.
Line 178 opens an if statement but does not close it, creating a syntax error that prevents the Lua chunk from loading.
🔧 Proposed fix
local function scheduleInterrupt(eCycle)
if type(eCycle) ~= 'number' then error 'PCSX.scheduleInterrupt requires a numeric cycle count' end
- if (eCycle < 0) or (eCycle > 0xffffffff) then error 'PCSX.scheduleInterrupt cycle count parameter should be an unsigned 32-bit integer'
+ if (eCycle < 0) or (eCycle > 0xffffffff) then
+ error 'PCSX.scheduleInterrupt cycle count parameter should be an unsigned 32-bit integer'
+ end
C.scheduleInterrupt(eCycle)
end🧰 Tools
🪛 Luacheck (1.2.0)
[error] 179-179: expected 'end' (to close 'if' on line 178) near 'C' (indentation-based guess)
(E011)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/pcsxffi.lua` around lines 176 - 180, The function scheduleInterrupt
contains an unclosed if: after checking (eCycle < 0) or (eCycle > 0xffffffff)
you must add a closing end before calling C.scheduleInterrupt; update the
scheduleInterrupt function so the numeric-range if block is properly terminated
(i.e., insert the missing end to close the if that validates eCycle) so the
function reads: validate type, validate range (with its own end), then call
C.scheduleInterrupt(eCycle).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding a method for scheduling callbacks into Lua based on elapsed CPU cycles. Builds on existing scheduler and event listener methods.