feat: 通过注册表设置游戏分辨率,替代无效的命令行参数#580
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough在启动时通过写入 Windows 注册表强制设置并备份游戏分辨率/全屏设置(替代命令行 -screen-* 参数);新增 RegistryPatch 工具以备份/写入/恢复注册表;在关闭流程前尝试恢复注册表值并继续原有关闭逻辑。 Changes分辨率管理与注册表工具
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Launcher as OpenGame
participant Registry as WindowsRegistry
participant Game as GameProcess
User->>Launcher: 含分辨率参数的启动请求
Launcher->>Registry: 调用 RegistryPatch.backup_and_set(分辨率/全屏值)
Registry-->>Launcher: 备份并写入注册表(成功/失败)
Launcher->>Game: 启动游戏进程(移除 -screen-* 参数)
Note right of Game: 游戏读取注册表并以该分辨率/显示模式运行
sequenceDiagram
autonumber
participant Controller as SrPcController
participant Restore as restore_resolution_registry
participant Registry as WindowsRegistry
participant Base as BaseController.close_game
Controller->>Restore: 在 close_game 前调用
Restore->>Registry: 调用 RegistryPatch.restore()
Registry-->>Restore: 恢复结果或抛出 OSError
Restore-->>Controller: 返回/抛出(异常被捕获并记录)
Controller->>Base: 继续调用父类 close_game()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
This PR implements a registry-based approach to set game resolution for Star Rail (崩坏:星穹铁道), replacing ineffective command-line parameters. The game ignores the -screen-width, -screen-height, and -screen-fullscreen command-line arguments, so the solution writes resolution settings directly to the Windows registry before launching the game and restores the original values when the game closes.
Changes:
- Added registry manipulation functions to backup and restore resolution settings
- Modified game launch logic to write resolution to registry instead of using command-line parameters
- Integrated registry restoration into the game close process
- Added logging for borderless window and monitor settings
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/sr_od/operations/enter_game/open_game.py | Implements registry-based resolution setting with backup/restore functionality; removes ineffective command-line resolution parameters |
| src/sr_od/context/sr_pc_controller.py | Overrides close_game method to restore registry values before closing the game window |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/sr_od/context/sr_pc_controller.py`:
- Around line 221-224: The call to restore_resolution_registry() inside
close_game can raise an exception and prevent super().close_game() from running;
wrap restore_resolution_registry() in a try/finally (inside the close_game
method) so that super().close_game() is always executed in the finally block,
and optionally catch/log the exception from restore_resolution_registry() before
re-raising or continuing.
In `@src/sr_od/operations/enter_game/open_game.py`:
- Around line 30-44: restore_resolution_registry currently lacks exception
handling and doesn't remove registry values that were newly created; wrap the
registry restoration in try/except to catch OSError around both
CreateKeyEx/SetValueEx/DeleteValue operations, log warnings and continue on
per-value errors, and ensure the function always clears the globals
_backup_subkey and _backup_values (use a try/finally so super().close_game()
won't be skipped). When restoring, detect entries that were absent in the
original backup (e.g., the backup stored None or a sentinel for missing keys)
and call winreg.DeleteValue for those names instead of SetValueEx; for keys that
existed, call SetValueEx with the original (value, reg_type). Ensure all
registry operations reference the same subkey stored in _backup_subkey and that
the globals are set to None in the finally block and a single info log
('注册表分辨率设置已恢复') is emitted.
- Around line 70-84: The backup logic must record which registry values
originally existed and ensure new values can be removed on restore; change the
values dict creation to pre-populate every name in _REG_VALUE_NAMES with None
(meaning "didn't exist") and then overwrite entries for names returned by
winreg.EnumValue so _backup_values always captures full key set; always set
_backup_subkey = subkey (not None) and assign _backup_values = values so
restore_resolution_registry() can delete any keys whose original value is None;
also wrap winreg.CreateKeyEx and each winreg.SetValueEx call in try/except
OSError blocks that log the error (use log.error) and continue to allow
open_game() and subprocess.Popen to proceed; finally update the global type
annotation for _backup_values/_backup_subkey to reflect Optional[dict[str,
tuple[bytes|int, int] | None]] or similar so type checkers accept the None
entries.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/sr_od/operations/enter_game/open_game.py (2)
84-85: 手动拼接 JSON 字符串存在脆弱性风险Line 84 手动构造 JSON 字符串并附加
\0终止符。虽然当前width/height为整数、is_full_screen为布尔值,格式不会出错,但建议使用json.dumps以提高可维护性和可读性。♻️ 建议重构
+import json ... - json_str = f'{{"width":{width},"height":{height},"isFullScreen":{str(is_full_screen).lower()}}}\0' - winreg.SetValueEx(key, _REG_GRAPHICS_SETTINGS, 0, winreg.REG_BINARY, json_str.encode('ascii')) + settings = {"width": width, "height": height, "isFullScreen": is_full_screen} + json_bytes = (json.dumps(settings, separators=(',', ':')) + '\0').encode('ascii') + winreg.SetValueEx(key, _REG_GRAPHICS_SETTINGS, 0, winreg.REG_BINARY, json_bytes)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sr_od/operations/enter_game/open_game.py` around lines 84 - 85, The code builds json_str manually then writes it as REG_BINARY; instead use json.dumps to serialize the dict of width, height, and isFullScreen (use the existing variables width, height, is_full_screen), then append the null terminator and encode to ASCII before calling winreg.SetValueEx with _REG_GRAPHICS_SETTINGS; update the creation of json_str and the call site in the same block (the variable json_str and winreg.SetValueEx invocation) to improve safety and readability.
25-27: 模块级全局可变状态存在潜在的多实例/多线程风险
_backup_subkey和_backup_values作为模块级全局变量,如果同时管理多个游戏账号或在多线程环境下调用,可能导致备份数据被覆盖或恢复错误的注册表值。当前使用场景可能是单实例,但值得留意。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sr_od/operations/enter_game/open_game.py` around lines 25 - 27, The module-level mutable globals _backup_subkey and _backup_values create race conditions for concurrent or multi-account use; refactor to remove these globals and store the backup state on a per-session or per-instance object (e.g., add attributes like self._backup_subkey and self._backup_values on the class that performs open/close operations, or encapsulate in an OpenGameSession/RegistryBackup object passed through the functions), or if truly multithreaded use threading.local() to keep backups thread-local; update all functions in open_game.py that read/write _backup_subkey/_backup_values to use the new instance/context storage instead of the module-level variables.
🤖 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/sr_od/operations/enter_game/open_game.py`:
- Around line 113-114: 在 open_game 中对 screen_size 的解析缺乏防护,split('x') 或 int()
在配置异常时会抛出未捕获异常;请在解析 screen_size(用于计算 screen_width 和
screen_height)的逻辑周围添加验证/异常处理:先确认 screen_size 是非空字符串并包含单个 'x',分割后有两个部分,再对每部分进行
int 转换并捕获 ValueError/IndexError;在出错时使用安全回退值(例如默认
'1920x1080')或抛出带有明确上下文的异常并记录错误信息以便排查。确保引用到的标识符为
screen_size、screen_width、screen_height 和 open_game。
---
Duplicate comments:
In `@src/sr_od/operations/enter_game/open_game.py`:
- Around line 76-82: The code currently collapses two different states (subkey
absent vs subkey present-but-empty) by setting _backup_subkey and _backup_values
to None when values is falsy, causing restore_resolution_registry() to skip
cleanup of newly created values; fix by explicitly recording whether the
registry subkey existed before writing: determine subkey existence before
reading (or check winreg.OpenKey failure) and then set _backup_subkey = subkey
always when the subkey existed, set _backup_values = values (empty dict) when
the subkey existed but had no matching entries, and set _backup_subkey = subkey
and _backup_values = None only when the subkey did not exist (or vice‑versa
depending on your sentinel convention), then update
restore_resolution_registry() to treat _backup_values == None as “originally
missing” (so delete any created values/subkey) and _backup_values == {} as
“originally present but empty” (so remove only values you added); refer to
symbols values, _backup_subkey, _backup_values and the
restore_resolution_registry function to implement this logic.
---
Nitpick comments:
In `@src/sr_od/operations/enter_game/open_game.py`:
- Around line 84-85: The code builds json_str manually then writes it as
REG_BINARY; instead use json.dumps to serialize the dict of width, height, and
isFullScreen (use the existing variables width, height, is_full_screen), then
append the null terminator and encode to ASCII before calling winreg.SetValueEx
with _REG_GRAPHICS_SETTINGS; update the creation of json_str and the call site
in the same block (the variable json_str and winreg.SetValueEx invocation) to
improve safety and readability.
- Around line 25-27: The module-level mutable globals _backup_subkey and
_backup_values create race conditions for concurrent or multi-account use;
refactor to remove these globals and store the backup state on a per-session or
per-instance object (e.g., add attributes like self._backup_subkey and
self._backup_values on the class that performs open/close operations, or
encapsulate in an OpenGameSession/RegistryBackup object passed through the
functions), or if truly multithreaded use threading.local() to keep backups
thread-local; update all functions in open_game.py that read/write
_backup_subkey/_backup_values to use the new instance/context storage instead of
the module-level variables.
|
@coderabbitai以后不要copilot审查代码,你怎么看 |
|
Tip For best results, initiate chat on the files or code changes.
我是 CodeRabbit,不是 GitHub Copilot。这是两个完全不同的 AI 工具:
目前为这个 PR 提供审查的是 CodeRabbit,而不是 Copilot。 如果您希望:
请告诉我您具体希望如何调整,我很乐意帮助您!😊 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/one_dragon/utils/reg_utils.py`:
- Around line 54-64: The code clears self._backup before attempting registry
restore, so if winreg.CreateKeyEx/SetValueEx raises an OSError the original
backup is lost; change the logic in the restore routine (the block that uses
winreg.CreateKeyEx, winreg.SetValueEx and logs via log.info/log.exception) to
defer clearing self._backup until after the restore succeeds (i.e., only set
self._backup = None after the with block completes without exception), and
ensure exceptions are caught and logged with log.exception without mutating
self._backup so retries remain possible.
In `@src/sr_od/operations/enter_game/open_game.py`:
- Around line 63-68: Before assigning a new RegistryPatch to the module-global
_resolution_patch, restore any existing patch to avoid losing its original
backup: if _resolution_patch is truthy, call its restore method (e.g.,
_resolution_patch.restore() or _resolution_patch.restore_backup()/revert()
depending on the class API) and handle errors, then proceed to create
RegistryPatch(subkey), call backup_and_set(values) and only on success assign
_resolution_patch = patch; update the code around RegistryPatch, backup_and_set
and _resolution_patch to perform this restore-before-overwrite sequence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 469fe6a8-4027-4f94-bf71-2c5f219fe3cb
📒 Files selected for processing (3)
src/one_dragon/utils/reg_utils.pysrc/sr_od/context/sr_pc_controller.pysrc/sr_od/operations/enter_game/open_game.py
|
已测试最终效果 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/sr_od/operations/enter_game/open_game.py (1)
63-71:⚠️ Potential issue | 🟠 Major恢复失败后不要继续覆盖
_resolution_patch。
RegistryPatch.restore()失败时只会记录日志并保留备份。现在 Line 64-71 仍会继续创建并赋值新的patch,把旧补丁里尚未恢复的原始快照直接覆盖掉。restore()之后应检查has_backup;如果仍为True,应中止本次改写或至少保留旧补丁不被覆盖。🛠️ 建议修复
global _resolution_patch if _resolution_patch is not None: _resolution_patch.restore() + if _resolution_patch.has_backup: + log.warning('上一次注册表恢复失败,跳过本次分辨率改写') + return patch = RegistryPatch(subkey) if not patch.backup_and_set(values): log.warning('注册表分辨率设置失败,将继续使用当前设置') return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sr_od/operations/enter_game/open_game.py` around lines 63 - 71, If the previous patch's restore() failed (left its backup intact) we must not overwrite the existing _resolution_patch; after calling _resolution_patch.restore() check _resolution_patch.has_backup and if it is still True abort the operation (return) so the old patch remains; only create and assign a new RegistryPatch (variable patch) and set _resolution_patch after patch.backup_and_set(values) returns True. Ensure the code references _resolution_patch, RegistryPatch.restore(), RegistryPatch.has_backup and RegistryPatch.backup_and_set(values) accordingly.
🤖 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/one_dragon/utils/reg_utils.py`:
- Around line 34-47: The code currently clears self._backup on OSError which
loses the only rollback snapshot; instead do not set self._backup = None in the
except block and perform a best-effort restore using the saved snapshot: open
the same key via winreg.CreateKeyEx(self._root, self._subkey, ...) and iterate
over self._backup to call winreg.SetValueEx for each name/type/data (and
optionally remove values that were not in the backup), catching and logging any
exceptions; ensure you log restore failures with context (self._subkey) and
still return False if restore fails or partial, leaving self._backup intact for
callers to inspect.
---
Duplicate comments:
In `@src/sr_od/operations/enter_game/open_game.py`:
- Around line 63-71: If the previous patch's restore() failed (left its backup
intact) we must not overwrite the existing _resolution_patch; after calling
_resolution_patch.restore() check _resolution_patch.has_backup and if it is
still True abort the operation (return) so the old patch remains; only create
and assign a new RegistryPatch (variable patch) and set _resolution_patch after
patch.backup_and_set(values) returns True. Ensure the code references
_resolution_patch, RegistryPatch.restore(), RegistryPatch.has_backup and
RegistryPatch.backup_and_set(values) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5dd5a48e-bd75-4e48-91f2-23b5b93a35dd
📒 Files selected for processing (2)
src/one_dragon/utils/reg_utils.pysrc/sr_od/operations/enter_game/open_game.py
232644a to
e8eb956
Compare
…n into fix/parameters
…n into fix/parameters
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/one_dragon/utils/reg_utils.py`:
- Around line 38-42: The snapshot currently built by _read_values only includes
existing value names so backup_and_set() doesn't remember names that were absent
and restore() won't remove values it created; modify the snapshot logic in
backup_and_set()/ _read_values to accept the full set of target names
(values.keys()) and record a sentinel (e.g., a special marker or None) for names
that don't exist, and then update restore() to treat that sentinel as "delete
this value" (call winreg.DeleteValue or equivalent) while restoring actual data
for entries that were present; ensure functions referenced: backup_and_set(),
_read_values(), _backup, and restore() are updated consistently so
originally-missing registry values are removed on restore.
🪄 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: CHILL
Plan: Pro
Run ID: 6fd0aab3-a89d-45f1-9b04-bcc8a135b6d2
📒 Files selected for processing (1)
src/one_dragon/utils/reg_utils.py
fix #492
星穹铁道游戏内设置会覆盖 -screen-width/-screen-height/-screen-fullscreen
命令行参数,改为通过注册表写入分辨率和显示模式。
Summary by CodeRabbit
新增功能
修复