feat: add configurable player name - #9
Conversation
WalkthroughThe change adds configurable player names with validation and migration support. It exposes the name through the host API, patches relocated image references at runtime, updates examples and documentation, and adds configuration and image-patching tests. ChangesPlayer Name Support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant s3e_config
participant main
participant codboz_override_player_name
participant LoadedImage
s3e_config->>main: return configured player name
main->>codboz_override_player_name: pass relocated image and name address
codboz_override_player_name->>LoadedImage: validate metadata and patch reference
LoadedImage-->>codboz_override_player_name: return patch status
codboz_override_player_name-->>main: return success or failure
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
We've triggered an ultrareview automatically — Running ultrareview automatically — This PR introduces a configurable player name with memory patching of the game binary at specific offsets; a bug in offset validation or patching logic could crash the game or allow invalid names, affecting multiplayer identity and data integrity.. I'll post findings when complete. An ultrareview is cubic's deepest review, catching hard-to-find bugs in the most critical PRs. It runs a longer, multi-pass analysis using cubic's most capable review models, and typically takes around 30 minutes. It consumes your team's reviewed-lines allowance at 3× the standard rate. Automated ultrareviews are disabled by default. We triggered this run as part of your trial. Want cubic to do this for every high-risk PR? Enable auto-ultrareview in your settings. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packaging/ports/codboz/CODBOZ.sh`:
- Around line 50-51: Update the player_name append logic in CODBOZ.sh to ensure
a line boundary before the new setting, including when the existing config lacks
a trailing newline. Add a launcher test covering config.txt ending with
voice_chat=1 without a final newline, and verify both settings remain separately
parsed.
In `@tests/s3e_config_test.c`:
- Around line 131-137: Update the test around codboz_override_player_name so the
invalid-format case reaches the format validation rather than failing earlier on
the patched relative reference. Either move the memory[format_offset] = 'X'
assertion before the successful patch, or restore the original relative
reference in memory before the second call, ensuring the test specifically
detects removal of memcmp validation.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d5aa47d4-477f-4ef8-a09a-50c6aa290d71
📒 Files selected for processing (12)
MakefileREADME.mdinclude/s3e_host.hinclude/s3e_image.hpackaging/ports/codboz/CODBOZ.shpackaging/ports/codboz/README.mdpackaging/ports/codboz/codboz/config.example.txtsrc/codboz_assets.csrc/main.csrc/s3e_config.ctests/portmaster_launcher_test.shtests/s3e_config_test.c
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ultrareview completed in 16m 1s
1 issue found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/s3e_config_test.c">
<violation number="1" location="tests/s3e_config_test.c:117">
P2: The offset constants at the start of `test_player_name_patch` (`0x18f74c`, `0x18f60a`, `0x3af131`) duplicate the `PLAYER_NAME_REFERENCE_OFFSET`, `PLAYER_NAME_REFERENCE_PC_OFFSET`, and `PLAYER_NAME_FORMAT_OFFSET` enums defined in `src/codboz_assets.c`. If the game binary version changes and these offsets are updated in the implementation, nothing forces a corresponding update in the test — the test would silently verify against stale values. Consider moving these constants into a shared header (e.g., `s3e_image.h` or a new `codboz_patch_offsets.h`) so both the implementation and the test reference the same definitions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| } | ||
|
|
||
| static void test_player_name_patch(void) { | ||
| enum { |
There was a problem hiding this comment.
P2: The offset constants at the start of test_player_name_patch (0x18f74c, 0x18f60a, 0x3af131) duplicate the PLAYER_NAME_REFERENCE_OFFSET, PLAYER_NAME_REFERENCE_PC_OFFSET, and PLAYER_NAME_FORMAT_OFFSET enums defined in src/codboz_assets.c. If the game binary version changes and these offsets are updated in the implementation, nothing forces a corresponding update in the test — the test would silently verify against stale values. Consider moving these constants into a shared header (e.g., s3e_image.h or a new codboz_patch_offsets.h) so both the implementation and the test reference the same definitions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/s3e_config_test.c, line 117:
<comment>The offset constants at the start of `test_player_name_patch` (`0x18f74c`, `0x18f60a`, `0x3af131`) duplicate the `PLAYER_NAME_REFERENCE_OFFSET`, `PLAYER_NAME_REFERENCE_PC_OFFSET`, and `PLAYER_NAME_FORMAT_OFFSET` enums defined in `src/codboz_assets.c`. If the game binary version changes and these offsets are updated in the implementation, nothing forces a corresponding update in the test — the test would silently verify against stale values. Consider moving these constants into a shared header (e.g., `s3e_image.h` or a new `codboz_patch_offsets.h`) so both the implementation and the test reference the same definitions.</comment>
<file context>
@@ -101,6 +102,42 @@ static void test_voice_chat_opt_in(void) {
+}
+
+static void test_player_name_patch(void) {
+ enum {
+ reference_offset = 0x18f74c,
+ reference_pc_offset = 0x18f60a,
</file context>
There was a problem hiding this comment.
The game binary never changes. This loader support one exact version, and will never support any other version. Checksum of the apk is also locked
Incremental builds will now correctly rebuild s3e_config_test when s3e_image.h changes, since both the test and codboz_assets.c include this header.
Reorder test_player_name_patch to test format validation before the successful patch. This ensures the memcmp guard is actually tested, rather than failing early on the relative reference check.
1 participant
Summary by cubic
Add a configurable player name for CODBOZ online play. We read
player_namefrom config, validate it, and patch the game to replace the default "Player-%d". Falls back to "Player" if invalid.New Features
player_name(max 13 chars; letters, numbers, spaces,-,_,.).s3e_host_player_name()andcodboz_override_player_name().player_name=Player; README linkscod-boz-online.Bug Fixes
player_namewith a leading newline to avoid corrupting existing configs.s3e_image.handsrc/codboz_assets.cdeps tos3e_config_test.Written for commit d7346cb. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes