Annotate LoggingStats, DebugMoveCamera, and KeyEvent#7068
Annotate LoggingStats, DebugMoveCamera, and KeyEvent#7068lL1l1 wants to merge 5 commits intoFAForever:developfrom
Conversation
these things have console commands too
lL1l1
added
the
area: documentation
📝 WalkthroughWalkthroughThis pull request adds type annotations and documentation improvements across several files. The changes introduce a Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
Suggested reviewers
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)
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: 1
🧹 Nitpick comments (1)
engine/User/CMauiEdit.lua (1)
178-178: Scope theKeyCodemapping note to mouse events.
As written, it reads as universal. Clarifying “for mouse button events” avoids ambiguity.Suggested doc tweak
----@field KeyCode number # 0 = nothing, 1 = left click, 2 = middle click, 3 = right click +---@field KeyCode number # For mouse button events: 0 = none, 1 = left, 2 = middle, 3 = right🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@engine/User/CMauiEdit.lua` at line 178, The KeyCode field's comment is ambiguous; narrow it to mouse button events by updating the docstring for the KeyCode field (the `@field` KeyCode number comment in CMauiEdit.lua) to read something like "0 = nothing, 1 = left mouse button, 2 = middle mouse button, 3 = right mouse button (for mouse button events)" so it clearly applies only to mouse inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@engine/Core.lua`:
- Around line 69-72: The EndLoggingStats annotation incorrectly documents an
`@param exit boolean?` that doesn't match actual usage; locate the
EndLoggingStats function and either remove or correct the `@param exit`
annotation so it reflects that EndLoggingStats is called with no arguments (or,
if the parameter is intended, update EndLoggingStats' signature/uses to accept
and honor an `exit` boolean), and ensure the BeginLoggingStats and
EndLoggingStats annotations consistently describe their real parameters and
behavior.
---
Nitpick comments:
In `@engine/User/CMauiEdit.lua`:
- Line 178: The KeyCode field's comment is ambiguous; narrow it to mouse button
events by updating the docstring for the KeyCode field (the `@field` KeyCode
number comment in CMauiEdit.lua) to read something like "0 = nothing, 1 = left
mouse button, 2 = middle mouse button, 3 = right mouse button (for mouse button
events)" so it clearly applies only to mouse inputs.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae6cb576-d201-4336-a9e9-0d2a5537feea
📒 Files selected for processing (4)
changelog/snippets/other.7068.mdengine/Core.luaengine/User/CMauiEdit.lualua/SimSync.lua
| --- Begins the SupComMark performance benchmark. | ||
| ---@see EndLoggingStats # End the benchmark and save results to disk. | ||
| ---@param filename string # The name (with extension) for the resulting timestamped benchmark filename. | ||
| function BeginLoggingStats(filename) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate all declarations/comments and call sites.
rg -n -C2 '\b(BeginLoggingStats|EndLoggingStats)\b' engine lua
# 2) Check for zero-argument BeginLoggingStats usage (would conflict with required filename annotation).
rg -nP '\bBeginLoggingStats\s*\(\s*\)' engine lua || true
# 3) Check how EndLoggingStats is typically invoked (true/false/nil).
rg -nP '\bEndLoggingStats\s*\(\s*(true|false)?\s*\)' engine lua || trueRepository: FAForever/fa
Length of output: 820
🏁 Script executed:
# Search for actual invocations of BeginLoggingStats and EndLoggingStats across the entire codebase
rg -n 'BeginLoggingStats|EndLoggingStats' --type lua -A 1 -B 1
# Also search broadly without type restriction in case calls are in non-.lua files
rg -n 'BeginLoggingStats|EndLoggingStats' -A 1 -B 1 | head -100Repository: FAForever/fa
Length of output: 2351
🏁 Script executed:
# Count how many times each function is called
echo "=== BeginLoggingStats call count ==="
rg 'BeginLoggingStats\s*\(' --count-matches
echo "=== EndLoggingStats call count ==="
rg 'EndLoggingStats\s*\(' --count-matchesRepository: FAForever/fa
Length of output: 259
🏁 Script executed:
# Look for specific invocation patterns - find lines that call these functions
rg -n '(BeginLoggingStats|EndLoggingStats)\s*\(' --type lua -o -A 0 | head -50Repository: FAForever/fa
Length of output: 265
Line 117 annotation for EndLoggingStats is inaccurate and misleading.
The documentation states @param exit boolean? # Exits the application iff false, suggesting the exit parameter controls application behavior. However, the only actual call site (testmaps/PerfTestOriginal/perftest_script.lua:139) invokes it as EndLoggingStats() with no arguments, making the annotated parameter semantics irrelevant or wrong.
In contrast, Line 71's filename annotation for BeginLoggingStats is accurate—the single callsite passes 'perftest.log' as required.
Clarify the EndLoggingStats annotation to match actual behavior: either remove/correct the exit parameter description, or update the code to reflect the intended signature.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/Core.lua` around lines 69 - 72, The EndLoggingStats annotation
incorrectly documents an `@param exit boolean?` that doesn't match actual usage;
locate the EndLoggingStats function and either remove or correct the `@param
exit` annotation so it reflects that EndLoggingStats is called with no arguments
(or, if the parameter is intended, update EndLoggingStats' signature/uses to
accept and honor an `exit` boolean), and ensure the BeginLoggingStats and
EndLoggingStats annotations consistently describe their real parameters and
behavior.
3 participants
Description of the proposed changes
I looked in the engine to see what these do when I was looking at console commands:
Was working on a UI mod that interacts with worldview to annotate KeyEvent.
Testing done on the proposed changes
Additional context
Checklist
Summary by CodeRabbit
New Features
Documentation