feat(input): extend game vibration strength#44
Conversation
|
Warning Review limit reached
More reviews will be available in 6 minutes and 22 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughPR 将手柄震动配置从设备级 Changes游戏串流震动强度功能
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
entry/src/main/ets/service/usbdriver/NativeHidController.ets (1)
667-673: ⚡ Quick win在驱动边界补充 rumble 范围钳制,降低上游异常值外溢风险
这里已修复二次缩放,建议再加一层
0..65535钳制,保证 Native 命令入口自身满足协议范围,不依赖上游永远正确。🔧 建议修改
- // Native parser expects Moonlight rumble values in the same 0..65535 range as AbstractController. + // Native parser expects Moonlight rumble values in the same 0..65535 range as AbstractController. + const clampRumble = (v: number) => Math.max(0, Math.min(65535, Math.round(v))); const cmd = this.nativeParser.createRumbleCommand( this.vendorId, this.productId, - Math.round(lowFreqMotor), - Math.round(highFreqMotor) + clampRumble(lowFreqMotor), + clampRumble(highFreqMotor) );🤖 Prompt for 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. In `@entry/src/main/ets/service/usbdriver/NativeHidController.ets` around lines 667 - 673, The createRumbleCommand call in NativeHidController needs additional defensive clamping for the rumble motor values. Add a clamping layer to ensure both lowFreqMotor and highFreqMotor are bounded to the valid 0..65535 range before being rounded and passed to the createRumbleCommand method. This should be done by applying Math.max and Math.min constraints to each value so that even if upstream provides out-of-range values, the native command entry receives values that comply with the protocol specification.
🤖 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 `@entry/src/main/ets/pages/SettingsPageV2.ets`:
- Around line 529-530: The gamepadVibrationGain property is being assigned
directly from the preferences without validating that it falls within the
acceptable MIN/MAX bounds. When loading the vibration gain value using
PreferencesUtil.get with SettingsKeys.GAMEPAD_VIBRATION_GAIN, immediately clamp
the retrieved value to the valid range (between the minimum and maximum allowed
values for gamepad vibration gain). If the clamping operation modifies the
value, write the corrected value back to the preferences to ensure consistency
between the UI state and stored configuration.
In `@entry/src/main/ets/service/input/GamepadVibrationService.ets`:
- Line 75: The gamepadGain value at line 75 may be NaN, and using
Math.max/Math.min with NaN will propagate the NaN value through to lines 238/243
where it's used in the controller?.rumble() call, resulting in invalid
rumble(NaN, NaN) calls. Add Number.isFinite protection for the gamepadGain
parameter before the clamping operation on line 75 to validate it is a finite
number, and if not finite, use a default valid value instead of allowing NaN to
propagate. Additionally, add similar Number.isFinite checks at the rumble
invocation points (around lines 238/243) to ensure only valid finite values are
passed to the rumble method.
In `@entry/src/main/ets/service/SettingsService.ets`:
- Line 605: The gamepadVibrationGain value read from persistent storage at line
605 (and similarly at line 750) needs to be validated and clamped to the valid
range of 50-200 to ensure consistency with the configuration contract. After
retrieving the persisted value using getNumber with the
GAMEPAD_VIBRATION_GAIN_DEFAULT constant, add boundary validation logic to clamp
the value between 50 and 200 (inclusive) before returning or using it, ensuring
that any out-of-range persisted values do not propagate to downstream components
like StreamConfig and InputSettings. Apply the same normalization logic at both
locations where gamepadVibrationGain values are retrieved.
---
Nitpick comments:
In `@entry/src/main/ets/service/usbdriver/NativeHidController.ets`:
- Around line 667-673: The createRumbleCommand call in NativeHidController needs
additional defensive clamping for the rumble motor values. Add a clamping layer
to ensure both lowFreqMotor and highFreqMotor are bounded to the valid 0..65535
range before being rounded and passed to the createRumbleCommand method. This
should be done by applying Math.max and Math.min constraints to each value so
that even if upstream provides out-of-range values, the native command entry
receives values that comply with the protocol specification.
🪄 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: d57b6a38-cc93-4caf-a104-cdacccda846d
📒 Files selected for processing (7)
entry/src/main/ets/model/StreamConfig.etsentry/src/main/ets/pages/SettingsPageV2.etsentry/src/main/ets/service/SettingsService.etsentry/src/main/ets/service/input/GamepadManager.etsentry/src/main/ets/service/input/GamepadVibrationService.etsentry/src/main/ets/service/usbdriver/NativeHidController.etsentry/src/main/ets/viewmodel/StreamViewModel.ets
|
实现已按最新方向收敛:不再新增单独的手柄震动增益设置,而是复用原有 |
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 `@entry/src/main/ets/service/input/GamepadVibrationService.ets`:
- Around line 66-77: The setSettings method in GamepadVibrationService only
calls stopAll() when vibration is disabled (wasEnabled && !enabled), but it
should also stop vibration when the strength is set to 0 even if enabled remains
true. After normalizing the game vibration strength with
normalizeGameVibrationStrength(strength), add an additional check: if the
resulting strength value is 0, immediately call stopAll() to prevent residual
vibration from continuing when strength is adjusted to zero while the vibration
feature remains enabled.
In `@entry/src/main/ets/service/SettingsService.ets`:
- Around line 603-605: The getNumber() function uses parseInt(value) ||
defaultValue which incorrectly treats the string "0" as a falsy value, causing
it to fall back to the default value instead of returning 0. This breaks
legitimate 0% configuration values. Fix this by modifying the getNumber()
function to explicitly check if the parsed value is null or undefined rather
than relying on falsy evaluation, ensuring that valid 0 values are preserved and
only missing/invalid values trigger the fallback to defaultValue. This fix
applies to both the VIBRATE_FALLBACK_STRENGTH call at line 603 and the similar
pattern at line 748.
🪄 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: 01cd8552-2ea7-4302-b7e3-37aa27bf5740
📒 Files selected for processing (7)
entry/src/main/ets/model/StreamConfig.etsentry/src/main/ets/pages/SettingsPageV2.etsentry/src/main/ets/service/SettingsService.etsentry/src/main/ets/service/input/GamepadManager.etsentry/src/main/ets/service/input/GamepadVibrationService.etsentry/src/main/ets/service/usbdriver/NativeHidController.etsentry/src/main/ets/viewmodel/StreamViewModel.ets
✅ Files skipped from review due to trivial changes (1)
- entry/src/main/ets/service/input/GamepadManager.ets
🚧 Files skipped from review as they are similar to previous changes (2)
- entry/src/main/ets/service/usbdriver/NativeHidController.ets
- entry/src/main/ets/model/StreamConfig.ets
改了啥呀
为啥要改
验证
git diff --checkBuild (debug)passed on earlier PR revisions; latest push will rerun.NODE_PATH=/Users/mac/Program/moonlight-harmony/hvigor/node_modules node hvigorw.js tasks --no-daemon仍受DEVECO_SDK_HOME无效影响未完成。Summary by CodeRabbit
发布说明