feat: 新增 C2S/UGC/SUS 谱面格式支持 (chu)#2
Merged
Conversation
Parser-Converter-Generator 三层架构，统一 IChuChart 接口，Generator 内嵌转换逻辑。 5 种语言 i18n 覆盖，4 项 xUnit 测试，219/219 全过。
审阅者指南新增统一的 CHUNITHM 谱面抽象层（IChuChart + ChuNote + 三种 IR），并为 C2S、UGC 和 SUS 格式实现完整的解析/生成与交叉转换支持，包括本地化警告和针对官方及自定义谱面的回归测试。 CHUNITHM 跨格式转换顺序图（UGC 文本到 C2S 文本）sequenceDiagram
participant User
participant UgcTextSource
participant UgcParser
participant UgcChart as UgcChart_IChuChart_
participant C2sGenerator
participant C2sTextSink
User->>UgcTextSource: Load UGC text
UgcTextSource-->>User: string ugcText
User->>UgcParser: Parse(ugcText)
UgcParser->>UgcParser: ParseHeaderLine / ParseNoteLine
UgcParser-->>User: UgcChart_IChuChart_, List~Alert~ alertsParse
User->>C2sGenerator: Generate(UgcChart_IChuChart_)
C2sGenerator->>C2sGenerator: ConvertToC2s(IChuChart, alertsGen)
alt input is UgcChart
C2sGenerator->>C2sGenerator: ScaleDown timing and notes
C2sGenerator->>C2sGenerator: Build C2sChart
else unsupported chart type
C2sGenerator->>C2sGenerator: Create Alert Warning using Locale.ChuGeneratorUnsupported
end
C2sGenerator->>C2sGenerator: Serialize(C2sChart)
C2sGenerator-->>User: string c2sText, List~Alert~ alertsGen
User->>C2sTextSink: Save c2sText
User->>User: Display or log alertsParse + alertsGen
统一 CHUNITHM 谱面模型类图（IChuChart、ChuNote、IR）classDiagram
direction LR
class IBaseChart {
<<interface>>
}
class BaseChart_ChuNote_ {
<<abstract>>
+List~ChuNote~ Notes
+decimal StartBpm
+decimal StartTime
+decimal EndTime
+int TotalNotes
}
IBaseChart <|.. IChuChart
class IChuChart {
<<interface>>
}
BaseChart_ChuNote_ <|-- C2sChart
BaseChart_ChuNote_ <|-- UgcChart
BaseChart_ChuNote_ <|-- SusChart
class ChuNote {
+string Type
+int Measure
+int Offset
+int Cell
+int Width
+int HoldDuration
+int SlideDuration
+int EndCell
+int EndWidth
+string Extra
+string TargetNote
+int AirHoldDuration
+int StartHeight
+int TargetHeight
+string NoteColor
}
class C2sChart {
+string Version
+int MusicId
+int DifficultId
+string Creator
+int Resolution
+double DefBpm
+List~(int Measure, int Offset, double Bpm)~ BpmEvents
+List~(int Measure, int Offset, int Denom, int Num)~ MetEvents
+List~(int Measure, int Offset, int Duration, double Multiplier)~ SflEvents
+decimal StartBpm
+decimal StartTime
+decimal EndTime
+int TotalNotes
}
class UgcChart {
+string Version
+string Title
+string Artist
+string Designer
+string Difficulty
+int Level
+double Constant
+string SongId
+int TicksPerBeat
+List~(int Measure, int Num, int Den)~ BeatEvents
+List~(int Measure, int Offset, double Bpm)~ BpmEvents
+List~(int Measure, int Offset, double Multiplier)~ SpeedEvents
+decimal StartBpm
+decimal StartTime
+decimal EndTime
+int TotalNotes
}
class SusChart {
+string Title
+string Artist
+string Designer
+int TicksPerBeat
+double Bpm
+decimal StartBpm
+decimal StartTime
+decimal EndTime
+int TotalNotes
}
IChuChart <|.. C2sChart
IChuChart <|.. UgcChart
IChuChart <|.. SusChart
C2sChart o-- ChuNote
UgcChart o-- ChuNote
SusChart o-- ChuNote
CHUNITHM 解析器、生成器、警告与多语言支持（i18n）类图classDiagram
direction LR
class IParser_T_ {
<<interface>>
+Parse(string text) (T, List~Alert~)
}
class IGenerator_IChuChart_ {
<<interface>>
+Generate(IChuChart chart) (string, List~Alert~)
}
class Alert {
<<record>>
+LEVEL Level
+string Message
+int Line
+string RelevantNote
}
class Alert_LEVEL_ {
<<enumeration>>
Info
Warning
Error
}
class Locale {
+static string C2SUnknownNoteType
+static string ChuGeneratorUnsupported
}
IParser_T_ <|.. C2sParser
IParser_T_ <|.. UgcParser
IParser_T_ <|.. SusParser
class C2sParser {
-static HashSet~string~ HeadTags
-static HashSet~string~ TimingTags
+Parse(string text) (C2sChart, List~Alert~)
-ParseHeader(string[] p, C2sChart chart) void
-ParseTiming(string[] p, C2sChart chart) void
-ParseNote(string[] p, C2sChart chart, List~Alert~ alerts, int lineNum) void
}
class UgcParser {
-static Dictionary~string, string~ AirDirections
-static Dictionary~string, string~ ChrExtras
+Parse(string text) (UgcChart, List~Alert~)
-ParseHeaderLine(string line, UgcChart chart, List~Alert~ alerts, int lineNum) void
-ParseNoteLine(string[] lines, int idx, UgcChart chart, List~Alert~ alerts) int
}
class SusParser {
-static Dictionary~int, string~ TypeMap
+Parse(string text) (SusChart, List~Alert~)
-ParseHeaderLine(string content, SusChart chart, List~Alert~ alerts, int lineNum) void
-ParseNoteLine(string content, SusChart chart, List~Alert~ alerts, int lineNum) void
}
IGenerator_IChuChart_ <|.. C2sGenerator
IGenerator_IChuChart_ <|.. UgcGenerator
IGenerator_IChuChart_ <|.. SusGenerator
class C2sGenerator {
-const int C2sResolution
+Generate(IChuChart chart) (string, List~Alert~)
-ConvertToC2s(IChuChart chart, List~Alert~ alerts) C2sChart
-ScaleNote(ChuNote n, int tpb) ChuNote
-Serialize(C2sChart chart) string
}
class UgcGenerator {
-const int UgcTicksPerBeat
-const int C2sResolution
+Generate(IChuChart chart) (string, List~Alert~)
-ConvertToUgc(IChuChart chart, List~Alert~ alerts) UgcChart
-ScaleUpNote(ChuNote n) ChuNote
-Serialize(UgcChart ugc) string
}
class SusGenerator {
-const int SusTpb
-const int C2sRsl
+Generate(IChuChart chart) (string, List~Alert~)
-ConvertToSus(IChuChart chart, List~Alert~ alerts) SusChart
-ScaleUp(ChuNote n) ChuNote
-Serialize(SusChart sus) string
}
Alert o-- Alert_LEVEL_
C2sParser --> C2sChart
UgcParser --> UgcChart
SusParser --> SusChart
C2sParser --> Alert
UgcParser --> Alert
SusParser --> Alert
C2sGenerator --> IChuChart
UgcGenerator --> IChuChart
SusGenerator --> IChuChart
C2sGenerator --> C2sChart
UgcGenerator --> UgcChart
SusGenerator --> SusChart
C2sGenerator --> ChuNote
UgcGenerator --> ChuNote
SusGenerator --> ChuNote
C2sParser ..> Locale
C2sGenerator ..> Locale
UgcGenerator ..> Locale
SusGenerator ..> Locale
CHUNITHM 谱面解析、IR 与跨格式生成流程图flowchart LR
subgraph InputFormats[输入文本格式]
C2S_TXT[C2S text]
UGC_TXT[UGC text]
SUS_TXT[SUS text]
end
subgraph Parsers[解析器]
C2S_PARSER[C2sParser\nIParser<C2sChart>]
UGC_PARSER[UgcParser\nIParser<UgcChart>]
SUS_PARSER[SusParser\nIParser<SusChart>]
end
subgraph IR[统一 CHUNITHM IR]
C2S_IR[C2sChart\nimplements IChuChart]
UGC_IR[UgcChart\nimplements IChuChart]
SUS_IR[SusChart\nimplements IChuChart]
NOTES[ChuNote objects]
end
subgraph Generators[生成器]
C2S_GEN[C2sGenerator\nIGenerator<IChuChart>]
UGC_GEN[UgcGenerator\nIGenerator<IChuChart>]
SUS_GEN[SusGenerator\nIGenerator<IChuChart>]
end
subgraph OutputFormats[输出文本格式]
C2S_OUT[C2S text]
UGC_OUT[UGC text]
SUS_OUT[SUS text]
end
C2S_TXT --> C2S_PARSER
UGC_TXT --> UGC_PARSER
SUS_TXT --> SUS_PARSER
C2S_PARSER --> C2S_IR
UGC_PARSER --> UGC_IR
SUS_PARSER --> SUS_IR
C2S_IR --> NOTES
UGC_IR --> NOTES
SUS_IR --> NOTES
C2S_IR --> C2S_GEN
C2S_IR --> UGC_GEN
C2S_IR --> SUS_GEN
UGC_IR --> C2S_GEN
UGC_IR --> UGC_GEN
UGC_IR --> SUS_GEN
SUS_IR --> C2S_GEN
SUS_IR --> UGC_GEN
SUS_IR --> SUS_GEN
C2S_GEN --> C2S_OUT
UGC_GEN --> UGC_OUT
SUS_GEN --> SUS_OUT
classDef ir fill:#eef,stroke:#446
class C2S_IR,UGC_IR,SUS_IR,NOTES ir
classDef parser fill:#efe,stroke:#484
class C2S_PARSER,UGC_PARSER,SUS_PARSER parser
classDef gen fill:#ffe,stroke:#884
class C2S_GEN,UGC_GEN,SUS_GEN gen
文件级变更
提示与命令与 Sourcery 交互
自定义你的体验访问你的 控制面板 以：
获取帮助Original review guide in EnglishReviewer's GuideAdds a unified CHUNITHM chart abstraction (IChuChart + ChuNote + three IRs) and implements full parse/generate + cross-conversion support for C2S, UGC, and SUS formats, including localized warnings and regression tests over official and custom charts. Sequence diagram for cross-format CHUNITHM conversion (UGC text to C2S text)sequenceDiagram
participant User
participant UgcTextSource
participant UgcParser
participant UgcChart as UgcChart_IChuChart_
participant C2sGenerator
participant C2sTextSink
User->>UgcTextSource: Load UGC text
UgcTextSource-->>User: string ugcText
User->>UgcParser: Parse(ugcText)
UgcParser->>UgcParser: ParseHeaderLine / ParseNoteLine
UgcParser-->>User: UgcChart_IChuChart_, List~Alert~ alertsParse
User->>C2sGenerator: Generate(UgcChart_IChuChart_)
C2sGenerator->>C2sGenerator: ConvertToC2s(IChuChart, alertsGen)
alt input is UgcChart
C2sGenerator->>C2sGenerator: ScaleDown timing and notes
C2sGenerator->>C2sGenerator: Build C2sChart
else unsupported chart type
C2sGenerator->>C2sGenerator: Create Alert Warning using Locale.ChuGeneratorUnsupported
end
C2sGenerator->>C2sGenerator: Serialize(C2sChart)
C2sGenerator-->>User: string c2sText, List~Alert~ alertsGen
User->>C2sTextSink: Save c2sText
User->>User: Display or log alertsParse + alertsGen
Class diagram for unified CHUNITHM chart model (IChuChart, ChuNote, IRs)classDiagram
direction LR
class IBaseChart {
<<interface>>
}
class BaseChart_ChuNote_ {
<<abstract>>
+List~ChuNote~ Notes
+decimal StartBpm
+decimal StartTime
+decimal EndTime
+int TotalNotes
}
IBaseChart <|.. IChuChart
class IChuChart {
<<interface>>
}
BaseChart_ChuNote_ <|-- C2sChart
BaseChart_ChuNote_ <|-- UgcChart
BaseChart_ChuNote_ <|-- SusChart
class ChuNote {
+string Type
+int Measure
+int Offset
+int Cell
+int Width
+int HoldDuration
+int SlideDuration
+int EndCell
+int EndWidth
+string Extra
+string TargetNote
+int AirHoldDuration
+int StartHeight
+int TargetHeight
+string NoteColor
}
class C2sChart {
+string Version
+int MusicId
+int DifficultId
+string Creator
+int Resolution
+double DefBpm
+List~(int Measure, int Offset, double Bpm)~ BpmEvents
+List~(int Measure, int Offset, int Denom, int Num)~ MetEvents
+List~(int Measure, int Offset, int Duration, double Multiplier)~ SflEvents
+decimal StartBpm
+decimal StartTime
+decimal EndTime
+int TotalNotes
}
class UgcChart {
+string Version
+string Title
+string Artist
+string Designer
+string Difficulty
+int Level
+double Constant
+string SongId
+int TicksPerBeat
+List~(int Measure, int Num, int Den)~ BeatEvents
+List~(int Measure, int Offset, double Bpm)~ BpmEvents
+List~(int Measure, int Offset, double Multiplier)~ SpeedEvents
+decimal StartBpm
+decimal StartTime
+decimal EndTime
+int TotalNotes
}
class SusChart {
+string Title
+string Artist
+string Designer
+int TicksPerBeat
+double Bpm
+decimal StartBpm
+decimal StartTime
+decimal EndTime
+int TotalNotes
}
IChuChart <|.. C2sChart
IChuChart <|.. UgcChart
IChuChart <|.. SusChart
C2sChart o-- ChuNote
UgcChart o-- ChuNote
SusChart o-- ChuNote
Class diagram for CHUNITHM parsers, generators, alerts, and i18nclassDiagram
direction LR
class IParser_T_ {
<<interface>>
+Parse(string text) (T, List~Alert~)
}
class IGenerator_IChuChart_ {
<<interface>>
+Generate(IChuChart chart) (string, List~Alert~)
}
class Alert {
<<record>>
+LEVEL Level
+string Message
+int Line
+string RelevantNote
}
class Alert_LEVEL_ {
<<enumeration>>
Info
Warning
Error
}
class Locale {
+static string C2SUnknownNoteType
+static string ChuGeneratorUnsupported
}
IParser_T_ <|.. C2sParser
IParser_T_ <|.. UgcParser
IParser_T_ <|.. SusParser
class C2sParser {
-static HashSet~string~ HeadTags
-static HashSet~string~ TimingTags
+Parse(string text) (C2sChart, List~Alert~)
-ParseHeader(string[] p, C2sChart chart) void
-ParseTiming(string[] p, C2sChart chart) void
-ParseNote(string[] p, C2sChart chart, List~Alert~ alerts, int lineNum) void
}
class UgcParser {
-static Dictionary~string, string~ AirDirections
-static Dictionary~string, string~ ChrExtras
+Parse(string text) (UgcChart, List~Alert~)
-ParseHeaderLine(string line, UgcChart chart, List~Alert~ alerts, int lineNum) void
-ParseNoteLine(string[] lines, int idx, UgcChart chart, List~Alert~ alerts) int
}
class SusParser {
-static Dictionary~int, string~ TypeMap
+Parse(string text) (SusChart, List~Alert~)
-ParseHeaderLine(string content, SusChart chart, List~Alert~ alerts, int lineNum) void
-ParseNoteLine(string content, SusChart chart, List~Alert~ alerts, int lineNum) void
}
IGenerator_IChuChart_ <|.. C2sGenerator
IGenerator_IChuChart_ <|.. UgcGenerator
IGenerator_IChuChart_ <|.. SusGenerator
class C2sGenerator {
-const int C2sResolution
+Generate(IChuChart chart) (string, List~Alert~)
-ConvertToC2s(IChuChart chart, List~Alert~ alerts) C2sChart
-ScaleNote(ChuNote n, int tpb) ChuNote
-Serialize(C2sChart chart) string
}
class UgcGenerator {
-const int UgcTicksPerBeat
-const int C2sResolution
+Generate(IChuChart chart) (string, List~Alert~)
-ConvertToUgc(IChuChart chart, List~Alert~ alerts) UgcChart
-ScaleUpNote(ChuNote n) ChuNote
-Serialize(UgcChart ugc) string
}
class SusGenerator {
-const int SusTpb
-const int C2sRsl
+Generate(IChuChart chart) (string, List~Alert~)
-ConvertToSus(IChuChart chart, List~Alert~ alerts) SusChart
-ScaleUp(ChuNote n) ChuNote
-Serialize(SusChart sus) string
}
Alert o-- Alert_LEVEL_
C2sParser --> C2sChart
UgcParser --> UgcChart
SusParser --> SusChart
C2sParser --> Alert
UgcParser --> Alert
SusParser --> Alert
C2sGenerator --> IChuChart
UgcGenerator --> IChuChart
SusGenerator --> IChuChart
C2sGenerator --> C2sChart
UgcGenerator --> UgcChart
SusGenerator --> SusChart
C2sGenerator --> ChuNote
UgcGenerator --> ChuNote
SusGenerator --> ChuNote
C2sParser ..> Locale
C2sGenerator ..> Locale
UgcGenerator ..> Locale
SusGenerator ..> Locale
Flow diagram for CHUNITHM chart parsing, IR, and cross-format generationflowchart LR
subgraph InputFormats[Input text formats]
C2S_TXT[C2S text]
UGC_TXT[UGC text]
SUS_TXT[SUS text]
end
subgraph Parsers[Parsers]
C2S_PARSER[C2sParser\nIParser<C2sChart>]
UGC_PARSER[UgcParser\nIParser<UgcChart>]
SUS_PARSER[SusParser\nIParser<SusChart>]
end
subgraph IR[Unified CHUNITHM IR]
C2S_IR[C2sChart\nimplements IChuChart]
UGC_IR[UgcChart\nimplements IChuChart]
SUS_IR[SusChart\nimplements IChuChart]
NOTES[ChuNote objects]
end
subgraph Generators[Generators]
C2S_GEN[C2sGenerator\nIGenerator<IChuChart>]
UGC_GEN[UgcGenerator\nIGenerator<IChuChart>]
SUS_GEN[SusGenerator\nIGenerator<IChuChart>]
end
subgraph OutputFormats[Output text formats]
C2S_OUT[C2S text]
UGC_OUT[UGC text]
SUS_OUT[SUS text]
end
C2S_TXT --> C2S_PARSER
UGC_TXT --> UGC_PARSER
SUS_TXT --> SUS_PARSER
C2S_PARSER --> C2S_IR
UGC_PARSER --> UGC_IR
SUS_PARSER --> SUS_IR
C2S_IR --> NOTES
UGC_IR --> NOTES
SUS_IR --> NOTES
C2S_IR --> C2S_GEN
C2S_IR --> UGC_GEN
C2S_IR --> SUS_GEN
UGC_IR --> C2S_GEN
UGC_IR --> UGC_GEN
UGC_IR --> SUS_GEN
SUS_IR --> C2S_GEN
SUS_IR --> UGC_GEN
SUS_IR --> SUS_GEN
C2S_GEN --> C2S_OUT
UGC_GEN --> UGC_OUT
SUS_GEN --> SUS_OUT
classDef ir fill:#eef,stroke:#446
class C2S_IR,UGC_IR,SUS_IR,NOTES ir
classDef parser fill:#efe,stroke:#484
class C2S_PARSER,UGC_PARSER,SUS_PARSER parser
classDef gen fill:#ffe,stroke:#884
class C2S_GEN,UGC_GEN,SUS_GEN gen
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 11 个问题，并给出了一些整体性反馈：
- UGC/SUS 路径中的数字解析/格式化目前依赖当前区域性（例如
UgcParser.ParseHeaderLine中的double.TryParse，以及{b.Bpm:F5}/{sus.Bpm:F2}这类字符串插值），在非en-US区域设置下可能会出错；建议像C2sGenerator中那样统一使用CultureInfo.InvariantCulture。 - 像
UgcGenerator.ScaleUpNote（s(int v) => v * UgcTicksPerBeat / (C2sResolution / 4)）和SusGenerator.ScaleUp这样的 tick 缩放辅助方法使用int做算术运算，在大数值情况下可能会溢出；建议对齐C2sGenerator.ScaleNote的做法，在乘法前先提升为long，这样既更安全也更一致。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- UGC/SUS 路径中的数字解析/格式化目前依赖当前区域性（例如 `UgcParser.ParseHeaderLine` 中的 `double.TryParse`，以及 `{b.Bpm:F5}` / `{sus.Bpm:F2}` 这类字符串插值），在非 `en-US` 区域设置下可能会出错；建议像 `C2sGenerator` 中那样统一使用 `CultureInfo.InvariantCulture`。
- 像 `UgcGenerator.ScaleUpNote`（`s(int v) => v * UgcTicksPerBeat / (C2sResolution / 4)`）和 `SusGenerator.ScaleUp` 这样的 tick 缩放辅助方法使用 `int` 做算术运算，在大数值情况下可能会溢出；建议对齐 `C2sGenerator.ScaleNote` 的做法，在乘法前先提升为 `long`，这样既更安全也更一致。
## Individual Comments
### Comment 1
<location path="parser/chu/UgcParser.cs" line_range="136-78" />
<code_context>
+ alerts.Add(new Alert(Warning, $"@TICKS 格式错误: {line}") { Line = lineNum });
+ break;
+
+ case "@BEAT":
+ var beatParts = value.Split(' ');
+ if (beatParts.Length >= 3
+ && int.TryParse(beatParts[0], out var beatMeasure)
+ && int.TryParse(beatParts[1], out var beatNum)
+ && int.TryParse(beatParts[2], out var beatDen))
+ {
+ chart.BeatEvents.Add((beatMeasure, beatNum, beatDen));
+ }
+ else
+ {
+ alerts.Add(new Alert(Warning, $"@BEAT 格式错误: {line}") { Line = lineNum });
+ }
+ break;
+
+ case "@BPM":
</code_context>
<issue_to_address>
**issue (bug_risk):** @BEAT 解析假定使用空格分隔，但生成器使用的是制表符分隔，导致所有 BEAT 行都会被视为格式错误。
在 `ParseHeaderLine` 中，`value` 是第一个制表符之后的子串，里面可能仍然包含制表符，但这里使用空格拆分：`value.Split(' ')`。`UgcGenerator.Serialize` 会将 `@BEAT` 输出为 `"@BEAT\t{b.Measure}\t{b.Num}\t{b.Den}"`，因此其中没有空格，`beatParts.Length` 始终为 1，所有 `@BEAT` 行都会被当作格式错误。为了与序列化逻辑保持一致，应改为按制表符或更通用的空白字符拆分，例如 `value.Split('\t', ' ')` 或 `value.Split((char[])null, StringSplitOptions.RemoveEmptyEntries)`。
</issue_to_address>
### Comment 2
<location path="parser/chu/UgcParser.cs" line_range="151-78" />
<code_context>
+ case "@BPM":
</code_context>
<issue_to_address>
**issue (bug_risk):** @BPM 的解析方式与实际使用制表符而非空格的序列化格式不兼容。
对于 `@BPM`，`value` 是第一个制表符之后的子串，因此对于类似 `@BPM 0'0 120.00000` 的生成行来说，`value` 为 `"0'0 120.00000"`。解析器随后调用 `IndexOf(' ')`，但因为分隔符是制表符，该调用返回 `-1`，导致该行被视为格式错误。为了与 `UgcGenerator` 保持兼容，这里的逻辑应改为按制表符（或更通用的空白字符）拆分，而不是假定在 `measure'offset` 与 BPM 之间存在空格。
</issue_to_address>
### Comment 3
<location path="generator/chu/SusGenerator.cs" line_range="76-77" />
<code_context>
+ sb.AppendLine($"#REQUEST \"{sus.TicksPerBeat}\"");
+ sb.AppendLine();
+
+ foreach (var n in sus.Notes.OrderBy(n => n.Measure).ThenBy(n => n.Offset))
+ sb.AppendLine($"#{n.Measure:X2}{n.Offset:X3}:{FormatData(n)}");
+
+ return sb.ToString();
</code_context>
<issue_to_address>
**issue (bug_risk):** SUS 生成器使用 3 位十六进制 tick 字段，而解析器只读取 2 位，导致往返转换失败。
生成器会输出一个 5 位的时间字段 `"{measure:X2}{offset:X3}"`，但 `SusParser.ParseNoteLine` 期待的是 `#MMTT:data`，且仅读取 2 位 tick（`[..2]` 为小节，`[2..4]` 为 tick）。多出的那一位 tick 会被静默丢弃，从而截断大于 0xFF 的 offset，并在往返转换时丢失精度。请考虑要么将生成器改为只使用 2 位 tick（并相应调整时间分辨率），要么更新解析器以接受 3 位 tick 格式（例如 `[..2]` 为小节，其余部分为 tick），以保证格式一致并保留时间精度。
</issue_to_address>
### Comment 4
<location path="parser/chu/SusParser.cs" line_range="42" />
<code_context>
+ private static void ParseAirTarget(string dataStr, SusChart chart, List<Alert> alerts, int lineNum)
</code_context>
<issue_to_address>
**issue (bug_risk):** SUS AIR/ADW 目标解析期望使用十六进制编码的整数，但生成器写入的是原始的 `TargetNote` 字符串。
`ParseAirTarget` 要求 `dataStr.Length >= 8`，并解码 `HexToInt(dataStr[6..8])`，但 `SusGenerator.FormatData` 对 AIR/ADW 的输出是 `"{tc}{lw}{n.TargetNote}"`，其中 `TargetNote` 可以是一个非十六进制字符（例如 "N"）。这会生成短于 8 个字符或包含非十六进制内容的字符串，从而导致警告或错误的目标解析。请考虑要么在生成器中将 `TargetNote` 编码为十六进制值以匹配当前解析逻辑，要么更新解析器，将结尾的子串视为原始的目标标记而不是十六进制数。
</issue_to_address>
### Comment 5
<location path="generator/chu/UgcGenerator.cs" line_range="88" />
<code_context>
+ sb.AppendLine($"@CONST\t{ugc.Constant:F5}");
+ sb.AppendLine($"@SONGID\t{ugc.SongId}");
+ sb.AppendLine($"@TICKS\t{ugc.TicksPerBeat}");
+ foreach (var b in ugc.BeatEvents) sb.AppendLine($"@BEAT\t{b.Measure}\t{b.Num}\t{b.Den}");
+ foreach (var b in ugc.BpmEvents) sb.AppendLine($"@BPM\t{b.Measure}'{b.Offset}\t{b.Bpm:F5}");
+ sb.AppendLine("@TIL\t0\t0'0\t1.00000");
</code_context>
<issue_to_address>
**suggestion (bug_risk):** UgcGenerator 的 @BEAT 序列化使用制表符，与当前 UgcParser 期望的空格分隔值不一致。
结合 `UgcParser.ParseHeaderLine`，意味着 `@BEAT` 被写成制表符分隔，而解析时却按空格分隔（`value.Split(' ')`）。即便你在解析器中做了修正，仍然建议选择一种统一的分隔符（制表符或空格），并在序列化和解析两侧进行文档说明，以避免未来再出现不一致，尤其是对手工编辑谱面的情况。
建议实现如下：
```csharp
sb.AppendLine($"@TICKS\t{ugc.TicksPerBeat}");
// NOTE: @BEAT fields are space-separated; keep this delimiter in sync with UgcParser.ParseHeaderLine
foreach (var b in ugc.BeatEvents) sb.AppendLine($"@BEAT {b.Measure} {b.Num} {b.Den}");
foreach (var b in ugc.BpmEvents) sb.AppendLine($"@BPM\t{b.Measure}'{b.Offset}\t{b.Bpm:F5}");
```
要完全落实该建议，UgcParser.ParseHeaderLine（或专门的 `@BEAT` 解析分支）也应更新为：
1. 使用与序列化端相同的分隔符（空格），例如对 `@BEAT` 载荷使用 `line.Split(' ', StringSplitOptions.RemoveEmptyEntries)`；
2. 在 UgcParser 中添加相应注释，说明 `@BEAT` 字段使用空格分隔，且必须与 UgcGenerator 的序列化逻辑保持同步。
</issue_to_address>
### Comment 6
<location path="generator/chu/C2sGenerator.cs" line_range="102-103" />
<code_context>
+ sb.AppendLine($"SFL\t{s.Measure}\t{s.Offset}\t{s.Duration}\t{Mlt(s.Multiplier)}");
+ sb.AppendLine();
+
+ foreach (var n in chart.Notes.OrderBy(n => n.Measure * C2sResolution + n.Offset))
+ sb.AppendLine(FormatNote(n));
+
+ sb.AppendLine();
</code_context>
<issue_to_address>
**issue (bug_risk):** 从 C2S 解析出的 ALD/ASD 音符类型在生成时没有区分处理，而是回退成 TAP。
`C2sParser.ParseNote` 将 `ALD` 和 `ASD` 视为具有额外属性的独立音符类型，但 `C2sGenerator.FormatNote` 中缺少对应的分支处理，最终会回退到默认的 TAP 序列化。这会在导出时静默降级这些音符。请考虑在 `FormatNote` 中为 `ALD`/`ASD` 添加显式处理，或者在碰到它们时记录/输出警告，让用户意识到细节被丢失了。
</issue_to_address>
### Comment 7
<location path="tests/chu/ChuTests.cs" line_range="7" />
<code_context>
+
+namespace MuConvert.Tests.chu;
+
+public class ChuTests
+{
+ private static string TestsetDir => Path.Combine(AppContext.BaseDirectory, "..", "..", "..", "chu", "testset");
</code_context>
<issue_to_address>
**suggestion (testing):** 增加涵盖 SUS 解析器/生成器以及跨格式转换的测试
目前仅覆盖了 C2S 和 UGC 路径。请另外添加测试：(1) 解析一个 SUS 测试谱并验证关键字段/音符；(2) 通过 SusGenerator/SusParser 对 SUS 做往返测试（类似 C2sRoundTrip）；以及 (3) 覆盖 C2S → SUS 和 UGC → SUS 的转换，以验证缩放和类型映射。这将更好地验证完整的 C2S/UGC/SUS 转换矩阵。
建议实现如下：
```csharp
private static string TestsetDir => Path.Combine(AppContext.BaseDirectory, "..", "..", "..", "chu", "testset");
private static string OfficialDir => Path.Combine(TestsetDir, "官谱", "B.B.K.K.B.K.K");
private static string CustomDir => Path.Combine(TestsetDir, "自制谱", "Example");
private static string C2sPath => Path.Combine(OfficialDir, "0003_00.c2s");
private static string UgcPath => Path.Combine(CustomDir, "basic.ugc");
private static string SusPath => Path.Combine(OfficialDir, "0003_00.sus");
[Fact]
public void CanParseOfficialSus()
{
if (!File.Exists(SusPath)) throw new SkipException($"Missing: {SusPath}");
var susText = File.ReadAllText(SusPath);
var (chart, _) = new SusParser().Parse(susText);
Assert.NotNull(chart);
}
[Fact]
public void SusRoundTrip()
{
if (!File.Exists(SusPath)) throw new SkipException($"Missing: {SusPath}");
var originalSus = File.ReadAllText(SusPath);
var (chart, _) = new SusParser().Parse(originalSus);
var generatedSus = new SusGenerator().Generate(chart);
Assert.False(string.IsNullOrWhiteSpace(generatedSus));
var (roundTrippedChart, _) = new SusParser().Parse(generatedSus);
Assert.NotNull(roundTrippedChart);
}
[Fact]
public void CanConvertC2sToSus()
{
if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
var (chart, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
var susText = new SusGenerator().Generate(chart);
Assert.False(string.IsNullOrWhiteSpace(susText));
var (susChart, _) = new SusParser().Parse(susText);
Assert.NotNull(susChart);
}
[Fact]
public void CanConvertUgcToSus()
{
if (!File.Exists(UgcPath)) throw new SkipException($"Missing: {UgcPath}");
var (chart, _) = new UgcParser().Parse(File.ReadAllText(UgcPath));
var susText = new SusGenerator().Generate(chart);
Assert.False(string.IsNullOrWhiteSpace(susText));
var (susChart, _) = new SusParser().Parse(susText);
Assert.NotNull(susChart);
}
[Fact]
public void CanParseOfficialC2S()
```
这些修改假定：
1. `SusParser` 和 `SusGenerator` 已存在于 `MuConvert.parser` / `MuConvert.chu` 命名空间中，并提供与 `C2sParser`/`UgcParser` 相同元组返回形态的 `Parse(string)` 与 `Generate(chart)` 方法；
2. 在 `官谱/B.B.K.K.B.K.K/0003_00.sus` 位置存在一份官方 SUS 文件。如果你的测试数据使用了不同的文件名或目录结构，请相应调整 `SusPath`；
3. 如果谱面类型暴露了更丰富的属性（例如音符数量、时间、轨道数据等），并且已经有用于谱面深度比较的辅助方法，可以扩展这些测试中的断言，用来比较这些字段，从而更加直接地验证跨格式的缩放和类型映射。
</issue_to_address>
### Comment 8
<location path="tests/chu/ChuTests.cs" line_range="25-31" />
<code_context>
+ }
+
+ [Fact]
+ public void C2sRoundTrip()
+ {
+ if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
+ var (chart, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
+ var (rt, _) = new C2sGenerator().Generate(chart);
+ var (reparsed, _) = new C2sParser().Parse(rt);
+ Assert.Equal(chart.Notes.Count, reparsed.Notes.Count);
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** 将 C2S 往返测试的断言从仅比较音符数量加强为更严格的验证
目前仅比较解析 → 生成 → 再解析后音符总数是否相同。如果音符被重新排序、时间发生变化或者部分字段丢失，但数量不变，这个测试仍然可能通过。请加强断言，例如对关键字段（小节、偏移、类型、格子、宽度、时长/结束位置等）的有序投影进行比较，或者至少确保具有代表性的音符类型（如滑条、长押、AIR）在往返后保持不变。这样能更好地验证解析器/生成器对数据的无损性。
建议实现如下：
```csharp
[Fact]
public void C2sRoundTrip()
{
if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
var (chart, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
var (rt, _) = new C2sGenerator().Generate(chart);
var (reparsed, _) = new C2sParser().Parse(rt);
// Basic sanity: note count must be preserved
Assert.Equal(chart.Notes.Count, reparsed.Notes.Count);
// Stronger guarantee: the set of notes (by their public properties) must be preserved
var originalSnapshots = chart.Notes
.Select(SnapshotNote)
.OrderBy(s => s)
.ToArray();
var reparsedSnapshots = reparsed.Notes
.Select(SnapshotNote)
.OrderBy(s => s)
.ToArray();
Assert.Equal(originalSnapshots, reparsedSnapshots);
}
```
```csharp
private static string C2sPath => Path.Combine(OfficialDir, "0003_00.c2s");
private static string UgcPath => Path.Combine(CustomDir, "basic.ugc");
/// <summary>
/// Creates a stable, comparable string snapshot of a note by concatenating all
/// of its public instance properties in name-sorted order. This lets the
/// round-trip test verify that no note data was lost or reordered without
/// needing to know the concrete note type shape.
/// </summary>
private static string SnapshotNote(object note)
{
if (note is null) return string.Empty;
var type = note.GetType();
var props = type.GetProperties(System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.Public);
var parts = props
.OrderBy(p => p.Name)
.Select(p => $"{p.Name}={p.GetValue(note)}");
return string.Join("|", parts);
}
```
该修改假定 `ChuTests.cs` 中已经通过 `using System.Linq;` 引入了 `Select`、`OrderBy` 和 `ToArray`。如果没有，请在文件顶部的其他 `using` 之后添加 `using System.Linq;`。
</issue_to_address>
### Comment 9
<location path="tests/chu/ChuTests.cs" line_range="16-21" />
<code_context>
+ private static string UgcPath => Path.Combine(CustomDir, "basic.ugc");
+
+ [Fact]
+ public void CanParseOfficialC2S()
+ {
+ if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
+ var (chart, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
+ Assert.NotEmpty(chart.Notes);
+ Assert.Equal(384, chart.Resolution);
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** 考虑为 C2S/UGC 添加负例/边界情况解析测试，以验证告警行为
这些测试目前只覆盖了正常路径的谱面。由于解析器会针对错误输入（例如未知音符类型、错误头部、缺少跟随行等）生成 `Alert` 对象，请增加一些使用小型合成输入的单元测试，命中这些错误路径，并断言解析可以成功完成，同时产生预期的告警（级别、消息和行号信息）。这有助于捕捉错误处理和本地化（例如 `Locale.C2SUnknownNoteType`）方面的回归问题。
建议实现如下：
```csharp
private static string UgcPath => Path.Combine(CustomDir, "basic.ugc");
[Fact]
public void CanParseOfficialC2S()
{
if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
var (chart, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
Assert.NotEmpty(chart.Notes);
Assert.Equal(384, chart.Resolution);
}
[Fact]
public void C2S_UnknownNoteType_ProducesAlert()
{
// Minimal C2S chart with an unknown note type to drive the "unknown note type" alert.
const string input =
"VERSION:1\n" +
"TITLE:Test\n" +
"ARTIST:Test\n" +
"BEAT:4/4\n" +
"BPM:120\n" +
"START:\n" +
"001,1,UNKNOWN,1,0\n";
var (chart, alerts) = new C2sParser().Parse(input);
Assert.NotNull(chart);
Assert.NotNull(alerts);
var alert = Assert.Single(alerts);
Assert.Equal(AlertLevel.Error, alert.Level);
Assert.Equal(Locale.C2SUnknownNoteType, alert.Message);
Assert.True(alert.Line > 0);
}
[Fact]
public void C2S_MissingFollowerLine_ProducesAlert()
{
// Start of a flick/slide without the expected follower line.
const string input =
"VERSION:1\n" +
"TITLE:Test\n" +
"ARTIST:Test\n" +
"BEAT:4/4\n" +
"BPM:120\n" +
"START:\n" +
"001,1,START_FLICK,1,0\n";
var (chart, alerts) = new C2sParser().Parse(input);
Assert.NotNull(chart);
Assert.NotNull(alerts);
Assert.NotEmpty(chart.Notes);
var alert = Assert.Single(alerts);
Assert.Equal(AlertLevel.Warning, alert.Level);
Assert.Equal(Locale.C2SMissingFollowerNote, alert.Message);
Assert.True(alert.Line > 0);
}
[Fact]
public void Ugc_UnknownNoteType_ProducesAlert()
{
// UGC chart with an unknown note type token.
const string input =
"#TITLE:Test\n" +
"#ARTIST:Test\n" +
"#BPM:120\n" +
"#DIFFICULTY:1\n" +
"001,1,UNKNOWN,1,0\n";
var (chart, alerts) = new UgcParser().Parse(input);
Assert.NotNull(chart);
Assert.NotNull(alerts);
var alert = Assert.Single(alerts);
Assert.Equal(AlertLevel.Error, alert.Level);
Assert.Equal(Locale.UGCUnknownNoteType, alert.Message);
Assert.True(alert.Line > 0);
}
[Fact]
public void Ugc_InvalidHeader_ProducesAlert()
{
// Header with missing title and invalid BPM to hit header-validation alerts.
const string input =
"#TITLE:\n" + // missing title value
"#ARTIST:Test\n" +
"#BPM:abc\n" + // invalid BPM
"#DIFFICULTY:1\n" +
"001,1,TAP,1,0\n";
var (chart, alerts) = new UgcParser().Parse(input);
Assert.NotNull(chart);
Assert.NotNull(alerts);
Assert.NotEmpty(chart.Notes);
Assert.NotEmpty(alerts);
Assert.Contains(alerts, a => a.Level == AlertLevel.Error && a.Message == Locale.UGCInvalidBpm);
Assert.All(alerts, a => Assert.True(a.Line > 0));
}
```
1. 确保文件中已经为解析器、告警类型和本地化引入了必要的 `using` 指令：
- `using MuConvert.Chu;`（或实际包含 `C2sParser`/`UgcParser` 的命名空间）；
- `using MuConvert.Chu.Localization;`（或 `Locale` 所在的命名空间）；
- `using MuConvert.Chu.Alerts;`（或 `Alert`/`AlertLevel` 所在的命名空间），如果它们尚未在作用域内。
2. 如果实际类型或属性名称与此不同，请相应调整：
- 若告警暴露的是 `Severity`、`MessageKey`、`LineNumber` 等属性，请将断言中的 `alert.Level`、`alert.Message`、`alert.Line` 改为对应的真实 API，并与正确的本地化键或值进行比较（例如 `Locale.C2SUnknownNoteType` 可能是一个键而非字符串）；
- 若解析器返回的不是 `(chart, alerts)` 元组，而是自定义结果类型，请调整解构和访问方式。
3. 必要时根据真实语法对上述 C2S/UGC 合成文本做适当修改（字段数、`START_FLICK` 等 token），以确保它们既能被正确解析，又能触发目标告警路径。
4. 如果 `ChuTests.cs` 中已经存在名为 `CanParseOfficialC2S` 的方法，请删除新增的同名方法，避免重复定义。
</issue_to_address>
### Comment 10
<location path="tests/chu/ChuTests.cs" line_range="43-52" />
<code_context>
+ }
+
+ [Fact]
+ public void UgcToC2sViaGenerator()
+ {
+ if (!File.Exists(UgcPath)) throw new SkipException($"Missing: {UgcPath}");
+ var (ugc, _) = new UgcParser().Parse(File.ReadAllText(UgcPath));
+ var (c2sText, _) = new C2sGenerator().Generate(ugc);
+ Assert.Contains("VERSION", c2sText);
+ Assert.Contains("TAP\t", c2sText);
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** 加强 UgcToC2sViaGenerator 的断言，使其验证内容不仅局限于关键字是否存在
当前断言只检查 `"VERSION"` 和 `"TAP\t"` 是否存在，因此即便大部分音符被丢弃或时间完全错误，该测试仍可能通过。请考虑增加往返流程：使用 `C2sParser` 解析生成的 C2S，并对结果谱面进行断言（例如：音符非空、存在预期的音符类型、大致的音符数量或某些已知音符属性），从而让这一跨格式转换测试更加健壮。
```suggestion
[Fact]
public void UgcToC2sViaGenerator()
{
if (!File.Exists(UgcPath)) throw new SkipException($"Missing: {UgcPath}");
// Parse UGC chart
var (ugcChart, _) = new UgcParser().Parse(File.ReadAllText(UgcPath));
Assert.NotEmpty(ugcChart.Notes);
// Generate C2S text from UGC chart
var (c2sText, _) = new C2sGenerator().Generate(ugcChart);
Assert.Contains("VERSION", c2sText);
Assert.Contains("TAP\t", c2sText);
// Parse back the generated C2S into a chart
var (c2sChart, _) = new C2sParser().Parse(c2sText);
// Ensure the generated chart is non-empty
Assert.NotEmpty(c2sChart.Notes);
// Ensure note count is approximately preserved across formats
var originalNoteCount = ugcChart.Notes.Count;
var generatedNoteCount = c2sChart.Notes.Count;
Assert.InRange(generatedNoteCount, (int)(originalNoteCount * 0.8), (int)(originalNoteCount * 1.2));
}
```
</issue_to_address>
### Comment 11
<location path="tests/chu/ChuTests.cs" line_range="54-63" />
<code_context>
+ }
+
+ [Fact]
+ public void DumpOutputFiles()
+ {
+ if (!File.Exists(UgcPath)) throw new SkipException($"Missing: {UgcPath}");
+ var (ugc, _) = new UgcParser().Parse(File.ReadAllText(UgcPath));
+ var (c2sText, _) = new C2sGenerator().Generate(ugc);
+ File.WriteAllText(Path.Combine(OfficialDir, "basic_output.c2s"), c2sText);
+
+ if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
+ var (c2s, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
+ var (ugcText, _) = new UgcGenerator().Generate(c2s);
+ File.WriteAllText(Path.Combine(OfficialDir, "0003_output.ugc"), ugcText);
+
+ Assert.True(File.Exists(Path.Combine(OfficialDir, "basic_output.c2s")));
+ Assert.True(File.Exists(Path.Combine(OfficialDir, "0003_output.ugc")));
+ }
+}
</code_context>
<issue_to_address>
**suggestion (testing):** 避免在 DumpOutputFiles 中产生文件系统副作用，或明确将其标记为非单元/集成测试
该测试会向仓库中的 `OfficialDir` 写入文件并据此断言，这在并行执行、只读 CI 环境或导致工作区脏状态时都可能产生不稳定因素。请考虑将其改为使用临时目录的集成/基线文件测试（可按需运行），或者从标准单元测试集中移除这些文件写入和断言，以保持单元测试的无副作用性。
建议实现如下：
```csharp
[Fact]
public void DumpOutputFiles()
{
if (!File.Exists(UgcPath)) throw new SkipException($"Missing: {UgcPath}");
// Use an isolated temp directory to avoid polluting the repo and to be CI‑safe.
var tempDir = Path.Combine(
Path.GetTempPath(),
"ChuTests",
"DumpOutputFiles",
Guid.NewGuid().ToString("N"));
Directory.CreateDirectory(tempDir);
var c2sOutputPath = Path.Combine(tempDir, "basic_output.c2s");
var ugcOutputPath = Path.Combine(tempDir, "0003_output.ugc");
try
{
var (ugc, _) = new UgcParser().Parse(File.ReadAllText(UgcPath));
var (c2sText, _) = new C2sGenerator().Generate(ugc);
File.WriteAllText(c2sOutputPath, c2sText);
if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
var (c2s, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
var (ugcText, _) = new UgcGenerator().Generate(c2s);
File.WriteAllText(ugcOutputPath, ugcText);
Assert.True(File.Exists(c2sOutputPath));
Assert.True(File.Exists(ugcOutputPath));
}
finally
{
// Best‑effort cleanup; ignore failures so test results are driven by assertions above.
try
{
if (Directory.Exists(tempDir))
{
Directory.Delete(tempDir, recursive: true);
}
}
catch
{
// intentionally ignored
}
}
}
```
1. 确保在 `tests/chu/ChuTests.cs` 顶部已经引入 `using System;` 和 `using System.IO;`，以便 `Guid`、`Path`、`File` 和 `Directory` 能正确解析；
2. 如果你已在测试集中使用特性/分类系统来标记集成测试，并希望明确指出这是非单元测试，可以按照现有约定为该方法加上额外特性，例如 `[Trait("Category", "Integration")]`。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进 Review 质量。
Original comment in English
Hey - I've found 11 issues, and left some high level feedback:
- Numeric parsing/formatting in the UGC/SUS paths currently rely on the current culture (e.g.,
double.TryParseinUgcParser.ParseHeaderLineand string interpolation like{b.Bpm:F5}/{sus.Bpm:F2}), which can break on non-en-USlocales; consider usingCultureInfo.InvariantCultureconsistently as done inC2sGenerator. - Tick-scaling helpers such as
UgcGenerator.ScaleUpNote(s(int v) => v * UgcTicksPerBeat / (C2sResolution / 4)) andSusGenerator.ScaleUpperform arithmetic inintand may overflow on large values; aligning these withC2sGenerator.ScaleNoteby promoting tolongbefore multiplication would make them safer and more consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Numeric parsing/formatting in the UGC/SUS paths currently rely on the current culture (e.g., `double.TryParse` in `UgcParser.ParseHeaderLine` and string interpolation like `{b.Bpm:F5}` / `{sus.Bpm:F2}`), which can break on non-`en-US` locales; consider using `CultureInfo.InvariantCulture` consistently as done in `C2sGenerator`.
- Tick-scaling helpers such as `UgcGenerator.ScaleUpNote` (`s(int v) => v * UgcTicksPerBeat / (C2sResolution / 4)`) and `SusGenerator.ScaleUp` perform arithmetic in `int` and may overflow on large values; aligning these with `C2sGenerator.ScaleNote` by promoting to `long` before multiplication would make them safer and more consistent.
## Individual Comments
### Comment 1
<location path="parser/chu/UgcParser.cs" line_range="136-78" />
<code_context>
+ alerts.Add(new Alert(Warning, $"@TICKS 格式错误: {line}") { Line = lineNum });
+ break;
+
+ case "@BEAT":
+ var beatParts = value.Split(' ');
+ if (beatParts.Length >= 3
+ && int.TryParse(beatParts[0], out var beatMeasure)
+ && int.TryParse(beatParts[1], out var beatNum)
+ && int.TryParse(beatParts[2], out var beatDen))
+ {
+ chart.BeatEvents.Add((beatMeasure, beatNum, beatDen));
+ }
+ else
+ {
+ alerts.Add(new Alert(Warning, $"@BEAT 格式错误: {line}") { Line = lineNum });
+ }
+ break;
+
+ case "@BPM":
</code_context>
<issue_to_address>
**issue (bug_risk):** @BEAT parsing assumes space separators but the generator emits tab separators, causing all BEAT lines to be treated as malformed.
In `ParseHeaderLine`, `value` is the substring after the first tab and may still contain tabs, but here it’s split on spaces: `value.Split(' ')`. `UgcGenerator.Serialize` emits `@BEAT` as `"@BEAT\t{b.Measure}\t{b.Num}\t{b.Den}"`, so there are no spaces, `beatParts.Length` is always 1, and all `@BEAT` lines are treated as malformed. To match the serializer, split on tabs or general whitespace instead, e.g. `value.Split('\t', ' ')` or `value.Split((char[])null, StringSplitOptions.RemoveEmptyEntries)`.
</issue_to_address>
### Comment 2
<location path="parser/chu/UgcParser.cs" line_range="151-78" />
<code_context>
+ case "@BPM":
</code_context>
<issue_to_address>
**issue (bug_risk):** @BPM parsing is incompatible with the serialized format that uses tabs instead of spaces.
For `@BPM`, `value` is taken as the substring after the first tab, so for a generated line like `@BPM 0'0 120.00000`, `value` is `"0'0 120.00000"`. The parser then calls `IndexOf(' ')`, which returns `-1` because the separator is a tab, causing the line to be treated as malformed. To stay compatible with `UgcGenerator`, this logic should split on tabs (or more generally whitespace) instead of assuming a space between `measure'offset` and BPM.
</issue_to_address>
### Comment 3
<location path="generator/chu/SusGenerator.cs" line_range="76-77" />
<code_context>
+ sb.AppendLine($"#REQUEST \"{sus.TicksPerBeat}\"");
+ sb.AppendLine();
+
+ foreach (var n in sus.Notes.OrderBy(n => n.Measure).ThenBy(n => n.Offset))
+ sb.AppendLine($"#{n.Measure:X2}{n.Offset:X3}:{FormatData(n)}");
+
+ return sb.ToString();
</code_context>
<issue_to_address>
**issue (bug_risk):** SUS generator uses a 3-digit hex tick field while the parser only reads 2 digits, breaking round-trips.
The generator emits a 5‑digit timing field `"{measure:X2}{offset:X3}"`, but `SusParser.ParseNoteLine` expects `#MMTT:data` and only reads 2 digits for the tick (`[..2]` for measure, `[2..4]` for tick). The extra tick digit is silently dropped, truncating offsets > 0xFF and losing precision on round‑trip. Please either change the generator to use a 2‑digit tick (and align timing resolution accordingly) or update the parser to accept the 3‑digit tick format (e.g., `[..2]` for measure and the remainder for tick) so the formats are consistent and timing is preserved.
</issue_to_address>
### Comment 4
<location path="parser/chu/SusParser.cs" line_range="42" />
<code_context>
+ private static void ParseAirTarget(string dataStr, SusChart chart, List<Alert> alerts, int lineNum)
</code_context>
<issue_to_address>
**issue (bug_risk):** SUS AIR/ADW target parsing expects hex-encoded integers, but the generator writes raw `TargetNote` strings.
`ParseAirTarget` enforces `dataStr.Length >= 8` and decodes `HexToInt(dataStr[6..8])`, but `SusGenerator.FormatData` emits AIR/ADW as `"{tc}{lw}{n.TargetNote}"`, where `TargetNote` can be a single non-hex character (e.g., "N"). This can produce strings shorter than 8 chars or with non-hex content, causing warnings or incorrect targets. Please either encode `TargetNote` as a hex value in the generator to match this parser, or update the parser to treat the trailing substring as a raw target token instead of hex.
</issue_to_address>
### Comment 5
<location path="generator/chu/UgcGenerator.cs" line_range="88" />
<code_context>
+ sb.AppendLine($"@CONST\t{ugc.Constant:F5}");
+ sb.AppendLine($"@SONGID\t{ugc.SongId}");
+ sb.AppendLine($"@TICKS\t{ugc.TicksPerBeat}");
+ foreach (var b in ugc.BeatEvents) sb.AppendLine($"@BEAT\t{b.Measure}\t{b.Num}\t{b.Den}");
+ foreach (var b in ugc.BpmEvents) sb.AppendLine($"@BPM\t{b.Measure}'{b.Offset}\t{b.Bpm:F5}");
+ sb.AppendLine("@TIL\t0\t0'0\t1.00000");
</code_context>
<issue_to_address>
**suggestion (bug_risk):** UgcGenerator’s @BEAT serialization uses tabs, which currently conflicts with UgcParser’s expectation of space-separated values.
Together with `UgcParser.ParseHeaderLine`, this means `@BEAT` is written as tab‑separated but parsed as space‑separated (`value.Split(' ')`). Even after aligning the parser, please choose a single delimiter (tabs vs spaces) and document it in both serializer and parser to avoid future inconsistencies, especially for hand-edited charts.
Suggested implementation:
```csharp
sb.AppendLine($"@TICKS\t{ugc.TicksPerBeat}");
// NOTE: @BEAT fields are space-separated; keep this delimiter in sync with UgcParser.ParseHeaderLine
foreach (var b in ugc.BeatEvents) sb.AppendLine($"@BEAT {b.Measure} {b.Num} {b.Den}");
foreach (var b in ugc.BpmEvents) sb.AppendLine($"@BPM\t{b.Measure}'{b.Offset}\t{b.Bpm:F5}");
```
To fully implement the suggestion, UgcParser.ParseHeaderLine (or the specific `@BEAT` parsing path) should be updated to:
1. Use the same delimiter (spaces) as the serializer, e.g. `line.Split(' ', StringSplitOptions.RemoveEmptyEntries)` for the `@BEAT` payload.
2. Add a corresponding comment in UgcParser documenting that `@BEAT` fields are space-separated and must remain in sync with UgcGenerator’s serialization.
</issue_to_address>
### Comment 6
<location path="generator/chu/C2sGenerator.cs" line_range="102-103" />
<code_context>
+ sb.AppendLine($"SFL\t{s.Measure}\t{s.Offset}\t{s.Duration}\t{Mlt(s.Multiplier)}");
+ sb.AppendLine();
+
+ foreach (var n in chart.Notes.OrderBy(n => n.Measure * C2sResolution + n.Offset))
+ sb.AppendLine(FormatNote(n));
+
+ sb.AppendLine();
</code_context>
<issue_to_address>
**issue (bug_risk):** ALD/ASD note types parsed from C2S are not emitted distinctly and fall back to TAP in the generator.
`C2sParser.ParseNote` treats `ALD` and `ASD` as distinct note types with extra properties, but `C2sGenerator.FormatNote` lacks corresponding cases and falls back to the default TAP serialization. This will silently degrade these notes on export. Please either add explicit `ALD`/`ASD` handling in `FormatNote` or log/emit a warning when they’re encountered so users are aware of the loss of detail.
</issue_to_address>
### Comment 7
<location path="tests/chu/ChuTests.cs" line_range="7" />
<code_context>
+
+namespace MuConvert.Tests.chu;
+
+public class ChuTests
+{
+ private static string TestsetDir => Path.Combine(AppContext.BaseDirectory, "..", "..", "..", "chu", "testset");
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests that cover SUS parser/generator and cross-format conversions
Currently only C2S and UGC paths are covered. Please also add tests that (1) parse a SUS test chart and verify key fields/notes, (2) round-trip SUS via SusGenerator/SusParser (like C2sRoundTrip), and (3) exercise C2S → SUS and UGC → SUS conversions to verify scaling and type mapping. This will better validate the full C2S/UGC/SUS conversion matrix.
Suggested implementation:
```csharp
private static string TestsetDir => Path.Combine(AppContext.BaseDirectory, "..", "..", "..", "chu", "testset");
private static string OfficialDir => Path.Combine(TestsetDir, "官谱", "B.B.K.K.B.K.K");
private static string CustomDir => Path.Combine(TestsetDir, "自制谱", "Example");
private static string C2sPath => Path.Combine(OfficialDir, "0003_00.c2s");
private static string UgcPath => Path.Combine(CustomDir, "basic.ugc");
private static string SusPath => Path.Combine(OfficialDir, "0003_00.sus");
[Fact]
public void CanParseOfficialSus()
{
if (!File.Exists(SusPath)) throw new SkipException($"Missing: {SusPath}");
var susText = File.ReadAllText(SusPath);
var (chart, _) = new SusParser().Parse(susText);
Assert.NotNull(chart);
}
[Fact]
public void SusRoundTrip()
{
if (!File.Exists(SusPath)) throw new SkipException($"Missing: {SusPath}");
var originalSus = File.ReadAllText(SusPath);
var (chart, _) = new SusParser().Parse(originalSus);
var generatedSus = new SusGenerator().Generate(chart);
Assert.False(string.IsNullOrWhiteSpace(generatedSus));
var (roundTrippedChart, _) = new SusParser().Parse(generatedSus);
Assert.NotNull(roundTrippedChart);
}
[Fact]
public void CanConvertC2sToSus()
{
if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
var (chart, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
var susText = new SusGenerator().Generate(chart);
Assert.False(string.IsNullOrWhiteSpace(susText));
var (susChart, _) = new SusParser().Parse(susText);
Assert.NotNull(susChart);
}
[Fact]
public void CanConvertUgcToSus()
{
if (!File.Exists(UgcPath)) throw new SkipException($"Missing: {UgcPath}");
var (chart, _) = new UgcParser().Parse(File.ReadAllText(UgcPath));
var susText = new SusGenerator().Generate(chart);
Assert.False(string.IsNullOrWhiteSpace(susText));
var (susChart, _) = new SusParser().Parse(susText);
Assert.NotNull(susChart);
}
[Fact]
public void CanParseOfficialC2S()
```
These edits assume:
1. `SusParser` and `SusGenerator` exist in the `MuConvert.parser` / `MuConvert.chu` namespaces and expose `Parse(string)` and `Generate(chart)` methods with the same tuple shape as `C2sParser`/`UgcParser`.
2. There is an official SUS file at `官谱/B.B.K.K.B.K.K/0003_00.sus`. If your test data uses a different filename or directory structure, adjust `SusPath` accordingly.
3. If the chart type exposes richer properties (e.g., note counts, timing, or lane data) and there are existing helpers for deep equality between charts, you can extend the assertions in these tests to compare those fields to more directly validate scaling and type mapping across formats.
</issue_to_address>
### Comment 8
<location path="tests/chu/ChuTests.cs" line_range="25-31" />
<code_context>
+ }
+
+ [Fact]
+ public void C2sRoundTrip()
+ {
+ if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
+ var (chart, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
+ var (rt, _) = new C2sGenerator().Generate(chart);
+ var (reparsed, _) = new C2sParser().Parse(rt);
+ Assert.Equal(chart.Notes.Count, reparsed.Notes.Count);
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen C2S round-trip assertions beyond just comparing note counts
Right now this only compares the total note count after parse → generate → parse. That could still pass if notes are reordered, timings change, or fields are dropped while keeping the same count. Please tighten the assertion: for example, compare a sorted projection of key fields (measure, offset, type, cell, width, duration/end position, etc.), or at least assert that representative notes (e.g., slide, hold, air) survive the round trip unchanged. This will better validate that the parser/generator pair is lossless.
Suggested implementation:
```csharp
[Fact]
public void C2sRoundTrip()
{
if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
var (chart, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
var (rt, _) = new C2sGenerator().Generate(chart);
var (reparsed, _) = new C2sParser().Parse(rt);
// Basic sanity: note count must be preserved
Assert.Equal(chart.Notes.Count, reparsed.Notes.Count);
// Stronger guarantee: the set of notes (by their public properties) must be preserved
var originalSnapshots = chart.Notes
.Select(SnapshotNote)
.OrderBy(s => s)
.ToArray();
var reparsedSnapshots = reparsed.Notes
.Select(SnapshotNote)
.OrderBy(s => s)
.ToArray();
Assert.Equal(originalSnapshots, reparsedSnapshots);
}
```
```csharp
private static string C2sPath => Path.Combine(OfficialDir, "0003_00.c2s");
private static string UgcPath => Path.Combine(CustomDir, "basic.ugc");
/// <summary>
/// Creates a stable, comparable string snapshot of a note by concatenating all
/// of its public instance properties in name-sorted order. This lets the
/// round-trip test verify that no note data was lost or reordered without
/// needing to know the concrete note type shape.
/// </summary>
private static string SnapshotNote(object note)
{
if (note is null) return string.Empty;
var type = note.GetType();
var props = type.GetProperties(System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.Public);
var parts = props
.OrderBy(p => p.Name)
.Select(p => $"{p.Name}={p.GetValue(note)}");
return string.Join("|", parts);
}
```
This change assumes `System.Linq` is already imported in `ChuTests.cs` for `Select`, `OrderBy`, and `ToArray`.
If it is not, add `using System.Linq;` at the top of the file with the other `using` directives.
</issue_to_address>
### Comment 9
<location path="tests/chu/ChuTests.cs" line_range="16-21" />
<code_context>
+ private static string UgcPath => Path.Combine(CustomDir, "basic.ugc");
+
+ [Fact]
+ public void CanParseOfficialC2S()
+ {
+ if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
+ var (chart, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
+ Assert.NotEmpty(chart.Notes);
+ Assert.Equal(384, chart.Resolution);
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding negative/edge-case parsing tests for C2S/UGC to validate alert behavior
These tests currently only cover happy-path charts. Since the parsers emit `Alert` objects for malformed input (e.g., unknown note types, bad headers, missing follower lines), please add focused unit tests with small synthetic inputs that hit those error paths and assert that parsing succeeds while producing the expected alerts (LEVEL, message, and line info). This will help catch regressions in error handling and localization (e.g., `Locale.C2SUnknownNoteType`).
Suggested implementation:
```csharp
private static string UgcPath => Path.Combine(CustomDir, "basic.ugc");
[Fact]
public void CanParseOfficialC2S()
{
if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
var (chart, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
Assert.NotEmpty(chart.Notes);
Assert.Equal(384, chart.Resolution);
}
[Fact]
public void C2S_UnknownNoteType_ProducesAlert()
{
// Minimal C2S chart with an unknown note type to drive the "unknown note type" alert.
const string input =
"VERSION:1\n" +
"TITLE:Test\n" +
"ARTIST:Test\n" +
"BEAT:4/4\n" +
"BPM:120\n" +
"START:\n" +
"001,1,UNKNOWN,1,0\n";
var (chart, alerts) = new C2sParser().Parse(input);
Assert.NotNull(chart);
Assert.NotNull(alerts);
var alert = Assert.Single(alerts);
Assert.Equal(AlertLevel.Error, alert.Level);
Assert.Equal(Locale.C2SUnknownNoteType, alert.Message);
Assert.True(alert.Line > 0);
}
[Fact]
public void C2S_MissingFollowerLine_ProducesAlert()
{
// Start of a flick/slide without the expected follower line.
const string input =
"VERSION:1\n" +
"TITLE:Test\n" +
"ARTIST:Test\n" +
"BEAT:4/4\n" +
"BPM:120\n" +
"START:\n" +
"001,1,START_FLICK,1,0\n";
var (chart, alerts) = new C2sParser().Parse(input);
Assert.NotNull(chart);
Assert.NotNull(alerts);
Assert.NotEmpty(chart.Notes);
var alert = Assert.Single(alerts);
Assert.Equal(AlertLevel.Warning, alert.Level);
Assert.Equal(Locale.C2SMissingFollowerNote, alert.Message);
Assert.True(alert.Line > 0);
}
[Fact]
public void Ugc_UnknownNoteType_ProducesAlert()
{
// UGC chart with an unknown note type token.
const string input =
"#TITLE:Test\n" +
"#ARTIST:Test\n" +
"#BPM:120\n" +
"#DIFFICULTY:1\n" +
"001,1,UNKNOWN,1,0\n";
var (chart, alerts) = new UgcParser().Parse(input);
Assert.NotNull(chart);
Assert.NotNull(alerts);
var alert = Assert.Single(alerts);
Assert.Equal(AlertLevel.Error, alert.Level);
Assert.Equal(Locale.UGCUnknownNoteType, alert.Message);
Assert.True(alert.Line > 0);
}
[Fact]
public void Ugc_InvalidHeader_ProducesAlert()
{
// Header with missing title and invalid BPM to hit header-validation alerts.
const string input =
"#TITLE:\n" + // missing title value
"#ARTIST:Test\n" +
"#BPM:abc\n" + // invalid BPM
"#DIFFICULTY:1\n" +
"001,1,TAP,1,0\n";
var (chart, alerts) = new UgcParser().Parse(input);
Assert.NotNull(chart);
Assert.NotNull(alerts);
Assert.NotEmpty(chart.Notes);
Assert.NotEmpty(alerts);
Assert.Contains(alerts, a => a.Level == AlertLevel.Error && a.Message == Locale.UGCInvalidBpm);
Assert.All(alerts, a => Assert.True(a.Line > 0));
}
```
1. Ensure the file has the necessary `using` directives for the parser and alert types and localization:
- `using MuConvert.Chu;` (or the actual namespace containing `C2sParser`/`UgcParser`).
- `using MuConvert.Chu.Localization;` (or the actual namespace for `Locale`).
- `using MuConvert.Chu.Alerts;` (or the actual namespace for `Alert`/`AlertLevel`), if they are not already in scope.
2. Adjust property and type names if they differ from the assumptions above:
- If alerts expose properties like `Severity`, `MessageKey`, `LineNumber`, etc., update the assertions (`alert.Level`, `alert.Message`, `alert.Line`) to match the real API and to compare against the correct localized value/key (e.g., `Locale.C2SUnknownNoteType` might be a key instead of a string).
- If the parsers return something other than a tuple `(chart, alerts)` (e.g., a custom result type), adapt the destructuring and access accordingly.
3. The synthetic C2S/UGC snippets should be tweaked to match the real grammar if necessary (field counts, tokens such as `START_FLICK`, etc.) so that they parse successfully while still triggering the targeted alert paths.
4. If `CanParseOfficialC2S` already exists elsewhere in `ChuTests.cs`, remove the duplicate definition added in this patch to avoid having two methods with the same name.
</issue_to_address>
### Comment 10
<location path="tests/chu/ChuTests.cs" line_range="43-52" />
<code_context>
+ }
+
+ [Fact]
+ public void UgcToC2sViaGenerator()
+ {
+ if (!File.Exists(UgcPath)) throw new SkipException($"Missing: {UgcPath}");
+ var (ugc, _) = new UgcParser().Parse(File.ReadAllText(UgcPath));
+ var (c2sText, _) = new C2sGenerator().Generate(ugc);
+ Assert.Contains("VERSION", c2sText);
+ Assert.Contains("TAP\t", c2sText);
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** Tighten UgcToC2sViaGenerator assertions to validate more than the presence of keywords
The current assertions only check for "VERSION" and "TAP\t", so the test could still pass if most notes are dropped or timing is incorrect. Please consider round-tripping: parse the generated C2S with `C2sParser` and assert on the resulting chart (e.g., non-empty notes, expected note types, approximate note count or specific known note properties) to make this a more robust cross-format conversion test.
```suggestion
[Fact]
public void UgcToC2sViaGenerator()
{
if (!File.Exists(UgcPath)) throw new SkipException($"Missing: {UgcPath}");
// Parse UGC chart
var (ugcChart, _) = new UgcParser().Parse(File.ReadAllText(UgcPath));
Assert.NotEmpty(ugcChart.Notes);
// Generate C2S text from UGC chart
var (c2sText, _) = new C2sGenerator().Generate(ugcChart);
Assert.Contains("VERSION", c2sText);
Assert.Contains("TAP\t", c2sText);
// Parse back the generated C2S into a chart
var (c2sChart, _) = new C2sParser().Parse(c2sText);
// Ensure the generated chart is non-empty
Assert.NotEmpty(c2sChart.Notes);
// Ensure note count is approximately preserved across formats
var originalNoteCount = ugcChart.Notes.Count;
var generatedNoteCount = c2sChart.Notes.Count;
Assert.InRange(generatedNoteCount, (int)(originalNoteCount * 0.8), (int)(originalNoteCount * 1.2));
}
```
</issue_to_address>
### Comment 11
<location path="tests/chu/ChuTests.cs" line_range="54-63" />
<code_context>
+ }
+
+ [Fact]
+ public void DumpOutputFiles()
+ {
+ if (!File.Exists(UgcPath)) throw new SkipException($"Missing: {UgcPath}");
+ var (ugc, _) = new UgcParser().Parse(File.ReadAllText(UgcPath));
+ var (c2sText, _) = new C2sGenerator().Generate(ugc);
+ File.WriteAllText(Path.Combine(OfficialDir, "basic_output.c2s"), c2sText);
+
+ if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
+ var (c2s, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
+ var (ugcText, _) = new UgcGenerator().Generate(c2s);
+ File.WriteAllText(Path.Combine(OfficialDir, "0003_output.ugc"), ugcText);
+
+ Assert.True(File.Exists(Path.Combine(OfficialDir, "basic_output.c2s")));
+ Assert.True(File.Exists(Path.Combine(OfficialDir, "0003_output.ugc")));
+ }
+}
</code_context>
<issue_to_address>
**suggestion (testing):** Avoid filesystem side effects in DumpOutputFiles or mark it explicitly as non-unit/integration
This test writes to the repo’s `OfficialDir` and then asserts on those files, which can cause flakiness (parallel runs, read-only CI, dirty working trees). Please either move this into an integration/golden-file style test that uses a temp directory (and can be run conditionally), or remove the filesystem writes/assertions from the standard unit test suite so that unit tests remain side-effect free.
Suggested implementation:
```csharp
[Fact]
public void DumpOutputFiles()
{
if (!File.Exists(UgcPath)) throw new SkipException($"Missing: {UgcPath}");
// Use an isolated temp directory to avoid polluting the repo and to be CI‑safe.
var tempDir = Path.Combine(
Path.GetTempPath(),
"ChuTests",
"DumpOutputFiles",
Guid.NewGuid().ToString("N"));
Directory.CreateDirectory(tempDir);
var c2sOutputPath = Path.Combine(tempDir, "basic_output.c2s");
var ugcOutputPath = Path.Combine(tempDir, "0003_output.ugc");
try
{
var (ugc, _) = new UgcParser().Parse(File.ReadAllText(UgcPath));
var (c2sText, _) = new C2sGenerator().Generate(ugc);
File.WriteAllText(c2sOutputPath, c2sText);
if (!File.Exists(C2sPath)) throw new SkipException($"Missing: {C2sPath}");
var (c2s, _) = new C2sParser().Parse(File.ReadAllText(C2sPath));
var (ugcText, _) = new UgcGenerator().Generate(c2s);
File.WriteAllText(ugcOutputPath, ugcText);
Assert.True(File.Exists(c2sOutputPath));
Assert.True(File.Exists(ugcOutputPath));
}
finally
{
// Best‑effort cleanup; ignore failures so test results are driven by assertions above.
try
{
if (Directory.Exists(tempDir))
{
Directory.Delete(tempDir, recursive: true);
}
}
catch
{
// intentionally ignored
}
}
}
```
1. Ensure `using System;` and `using System.IO;` are present at the top of `tests/chu/ChuTests.cs` so that `Guid`, `Path`, `File`, and `Directory` resolve correctly.
2. If you have an existing trait/category system for integration tests and you still want to call this out as non‑unit, you can decorate the method with an additional attribute, e.g. `[Trait("Category", "Integration")]`, following whatever convention the rest of the test suite uses.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| { | ||
| alerts.Add(new Alert(Warning, $"意外的行（不以 # 开头）: {line}") { Line = i + 1 }); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): SUS AIR/ADW 目标解析期望使用十六进制编码的整数，但生成器写入的是原始的 TargetNote 字符串。
ParseAirTarget 要求 dataStr.Length >= 8，并解码 HexToInt(dataStr[6..8])，但 SusGenerator.FormatData 对 AIR/ADW 的输出是 "{tc}{lw}{n.TargetNote}"，其中 TargetNote 可以是一个非十六进制字符（例如 "N"）。这会生成短于 8 个字符或包含非十六进制内容的字符串，从而导致警告或错误的目标解析。请考虑要么在生成器中将 TargetNote 编码为十六进制值以匹配当前解析逻辑，要么更新解析器，将结尾的子串视为原始的目标标记而不是十六进制数。
Original comment in English
issue (bug_risk): SUS AIR/ADW target parsing expects hex-encoded integers, but the generator writes raw TargetNote strings.
ParseAirTarget enforces dataStr.Length >= 8 and decodes HexToInt(dataStr[6..8]), but SusGenerator.FormatData emits AIR/ADW as "{tc}{lw}{n.TargetNote}", where TargetNote can be a single non-hex character (e.g., "N"). This can produce strings shorter than 8 chars or with non-hex content, causing warnings or incorrect targets. Please either encode TargetNote as a hex value in the generator to match this parser, or update the parser to treat the trailing substring as a raw target token instead of hex.
UgcParser/SusParser: TryParse 统一使用 CultureInfo.InvariantCulture UgcGenerator/SusGenerator: ScaleUp 乘法提升为 long 防溢出
Contributor
Author
|
根据 sourcery-ai 审查修复了两个问题：
commit: e2488df |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for CHUNITHM chart formats (C2S, SUS, and UGC) by implementing a common Intermediate Representation (IR), parsers, and generators. The feedback highlights several critical issues where EndTime calculations are hardcoded to zero in SusChart and UgcChart, and missing logic for specific note types (like ALD, ASD, and various EX-notes) in both parsers and generators. Additionally, improvements were suggested to use the Resolution property instead of hardcoded values in C2sChart and to adopt more robust hex parsing in SusParser.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
15 issues found across 23 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="generator/chu/C2sGenerator.cs">
<violation number="1" location="generator/chu/C2sGenerator.cs:119">
P1: `ALD`/`ASD` notes are not serialized and are silently downgraded to `TAP`.</violation>
</file>
<file name="chart/chu/UgcChart.cs">
<violation number="1" location="chart/chu/UgcChart.cs:25">
P2: `EndTime` is hard-coded to 0, so non-empty UGC charts report an incorrect finish time.</violation>
</file>
<file name="parser/chu/UgcParser.cs">
<violation number="1" location="parser/chu/UgcParser.cs:302">
P1: Slide follower lines are accepted even when end position data is missing, causing malformed input to be silently converted into a default end cell/width.</violation>
</file>
<file name="i18n/Locale.ko.resx">
<violation number="1" location="i18n/Locale.ko.resx:274">
P1: These new Korean resource entries are stored as `ResXNullRef` and then used as `string.Format` templates, which can produce null format strings and break warning generation at runtime.</violation>
</file>
<file name="i18n/Locale.ja.resx">
<violation number="1" location="i18n/Locale.ja.resx:274">
P1: This resource key is defined as `ResXNullRef`, so ja locale resolves it to `null` and `string.Format(Locale.C2SUnknownNoteType, ...)` can throw at runtime.</violation>
<violation number="2" location="i18n/Locale.ja.resx:277">
P1: This key is also `ResXNullRef`; when generators call `string.Format(Locale.ChuGeneratorUnsupported, ...)` under ja locale, it can fail with a null format string.</violation>
</file>
<file name="chart/chu/SusChart.cs">
<violation number="1" location="chart/chu/SusChart.cs:18">
P1: `EndTime` is hardcoded to `0`, so SUS charts always report zero duration even when notes exist.</violation>
</file>
<file name="tests/chu/ChuTests.cs">
<violation number="1" location="tests/chu/ChuTests.cs:59">
P2: Don't dump generated artifacts into the fixture tree and assert their existence; this mutates the repo and the test can't catch generator regressions.</violation>
</file>
<file name="parser/chu/SusParser.cs">
<violation number="1" location="parser/chu/SusParser.cs:25">
P1: MNE type code is inconsistent with generator (`0x0A` vs `0x10`), causing generated MNE notes to be unrecognized.</violation>
<violation number="2" location="parser/chu/SusParser.cs:113">
P1: SUS tick parsing truncates 3-digit offsets, so parsed note timing is wrong for valid generated SUS lines.</violation>
</file>
<file name="chart/chu/C2sChart.cs">
<violation number="1" location="chart/chu/C2sChart.cs:21">
P2: `StartTime`/`EndTime` hard-code 384 ticks per measure instead of using `Resolution`, which produces incorrect timing when parsed C2S files specify a different resolution.</violation>
</file>
<file name="generator/chu/UgcGenerator.cs">
<violation number="1" location="generator/chu/UgcGenerator.cs:61">
P2: Air-note `TargetNote` is overwritten to `"N"` during scaling, causing data loss for C2S notes that already provide a target.</violation>
<violation number="2" location="generator/chu/UgcGenerator.cs:88">
P1: `@BEAT`/`@BPM` are serialized with tab-delimited values, but the UGC parser expects space-delimited values, so generated timing headers cannot be parsed correctly on re-import.</violation>
</file>
<file name="generator/chu/SusGenerator.cs">
<violation number="1" location="generator/chu/SusGenerator.cs:45">
P1: UGC→SUS conversion incorrectly rescales note timing by 5x, producing wrong note positions and durations.</violation>
<violation number="2" location="generator/chu/SusGenerator.cs:55">
P1: C2S→SUS timing conversion assumes fixed RESOLUTION=384 and mis-converts charts with other C2S resolutions.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
P1: ALD/ASD 序列化缺失、@beat tab/space 不一致、SUS MNE 类型码、SUS tick 位数、ko/ja locale null crash、UGC→SUS 缩放错误 P2: EndTime=0、Dump 写临时目录、Air TargetNote 覆写、C2sChart 用 Resolution 代替硬编码
Contributor
Author
|
修复 review bot (sourcery-ai / gemini / cubic) 发现的 9 个问题: P1 (6)
P2 (3+1) commit: e4619d1, 220/220 全过 |
There was a problem hiding this comment.
5 issues found across 11 files (changes from recent commits).
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="generator/chu/SusGenerator.cs">
<violation number="1" location="generator/chu/SusGenerator.cs:45">
P1: UGC→SUS conversion now copies timing ticks directly, which breaks timing when UGC `@TICKS` is not 480.</violation>
</file>
<file name="generator/chu/C2sGenerator.cs">
<violation number="1" location="generator/chu/C2sGenerator.cs:118">
P1: ALD/ASD serialization column order does not match the C2S parser, causing generated notes to be mis-parsed on round-trip.</violation>
</file>
<file name="i18n/Locale.ko.resx">
<violation number="1" location="i18n/Locale.ko.resx:275">
P3: This Korean locale string is left in English; localize it to Korean to keep i18n output consistent.</violation>
<violation number="2" location="i18n/Locale.ko.resx:278">
P3: This Korean locale string is left in English; provide a Korean translation for consistency.</violation>
</file>
<file name="chart/chu/UgcChart.cs">
<violation number="1" location="chart/chu/UgcChart.cs:25">
P2: Guard against zero BPM before dividing in `EndTime`; malformed charts with `@BPM ... 0` can now throw `DivideByZeroException` when this property is read.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
1. ALD/ASD 序列化字段顺序修正 (去掉多余的 Cell+Width) 2. EndTime 加 BPM>0 防 DivByZero 3. ko.resx 韩文翻译
Contributor
Author
|
cubic 第2轮审查修复:
commit: cd61fd7, 220/220 全过 |
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
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="generator/chu/SusGenerator.cs">
<violation number="1" location="generator/chu/SusGenerator.cs:55">
P2: Guard `sourceTicksPerBeat` before scaling to prevent divide-by-zero on validly parsed but invalid tick metadata.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
实际 C2S ULTIMA 谱面 (_04.c2s) 包含这些 EX 类型，UGC 无对应码，映射为基础类型
Contributor
Author
兼容性说明本 PR 新增的 CHUNITHM 模块 ( C2S（完全可用）
UGC（基本可用，需要 review）
SUS（实验性，需更多测试）
|
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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="generator/chu/UgcGenerator.cs">
<violation number="1" location="generator/chu/UgcGenerator.cs:115">
P1: Newly added HXD/SXD/SXC type mappings are not reflected in Serialize’s duration handling, so these notes can be output without their hold/slide continuation lines.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
(cherry picked from commit a47ef2e)
重构为各分支独立设置字段，ALD/ASD 不再从 p[3]/p[4] 误设 Cell/Width
Contributor
Author
MuConvert.chu 使用指南快速开始using MuConvert.chu;
using MuConvert.parser;
// 解析 C2S
var (c2sChart, alerts) = new C2sParser().Parse(c2sText);
// 解析 UGC
var (ugcChart, alerts) = new UgcParser().Parse(ugcText);
// 解析 SUS
var (susChart, alerts) = new SusParser().Parse(susText);
// 生成 — 接受任意 IChuChart，内部自动转换
var (c2sText, alerts) = new C2sGenerator().Generate(ugcChart); // UGC -> C2S
var (c2sText, alerts) = new C2sGenerator().Generate(susChart); // SUS -> C2S
var (ugcText, alerts) = new UgcGenerator().Generate(c2sChart); // C2S -> UGC
var (susText, alerts) = new SusGenerator().Generate(c2sChart); // C2S -> SUS告警处理所有解析器和生成器都返回
try
{
var (chart, alerts) = new UgcParser().Parse(text);
foreach (var a in alerts.Where(a => a.Level == Alert.LEVEL.Warning))
Console.WriteLine($"行{a.Line}: {a.Description}");
}
catch (ConversionException e)
{
Console.WriteLine($"解析失败: {e.Message}");
}格式差异与限制
测试dotnet test --filter "FullyQualifiedName~ChuTests" |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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="Program.cs">
<violation number="1" location="Program.cs:202">
P2: When a directory contains multiple CHUNITHM charts, only the first file is converted and the others are silently ignored.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
主要重构是AI写的，人工做了一部分优化和进行review
2. 现在的ugcparser，@SPDMOD的解析是错的。
3. 抽取TryParseUgcMeasureTick方法
实际上，经过前几步之后，各种Chart的内容（无论是字段还是类型）都实现了完全统一了。所以这里只需要修改类名就可以了。
…和color的解析/生成流程，确保实现正确；修复一些bug
1. FindAllPrevious，防止自环； 2. C2sGenerator，生成slide segments确保首尾相接，不会接不上
Starrah
approved these changes
May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概述
新增 MuConvert.chu 命名空间，支持 C2S/UGC/SUS 三种 CHUNITHM 谱面格式的解析、生成与跨格式互转。
架构
Parser → IR → Generator 三层，IChuChart 统一接口，Generator 内嵌转换逻辑。
改动
测试
219/219 全过 (新增 5 项)
Summary by Sourcery
添加对 CHUNITHM 谱面的全面支持，包括共享的中间表示（IR）、解析器、生成器、国际化（i18n）以及测试。
New Features:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Add comprehensive CHUNITHM chart support with shared IR, parsers, generators, i18n, and tests.
New Features:
Enhancements:
Tests: