feat: HeadphoneVolumeOverride#136
Conversation
审阅者指南引入了一个新的可配置模组 HeadphoneVolumeOverride,它在 HeadphoneVolumeOverride 在 SetAisacControl 上的 Harmony 前缀顺序图sequenceDiagram
participant GameAudioSystem
participant CriAtomExPlayer
participant HeadphoneVolumeOverride
GameAudioSystem->>CriAtomExPlayer: SetAisacControl(controlId, value)
activate CriAtomExPlayer
CriAtomExPlayer->>HeadphoneVolumeOverride: PreSetAisacControl(controlId, ref value)
activate HeadphoneVolumeOverride
alt [controlId == 2 && p1 >= 0]
HeadphoneVolumeOverride->>HeadphoneVolumeOverride: Mathf.Clamp(p1 / 20f, 0f, 1f)
HeadphoneVolumeOverride-->>CriAtomExPlayer: 为 1P 覆盖 value
else [controlId == 3 && p2 >= 0]
HeadphoneVolumeOverride->>HeadphoneVolumeOverride: Mathf.Clamp(p2 / 20f, 0f, 1f)
HeadphoneVolumeOverride-->>CriAtomExPlayer: 为 2P 覆盖 value
else [其他 controlId 或已禁用]
HeadphoneVolumeOverride-->>CriAtomExPlayer: value 不变(保留用户设置)
end
deactivate HeadphoneVolumeOverride
CriAtomExPlayer-->>GameAudioSystem: SetAisacControl 完成
deactivate CriAtomExPlayer
文件级变更
提示和命令与 Sourcery 交互
自定义你的体验访问你的 控制面板 以:
获取帮助Original review guide in EnglishReviewer's GuideIntroduces a new configurable mod, HeadphoneVolumeOverride, that uses a Harmony prefix patch on CriAtomExPlayer.SetAisacControl to clamp and override the 1P/2P headphone AISAC volume controls based on config entries, allowing fixed per-side volumes or deferring to user settings. Sequence diagram for HeadphoneVolumeOverride Harmony prefix on SetAisacControlsequenceDiagram
participant GameAudioSystem
participant CriAtomExPlayer
participant HeadphoneVolumeOverride
GameAudioSystem->>CriAtomExPlayer: SetAisacControl(controlId, value)
activate CriAtomExPlayer
CriAtomExPlayer->>HeadphoneVolumeOverride: PreSetAisacControl(controlId, ref value)
activate HeadphoneVolumeOverride
alt [controlId == 2 && p1 >= 0]
HeadphoneVolumeOverride->>HeadphoneVolumeOverride: Mathf.Clamp(p1 / 20f, 0f, 1f)
HeadphoneVolumeOverride-->>CriAtomExPlayer: value overridden for 1P
else [controlId == 3 && p2 >= 0]
HeadphoneVolumeOverride->>HeadphoneVolumeOverride: Mathf.Clamp(p2 / 20f, 0f, 1f)
HeadphoneVolumeOverride-->>CriAtomExPlayer: value overridden for 2P
else [other controlId or disabled]
HeadphoneVolumeOverride-->>CriAtomExPlayer: value unchanged (user setting)
end
deactivate HeadphoneVolumeOverride
CriAtomExPlayer-->>GameAudioSystem: SetAisacControl completes
deactivate CriAtomExPlayer
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并且给出了一些总体反馈:
- 将 magic controlId 值 2 和 3 替换为具名常量或枚举(例如 Headphone1ControlId/Headphone2ControlId)会更加清晰和安全,这样可以更好地说明它们的含义并避免被意外误用。
- 配置字段名 p1 和 p2 有点不直观;可以考虑将它们重命名为类似 p1HeadphoneVolumeOverride/p2HeadphoneVolumeOverride 的名称,以更好地对应配置描述并提升可读性。
面向 AI 代理的提示
请根据本次代码评审中的评论进行修改:
## 总体评论
- 将 magic controlId 值 2 和 3 替换为具名常量或枚举(例如 Headphone1ControlId/Headphone2ControlId)会更加清晰和安全,这样可以更好地说明它们的含义并避免被意外误用。
- 配置字段名 p1 和 p2 有点不直观;可以考虑将它们重命名为类似 p1HeadphoneVolumeOverride/p2HeadphoneVolumeOverride 的名称,以更好地对应配置描述并提升可读性。
## 单独评论
### 评论 1
<location path="AquaMai.Mods/GameSystem/HeadphoneVolumeOverride.cs" line_range="30-34" />
<code_context>
+ [HarmonyPatch(typeof(CriAtomExPlayer), "SetAisacControl", [typeof(uint), typeof(float)])]
+ public static void PreSetAisacControl(uint controlId, ref float value)
+ {
+ if (controlId == 2 && p1 >= 0f)
+ {
+ value = Mathf.Clamp(p1 / 20f, 0f, 1f);
+ }
+ else if (controlId == 3 && p2 >= 0f)
+ {
+ value = Mathf.Clamp(p2 / 20f, 0f, 1f);
</code_context>
<issue_to_address>
**suggestion:** 考虑将魔法 AISAC 控制 ID 替换为具名常量或枚举。
直接使用像 `2` 和 `3` 这样的原始数值会掩盖它们的含义,并且让理解依赖于外部上下文。通过定义具名常量(例如 `const uint Headphone1ControlId = 2;`)或枚举,可以更清晰地表达意图,并帮助防止在其他地方错误地复用或修改这些 ID。
建议实现方式:
```csharp
[ConfigEntry(
name: "2P 音量覆盖",
en: "2P headphone volume override. -1 = use user setting, 0–20 = fixed volume (20 is max).",
zh: "2P 耳机音量覆盖值。-1 = 使用用户设置,0–20 = 固定音量(20 为最大值)。")]
private static readonly float p2 = -1f;
private const uint Headphone1AisacControlId = 2;
private const uint Headphone2AisacControlId = 3;
[HarmonyPrefix]
```
```csharp
public static void PreSetAisacControl(uint controlId, ref float value)
{
if (controlId == Headphone1AisacControlId && p1 >= 0f)
{
value = Mathf.Clamp(p1 / 20f, 0f, 1f);
}
else if (controlId == Headphone2AisacControlId && p2 >= 0f)
{
value = Mathf.Clamp(p2 / 20f, 0f, 1f);
}
}
```
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据这些反馈来改进评审质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The magic controlId values 2 and 3 would be clearer and safer if replaced with named constants or an enum (e.g., Headphone1ControlId/Headphone2ControlId) to document their meaning and avoid accidental misuse.
- The config field names p1 and p2 are a bit opaque; consider renaming them to something like p1HeadphoneVolumeOverride/p2HeadphoneVolumeOverride to better match the config descriptions and improve readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The magic controlId values 2 and 3 would be clearer and safer if replaced with named constants or an enum (e.g., Headphone1ControlId/Headphone2ControlId) to document their meaning and avoid accidental misuse.
- The config field names p1 and p2 are a bit opaque; consider renaming them to something like p1HeadphoneVolumeOverride/p2HeadphoneVolumeOverride to better match the config descriptions and improve readability.
## Individual Comments
### Comment 1
<location path="AquaMai.Mods/GameSystem/HeadphoneVolumeOverride.cs" line_range="30-34" />
<code_context>
+ [HarmonyPatch(typeof(CriAtomExPlayer), "SetAisacControl", [typeof(uint), typeof(float)])]
+ public static void PreSetAisacControl(uint controlId, ref float value)
+ {
+ if (controlId == 2 && p1 >= 0f)
+ {
+ value = Mathf.Clamp(p1 / 20f, 0f, 1f);
+ }
+ else if (controlId == 3 && p2 >= 0f)
+ {
+ value = Mathf.Clamp(p2 / 20f, 0f, 1f);
</code_context>
<issue_to_address>
**suggestion:** Consider replacing magic AISAC control IDs with named constants or an enum.
Using raw values like `2` and `3` obscures their meaning and couples understanding to external context. Defining named constants (e.g., `const uint Headphone1ControlId = 2;`) or an enum will clarify intent and help prevent incorrect reuse or modification of these IDs elsewhere.
Suggested implementation:
```csharp
[ConfigEntry(
name: "2P 音量覆盖",
en: "2P headphone volume override. -1 = use user setting, 0–20 = fixed volume (20 is max).",
zh: "2P 耳机音量覆盖值。-1 = 使用用户设置,0–20 = 固定音量(20 为最大值)。")]
private static readonly float p2 = -1f;
private const uint Headphone1AisacControlId = 2;
private const uint Headphone2AisacControlId = 3;
[HarmonyPrefix]
```
```csharp
public static void PreSetAisacControl(uint controlId, ref float value)
{
if (controlId == Headphone1AisacControlId && p1 >= 0f)
{
value = Mathf.Clamp(p1 / 20f, 0f, 1f);
}
else if (controlId == Headphone2AisacControlId && p2 >= 0f)
{
value = Mathf.Clamp(p2 / 20f, 0f, 1f);
}
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if (controlId == 2 && p1 >= 0f) | ||
| { | ||
| value = Mathf.Clamp(p1 / 20f, 0f, 1f); | ||
| } | ||
| else if (controlId == 3 && p2 >= 0f) |
There was a problem hiding this comment.
suggestion: 考虑将魔法 AISAC 控制 ID 替换为具名常量或枚举。
直接使用像 2 和 3 这样的原始数值会掩盖它们的含义,并且让理解依赖于外部上下文。通过定义具名常量(例如 const uint Headphone1ControlId = 2;)或枚举,可以更清晰地表达意图,并帮助防止在其他地方错误地复用或修改这些 ID。
建议实现方式:
[ConfigEntry(
name: "2P 音量覆盖",
en: "2P headphone volume override. -1 = use user setting, 0–20 = fixed volume (20 is max).",
zh: "2P 耳机音量覆盖值。-1 = 使用用户设置,0–20 = 固定音量(20 为最大值)。")]
private static readonly float p2 = -1f;
private const uint Headphone1AisacControlId = 2;
private const uint Headphone2AisacControlId = 3;
[HarmonyPrefix] public static void PreSetAisacControl(uint controlId, ref float value)
{
if (controlId == Headphone1AisacControlId && p1 >= 0f)
{
value = Mathf.Clamp(p1 / 20f, 0f, 1f);
}
else if (controlId == Headphone2AisacControlId && p2 >= 0f)
{
value = Mathf.Clamp(p2 / 20f, 0f, 1f);
}
}Original comment in English
suggestion: Consider replacing magic AISAC control IDs with named constants or an enum.
Using raw values like 2 and 3 obscures their meaning and couples understanding to external context. Defining named constants (e.g., const uint Headphone1ControlId = 2;) or an enum will clarify intent and help prevent incorrect reuse or modification of these IDs elsewhere.
Suggested implementation:
[ConfigEntry(
name: "2P 音量覆盖",
en: "2P headphone volume override. -1 = use user setting, 0–20 = fixed volume (20 is max).",
zh: "2P 耳机音量覆盖值。-1 = 使用用户设置,0–20 = 固定音量(20 为最大值)。")]
private static readonly float p2 = -1f;
private const uint Headphone1AisacControlId = 2;
private const uint Headphone2AisacControlId = 3;
[HarmonyPrefix] public static void PreSetAisacControl(uint controlId, ref float value)
{
if (controlId == Headphone1AisacControlId && p1 >= 0f)
{
value = Mathf.Clamp(p1 / 20f, 0f, 1f);
}
else if (controlId == Headphone2AisacControlId && p2 >= 0f)
{
value = Mathf.Clamp(p2 / 20f, 0f, 1f);
}
}There was a problem hiding this comment.
Code Review
This pull request introduces the HeadphoneVolumeOverride class, which uses Harmony patches to override 1P and 2P headphone volumes to fixed values. The review feedback correctly points out that the config fields p1 and p2 should not be marked as readonly because they need to be modified by the config loader at runtime, and static readonly primitive fields can be inlined by the JIT compiler.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| name: "1P 音量覆盖", | ||
| en: "1P headphone volume override. -1 = use user setting, 0–20 = fixed volume (20 is max).", | ||
| zh: "1P 耳机音量覆盖值。-1 = 使用用户设置,0–20 = 固定音量(20 为最大值)。")] | ||
| private static readonly float p1 = -1f; |
There was a problem hiding this comment.
Since this field is intended to be modified by the config loader at runtime, it should not be marked as readonly. In some .NET runtimes or under certain JIT optimization levels, static readonly fields of primitive types can be inlined as constants by the JIT compiler, which would prevent any runtime config updates from taking effect. Removing readonly ensures correctness and avoids potential JIT inlining issues.
private static float p1 = -1f;| name: "2P 音量覆盖", | ||
| en: "2P headphone volume override. -1 = use user setting, 0–20 = fixed volume (20 is max).", | ||
| zh: "2P 耳机音量覆盖值。-1 = 使用用户设置,0–20 = 固定音量(20 为最大值)。")] | ||
| private static readonly float p2 = -1f; |
There was a problem hiding this comment.
Since this field is intended to be modified by the config loader at runtime, it should not be marked as readonly. In some .NET runtimes or under certain JIT optimization levels, static readonly fields of primitive types can be inlined as constants by the JIT compiler, which would prevent any runtime config updates from taking effect. Removing readonly ensures correctness and avoids potential JIT inlining issues.
private static float p2 = -1f;There was a problem hiding this comment.
1 issue found across 1 file
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="AquaMai.Mods/GameSystem/HeadphoneVolumeOverride.cs">
<violation number="1" location="AquaMai.Mods/GameSystem/HeadphoneVolumeOverride.cs:30">
P3: Replace magic AISAC control IDs `2` and `3` with named constants (e.g., `const uint Headphone1AisacControlId = 2;`) to clarify their meaning and prevent accidental misuse.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| [HarmonyPatch(typeof(CriAtomExPlayer), "SetAisacControl", [typeof(uint), typeof(float)])] | ||
| public static void PreSetAisacControl(uint controlId, ref float value) | ||
| { | ||
| if (controlId == 2 && p1 >= 0f) |
There was a problem hiding this comment.
P3: Replace magic AISAC control IDs 2 and 3 with named constants (e.g., const uint Headphone1AisacControlId = 2;) to clarify their meaning and prevent accidental misuse.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At AquaMai.Mods/GameSystem/HeadphoneVolumeOverride.cs, line 30:
<comment>Replace magic AISAC control IDs `2` and `3` with named constants (e.g., `const uint Headphone1AisacControlId = 2;`) to clarify their meaning and prevent accidental misuse.</comment>
<file context>
@@ -0,0 +1,39 @@
+ [HarmonyPatch(typeof(CriAtomExPlayer), "SetAisacControl", [typeof(uint), typeof(float)])]
+ public static void PreSetAisacControl(uint controlId, ref float value)
+ {
+ if (controlId == 2 && p1 >= 0f)
+ {
+ value = Mathf.Clamp(p1 / 20f, 0f, 1f);
</file context>
|
要理ai吗 要的话我可以改成 using HarmonyLib;
using AquaMai.Config.Attributes;
using UnityEngine;
namespace AquaMai.Mods.GameSystem;
[ConfigSection(
name: "耳机音量覆盖",
en: "Override 1P/2P headphone volume to a fixed value, ignoring the user's in-game setting. Set to -1 to disable (use user setting). Range: 0–20.",
zh: "将 1P/2P 耳机音量强制覆盖为固定值,忽略用户的游戏内设置。设为 -1 则禁用(使用用户设置)。范围:0–20。")]
public static class HeadphoneVolumeOverride
{
[ConfigEntry(
name: "1P 音量覆盖",
en: "1P headphone volume override. -1 = use user setting, 0–20 = fixed volume (20 is max).",
zh: "1P 耳机音量覆盖值。-1 = 使用用户设置,0–20 = 固定音量(20 为最大值)。")]
private static readonly float p1HeadphoneVolumeOverride = -1f;
[ConfigEntry(
name: "2P 音量覆盖",
en: "2P headphone volume override. -1 = use user setting, 0–20 = fixed volume (20 is max).",
zh: "2P 耳机音量覆盖值。-1 = 使用用户设置,0–20 = 固定音量(20 为最大值)。")]
private static readonly float p2HeadphoneVolumeOverride = -1f;
private const uint P1HeadphoneAisacControlId = 2;
private const uint P2HeadphoneAisacControlId = 3;
[HarmonyPrefix]
[HarmonyPatch(typeof(CriAtomExPlayer), "SetAisacControl", [typeof(uint), typeof(float)])]
public static void PreSetAisacControl(uint controlId, ref float value)
{
if (controlId == P1HeadphoneAisacControlId && p1HeadphoneVolumeOverride >= 0f)
{
value = Mathf.Clamp(p1HeadphoneVolumeOverride / 20f, 0f, 1f);
}
else if (controlId == P2HeadphoneAisacControlId && p2HeadphoneVolumeOverride >= 0f)
{
value = Mathf.Clamp(p2HeadphoneVolumeOverride / 20f, 0f, 1f);
}
}
}不过我是照着已有的HeadphoneVolumeMultiply写的。已有的代码也没有这么做 |
Summary by Sourcery
新功能:
HeadphoneVolumeOverride配置部分,为每位玩家(1P 和 2P)提供固定耳机音量选项,包括可禁用音量覆盖并尊重用户自身设置的能力。Original summary in English
Summary by Sourcery
New Features: