refactor(client): extract demo subsystem into cl_demo module#155
refactor(client): extract demo subsystem into cl_demo module#155
Conversation
Move record/demo/stoprecord commands, demo file I/O, and net-packet demowriting from cl_main.c into cl_demo.c. cl_main calls CL_Demo_InitCommands/Shutdown and CL_Demo_WriteServerPacket for live recording. Public entry points (CL_ReadDemoMessage, CL_StopRecord_f) remain in cl_demo.c for cl_parse and cgame. Co-authored-by: Tim Fox <timfox@outlook.com>
Co-authored-by: Tim Fox <timfox@outlook.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e908a18138
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RB_BeginDrawingView(); | ||
|
|
||
| #ifdef USE_VULKAN | ||
| vk_forward_plus_upload_refdef(); |
There was a problem hiding this comment.
Call Forward+ upload before entering render pass
vk_forward_plus_upload_refdef() records a transfer barrier and vkCmdCopyBuffer, but this call site runs after RB_BeginDrawingView(). In this renderer path the command buffer is already inside the main render pass (started in vk_begin_frame()), so recording transfer commands here violates Vulkan command-validity rules and can trigger validation errors or device loss when r_forwardPlus=1. Move the upload to a point before the main pass begins (or outside any active render pass).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Stale comment
Security review complete: no high-confidence medium-or-higher vulnerabilities found in the modified code.
Reviewed the demo command inputs, demo file open/write paths, pure-FS bypass usage, and moved playback/recording code against the prior implementation. The risky surfaces identified here appear to be pre-existing behavior preserved by the refactor rather than newly introduced or exposed by this PR. No new inline security findings.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Security review complete. I did not find any high-confidence medium, high, or critical vulnerabilities introduced by this PR.
The demo filename/file parsing paths and nextdemo command execution remain the same behavior as the code previously in cl_main.c; this refactor primarily moves that logic into cl_demo.c and replaces call sites with module init/shutdown and CL_Demo_WriteServerPacket() delegation. No new finding comments are being left.
Sent by Cursor Automation: Find vulnerabilities


Summary
This change modularizes a large, self-contained slice of the client: demo recording and playback (
.dm_*files,record/demo/stoprecordconsole commands, and the logic that copies live server packets into an open demo file).What changed
src/client/cl_demo.candcl_demo.h— all demo I/O, command registration, and helpers that previously lived incl_main.c.cl_main.c— callsCL_Demo_InitCommands()/CL_Demo_ShutdownCommands()fromCL_Init/CL_Shutdown, andCL_Demo_WriteServerPacket()fromCL_PacketEventwhen recording a live connection.docs/ARCHITECTURE.mdlistscl_demoundersrc/client/.Behavior
No intended gameplay or protocol changes: public symbols
CL_ReadDemoMessageandCL_StopRecord_fremain for existing call sites (cl_parse.c,cl_cgame.c).Validation
./scripts/compile_engine.sh vulkan(Release)ctestinbuild-vk-Release— 19/19 passed