chunithm转谱：IR大重构，修正当前C2S和UGC的Parser与Generator中的许多错误。#3
Open
Conversation
主要重构是AI写的，人工做了一部分优化和进行review
2. 现在的ugcparser，@SPDMOD的解析是错的。
3. 抽取TryParseUgcMeasureTick方法
实际上，经过前几步之后，各种Chart的内容（无论是字段还是类型）都实现了完全统一了。所以这里只需要修改类名就可以了。
Reviewer's Guide将 CHUNITHM 的中间表示重构为统一的 ChuChart/ChuNote 模型（使用有理数时间），更新所有 C2S/UGC/SUS 解析器和生成器以使用该模型，修复许多此前不正确的 UGC 和 C2S 转换（尤其是滑条和空气类音符），并相应加强往返测试和文档。 使用 ChuChart 进行 UGC 滑条与空气滑条往返转换的时序图sequenceDiagram
participant User
participant Program as Program_RunConvertChuSingleFile
participant UgcParser
participant ChuChart
participant BaseChuParser
participant UgcGenerator
User->>Program: request convert UGC chart
Program->>UgcParser: Parse(text)
UgcParser->>ChuChart: new ChuChart()
UgcParser->>ChuChart: parse header (BPM, BEAT, SPDMOD -> BpmList,MetList,SflList)
loop note lines
UgcParser->>ChuChart: ParseNoteLine
alt slide parent (s/S)
UgcParser->>ChuChart: create first ChuNote (SLD/ASD)
UgcParser->>ChuChart: add segment notes for followers
else other notes
UgcParser->>ChuChart: add ChuNote
end
end
UgcParser->>ChuChart: FinalizeUgcSflDurations
UgcParser->>BaseChuParser: FillAllPrevious(chart,alerts)
BaseChuParser->>ChuChart: scan notes, build endDict
BaseChuParser->>ChuChart: set Previous for SLD/ASD/AIR/AHD
BaseChuParser-->>UgcParser: chart with linked slide chains
UgcParser-->>Program: ChuChart, alerts
Program->>UgcGenerator: Generate(ChuChart)
UgcGenerator->>ChuChart: Sort()
UgcGenerator->>UgcGenerator: BuildSlideChains(Notes)
loop notes
alt slide head (SLD/ASD)
UgcGenerator->>UgcGenerator: UCode(head)
UgcGenerator-->>Program: write parent line s/S
UgcGenerator->>UgcGenerator: emit follower lines using segments
else hold / air hold
UgcGenerator->>UgcGenerator: UCode(note)
UgcGenerator-->>Program: write base line
UgcGenerator-->>Program: write follower duration line
else other
UgcGenerator->>UgcGenerator: UCode(note)
UgcGenerator-->>Program: write single line
end
end
UgcGenerator-->>Program: ugcText, alerts
Program-->>User: output UGC chart text
统一 ChuChart 与 ChuNote IR 的类图classDiagram
direction TB
class BaseNote {
<<abstract>>
+Rational Time
+Rational EndTime
}
class ChuNote {
+string Type
+int Cell
+int Width
+Rational Duration
+int EndCell
+int EndWidth
+ChuNote Previous
+string Tag
+List~int~ ExtraData
+string TargetNote
+Rational EndTime
-int _endCell
-int _endWidth
}
BaseNote <|-- ChuNote
class BaseChart_ChuNote_ {
<<abstract>>
+List~ChuNote~ Notes
+BPMList BpmList
+List~MET~ MetList
+Rational ToSecond(Rational time)
+void Sort()
}
class ChuChart {
+string Title
+string Artist
+string Designer
+int Difficulty
+string DisplayLevel
+decimal Level
+string MusicId
+List~(Rational,Rational,decimal)~ SflList
}
BaseChart_ChuNote_ <|-- ChuChart
class BPMList {
+Rational ConvertTime(Rational startTime,Rational value,decimal srcBpm,decimal destBpm)
+ValueTuple~decimal,decimal,decimal,decimal~ BPM_DEF()
}
class BPM {
+Rational Time
+decimal Bpm
}
BPMList --> BPM : contains
class MET {
+Rational Time
+int Numerator
+int Denominator
}
ChuChart --> BPMList : has
ChuChart --> MET : has MetList
class BaseChuParser {
<<abstract>>
+(ChuChart,List~Alert~) Parse(string text)
+void FillAllPrevious(ChuChart chart,List~Alert~ alerts,Dictionary~ChuNote,string~ rawTargetNote)
+static bool IsSlide(string t)
+static bool IsAirSlide(string t)
+static bool IsAir(string t)
+static bool IsAirHold(string t)
+static bool IsAirCrush(string t)
+static bool IsGeneralizedAir(string t)
}
class C2sParser {
+(ChuChart,List~Alert~) Parse(string text)
-Dictionary~ChuNote,string~ _rawTargetNote
}
class UgcParser {
+(ChuChart,List~Alert~) Parse(string text)
-int RSL
}
class SusParser {
+(ChuChart,List~Alert~) Parse(string text)
-int RSL
}
BaseChuParser <|-- C2sParser
BaseChuParser <|-- UgcParser
BaseChuParser <|-- SusParser
class C2sGenerator {
+(string,List~Alert~) Generate(ChuChart chart)
-string Serialize(ChuChart chart,List~Alert~ alerts)
}
class UgcGenerator {
+(string,List~Alert~) Generate(ChuChart chart)
-string Serialize(ChuChart ugc,List~Alert~ alerts)
-Dictionary~ChuNote,List~ChuNote~~ BuildSlideChains(List~ChuNote~ notes)
}
class SusGenerator {
+(string,List~Alert~) Generate(ChuChart chart)
-string Serialize(ChuChart sus)
}
ChuChart --> ChuNote : notes
C2sParser --> ChuChart : builds
UgcParser --> ChuChart : builds
SusParser --> ChuChart : builds
C2sGenerator --> ChuChart : consumes
UgcGenerator --> ChuChart : consumes
SusGenerator --> ChuChart : consumes
class Utils {
+int Tick(Rational time,int resolution,int extraTicks,int? maxTick)
+ValueTuple~int,int~ BarAndTick(Rational time,int resolution)
+Rational Max(Rational a,Rational b)
}
class ExtensionUtils {
+void Add(Dictionary~K,List~V~ dict,K key,V value)
}
BPMList ..> Utils : uses
C2sGenerator ..> Utils : uses
UgcGenerator ..> Utils : uses
SusGenerator ..> Utils : uses
BaseChuParser ..> Utils : uses
文件级变更
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideRefactors CHUNITHM’s intermediate representation into a unified ChuChart/ChuNote model with rational timing, updates all C2S/UGC/SUS parsers and generators to use it, fixes many previously incorrect UGC and C2S conversions (especially slides and air types), and strengthens round‑trip tests and docs accordingly. Sequence diagram for UGC slide and air slide round trip using ChuChartsequenceDiagram
participant User
participant Program as Program_RunConvertChuSingleFile
participant UgcParser
participant ChuChart
participant BaseChuParser
participant UgcGenerator
User->>Program: request convert UGC chart
Program->>UgcParser: Parse(text)
UgcParser->>ChuChart: new ChuChart()
UgcParser->>ChuChart: parse header (BPM, BEAT, SPDMOD -> BpmList,MetList,SflList)
loop note lines
UgcParser->>ChuChart: ParseNoteLine
alt slide parent (s/S)
UgcParser->>ChuChart: create first ChuNote (SLD/ASD)
UgcParser->>ChuChart: add segment notes for followers
else other notes
UgcParser->>ChuChart: add ChuNote
end
end
UgcParser->>ChuChart: FinalizeUgcSflDurations
UgcParser->>BaseChuParser: FillAllPrevious(chart,alerts)
BaseChuParser->>ChuChart: scan notes, build endDict
BaseChuParser->>ChuChart: set Previous for SLD/ASD/AIR/AHD
BaseChuParser-->>UgcParser: chart with linked slide chains
UgcParser-->>Program: ChuChart, alerts
Program->>UgcGenerator: Generate(ChuChart)
UgcGenerator->>ChuChart: Sort()
UgcGenerator->>UgcGenerator: BuildSlideChains(Notes)
loop notes
alt slide head (SLD/ASD)
UgcGenerator->>UgcGenerator: UCode(head)
UgcGenerator-->>Program: write parent line s/S
UgcGenerator->>UgcGenerator: emit follower lines using segments
else hold / air hold
UgcGenerator->>UgcGenerator: UCode(note)
UgcGenerator-->>Program: write base line
UgcGenerator-->>Program: write follower duration line
else other
UgcGenerator->>UgcGenerator: UCode(note)
UgcGenerator-->>Program: write single line
end
end
UgcGenerator-->>Program: ugcText, alerts
Program-->>User: output UGC chart text
Class diagram for unified ChuChart and ChuNote IRclassDiagram
direction TB
class BaseNote {
<<abstract>>
+Rational Time
+Rational EndTime
}
class ChuNote {
+string Type
+int Cell
+int Width
+Rational Duration
+int EndCell
+int EndWidth
+ChuNote Previous
+string Tag
+List~int~ ExtraData
+string TargetNote
+Rational EndTime
-int _endCell
-int _endWidth
}
BaseNote <|-- ChuNote
class BaseChart_ChuNote_ {
<<abstract>>
+List~ChuNote~ Notes
+BPMList BpmList
+List~MET~ MetList
+Rational ToSecond(Rational time)
+void Sort()
}
class ChuChart {
+string Title
+string Artist
+string Designer
+int Difficulty
+string DisplayLevel
+decimal Level
+string MusicId
+List~(Rational,Rational,decimal)~ SflList
}
BaseChart_ChuNote_ <|-- ChuChart
class BPMList {
+Rational ConvertTime(Rational startTime,Rational value,decimal srcBpm,decimal destBpm)
+ValueTuple~decimal,decimal,decimal,decimal~ BPM_DEF()
}
class BPM {
+Rational Time
+decimal Bpm
}
BPMList --> BPM : contains
class MET {
+Rational Time
+int Numerator
+int Denominator
}
ChuChart --> BPMList : has
ChuChart --> MET : has MetList
class BaseChuParser {
<<abstract>>
+(ChuChart,List~Alert~) Parse(string text)
+void FillAllPrevious(ChuChart chart,List~Alert~ alerts,Dictionary~ChuNote,string~ rawTargetNote)
+static bool IsSlide(string t)
+static bool IsAirSlide(string t)
+static bool IsAir(string t)
+static bool IsAirHold(string t)
+static bool IsAirCrush(string t)
+static bool IsGeneralizedAir(string t)
}
class C2sParser {
+(ChuChart,List~Alert~) Parse(string text)
-Dictionary~ChuNote,string~ _rawTargetNote
}
class UgcParser {
+(ChuChart,List~Alert~) Parse(string text)
-int RSL
}
class SusParser {
+(ChuChart,List~Alert~) Parse(string text)
-int RSL
}
BaseChuParser <|-- C2sParser
BaseChuParser <|-- UgcParser
BaseChuParser <|-- SusParser
class C2sGenerator {
+(string,List~Alert~) Generate(ChuChart chart)
-string Serialize(ChuChart chart,List~Alert~ alerts)
}
class UgcGenerator {
+(string,List~Alert~) Generate(ChuChart chart)
-string Serialize(ChuChart ugc,List~Alert~ alerts)
-Dictionary~ChuNote,List~ChuNote~~ BuildSlideChains(List~ChuNote~ notes)
}
class SusGenerator {
+(string,List~Alert~) Generate(ChuChart chart)
-string Serialize(ChuChart sus)
}
ChuChart --> ChuNote : notes
C2sParser --> ChuChart : builds
UgcParser --> ChuChart : builds
SusParser --> ChuChart : builds
C2sGenerator --> ChuChart : consumes
UgcGenerator --> ChuChart : consumes
SusGenerator --> ChuChart : consumes
class Utils {
+int Tick(Rational time,int resolution,int extraTicks,int? maxTick)
+ValueTuple~int,int~ BarAndTick(Rational time,int resolution)
+Rational Max(Rational a,Rational b)
}
class ExtensionUtils {
+void Add(Dictionary~K,List~V~ dict,K key,V value)
}
BPMList ..> Utils : uses
C2sGenerator ..> Utils : uses
UgcGenerator ..> Utils : uses
SusGenerator ..> Utils : uses
BaseChuParser ..> Utils : uses
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.
你好——我发现了 3 个问题，并给出了一些总体反馈：
- 现在各个 CHU 解析器/生成器使用了可变的静态字段来表示 resolution/ticks（例如
UgcParser、UgcGenerator、C2sParser、SusParser、SusGenerator中的RSL），这意味着在并行解析不同谱面，或者顺序解析时使用不同的@TICKS/RESOLUTION值时，它们会相互干扰；建议将这些改为实例字段，或者显式传入 resolution，以便将状态限定在每个谱面内。 - 在
C2sParser.ParseTiming中，你改成了使用decimal.Parse(p[3])/decimal.Parse(p[4])，但没有指定区域设置或错误处理，现在对于格式错误或带本地化格式的 BPM/SFL 值，会直接抛异常，而旧代码会安全地回退；使用decimal.TryParse(..., NumberStyles.Float, CultureInfo.InvariantCulture, out var bpm)（并在失败时给出默认值或警告）会让解析器更健壮。
给 AI Agent 的提示
请根据这次代码审查中的评论进行修改：
## 总体评论
- 现在各个 CHU 解析器/生成器使用了可变的静态字段来表示 resolution/ticks（例如 `UgcParser`、`UgcGenerator`、`C2sParser`、`SusParser`、`SusGenerator` 中的 `RSL`），这意味着在并行解析不同谱面，或者顺序解析时使用不同的 `@TICKS`/`RESOLUTION` 值时，它们会相互干扰；建议将这些改为实例字段，或者显式传入 resolution，以便将状态限定在每个谱面内。
- 在 `C2sParser.ParseTiming` 中，你改成了使用 `decimal.Parse(p[3])` / `decimal.Parse(p[4])`，但没有指定区域设置或错误处理，现在对于格式错误或带本地化格式的 BPM/SFL 值，会直接抛异常，而旧代码会安全地回退；使用 `decimal.TryParse(..., NumberStyles.Float, CultureInfo.InvariantCulture, out var bpm)`（并在失败时给出默认值或警告）会让解析器更健壮。
## 逐条评论
### 评论 1
<location path="chart/chu/ChuNote.cs" line_range="17-18" />
<code_context>
- public int HoldDuration { get; set; }
- /** SLD 持续时长 */
- public int SlideDuration { get; set; }
+ /** HLD/SLD/AHD/ASD等的 持续时长 */
+ public Rational Duration { get; set => field = value.CanonicalForm; } = 0;
+
/** SLD 终点列 */
</code_context>
<issue_to_address>
**issue (bug_risk):** Duration 自动属性的实现无效，代码无法通过编译。
在 C# 中，自动属性的 setter 里不能使用 `field`，因此这段代码无法编译。请显式引入一个后备字段，并在 setter 中赋值 `CanonicalForm`，例如：
```csharp
private Rational _duration;
public Rational Duration
{
get => _duration;
set => _duration = value.CanonicalForm;
}
```
如果不需要在赋值时进行规范化，可以保留普通的自动属性，只在使用时做规范化处理。
</issue_to_address>
### 评论 2
<location path="parser/chu/C2sParser.cs" line_range="79" />
<code_context>
{
case "BPM":
- chart.BpmEvents.Add((Int(p, 1), Int(p, 2), Dbl(p, 3, 120.0)));
+ chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), decimal.Parse(p[3])));
break;
case "MET":
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 在不指定区域设置或校验的情况下使用 decimal.Parse，可能会错误解析 BPM 值。
在其他地方，我们使用 `TryParse` + `NumberStyles.Float` 和 `CultureInfo.InvariantCulture` 来解析数值元数据。这里的 `decimal.Parse(p[3])` 会依赖当前区域设置，并在输入不合法时抛出异常。
为了保持行为一致且健壮，建议使用：
```csharp
if (decimal.TryParse(p[3], NumberStyles.Float, CultureInfo.InvariantCulture, out var bpm))
{
chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), bpm));
}
else
{
alerts.Add(new Alert(Warning, $"BPM 格式错误: {string.Join('\t', p)}") { Line = lineNum });
}
```
你可能还需要将 `lineNum` 作为参数传入 `ParseTiming`，以便正确地填充 `Alert.Line`。
建议的实现方式：
```csharp
}
```
```csharp
private static void ParseTiming(string[] p, ChuChart chart, int lineNum, List<Alert> alerts)
```
```csharp
case "BPM":
if (decimal.TryParse(p[3], NumberStyles.Float, CultureInfo.InvariantCulture, out var bpm))
{
chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), bpm));
}
else
{
alerts.Add(new Alert(Warning, $"BPM 格式错误: {string.Join('\t', p)}") { Line = lineNum });
}
break;
```
1. 确保在 `parser/chu/C2sParser.cs` 顶部已经包含如下 `using`（如果尚未添加）：
- `using System.Globalization;`
2. 将方法签名改为 `ParseTiming(string[] p, ChuChart chart, int lineNum, List<Alert> alerts)` 之后，需要更新所有 `ParseTiming` 的调用点，传入当前行号以及共享的 `List<Alert>`（或你用于保存告警的集合类型）。
3. 如果 `Alert.Line` 不是可设置属性，或者 `Alert`/`Warning` 在不同命名空间中，请添加相应的 `using` 或使用完全限定名。
4. 为了与解析器其它部分的行为完全一致，你也可以将本文件中其它 `decimal.Parse` 调用（例如 `SFL` 分支）替换为 `TryParse` + `NumberStyles.Float` + `CultureInfo.InvariantCulture`，并在解析失败时以类似方式上报告警。
</issue_to_address>
### 评论 3
<location path="parser/chu/SusParser.cs" line_range="210-214" />
<code_context>
}
- private static void ParseAirTarget(string dataStr, ChuNote note, List<Alert> alerts, int lineNum)
+ private static void ParseAirTarget(string dataStr, ChuNote note, int tpm, List<Alert> alerts, int lineNum)
{
- if (dataStr.Length >= 8)
+ if (dataStr.Length < 8)
{
- note.TargetNote = HexToInt(dataStr[6..8]).ToString();
- }
- else
- {
- alerts.Add(new Alert(Warning, $"AIR/ADW 音符缺少目标: {dataStr}") { Line = lineNum, RelevantNote = FormatNoteRef(note) });
+ alerts.Add(new Alert(Warning, $"AIR/ADW 音符缺少目标: {dataStr}") { Line = lineNum, RelevantNote = FormatNoteRef(note, tpm) });
}
}
</code_context>
<issue_to_address>
**issue (bug_risk):** SUS AIR/ADW 的解析不再为音符设置任何目标 ID。
在之前的实现中，这个方法会解析目标 ID（十六进制 → 整数 → 字符串），并将其存到音符上（`note.TargetNote`）。新版本只检查长度并在缺失时发出告警，但从未把目标信息保存到任何地方。
由于 SUS AIR/ADW 的行为以及后续生成器依赖这个 ID，我们现在丢失了重建原始连接所需的信息。一旦你重新引入用于保存原始目标的稳定字段（例如 `ChuNote.TargetNote` 或新的 `RawTargetId`），该方法应该为其赋值，例如：
```csharp
note.RawTargetId = HexToInt(dataStr[6..8]).ToString();
```
这样生成器在输出 SUS/C2S 时就可以保留原始的目标 ID。
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，你的反馈会用来改进后续的代码审查。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- The various CHU parsers/generators now use mutable static fields for resolution/ticks (e.g.
RSLinUgcParser,UgcGenerator,C2sParser,SusParser,SusGenerator), which means parsing different charts in parallel or sequentially with different@TICKS/RESOLUTIONvalues can interfere with each other; consider making these instance fields or passing resolution explicitly to keep state per chart. - In
C2sParser.ParseTimingyou switched todecimal.Parse(p[3])/decimal.Parse(p[4])without culture or error handling, which can now throw on malformed or locale-formatted BPM/SFL values where the old code would fallback safely; usingdecimal.TryParse(..., NumberStyles.Float, CultureInfo.InvariantCulture, out var bpm)(and defaulting or warning on failure) would make the parser more robust.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The various CHU parsers/generators now use mutable static fields for resolution/ticks (e.g. `RSL` in `UgcParser`, `UgcGenerator`, `C2sParser`, `SusParser`, `SusGenerator`), which means parsing different charts in parallel or sequentially with different `@TICKS`/`RESOLUTION` values can interfere with each other; consider making these instance fields or passing resolution explicitly to keep state per chart.
- In `C2sParser.ParseTiming` you switched to `decimal.Parse(p[3])` / `decimal.Parse(p[4])` without culture or error handling, which can now throw on malformed or locale-formatted BPM/SFL values where the old code would fallback safely; using `decimal.TryParse(..., NumberStyles.Float, CultureInfo.InvariantCulture, out var bpm)` (and defaulting or warning on failure) would make the parser more robust.
## Individual Comments
### Comment 1
<location path="chart/chu/ChuNote.cs" line_range="17-18" />
<code_context>
- public int HoldDuration { get; set; }
- /** SLD 持续时长 */
- public int SlideDuration { get; set; }
+ /** HLD/SLD/AHD/ASD等的 持续时长 */
+ public Rational Duration { get; set => field = value.CanonicalForm; } = 0;
+
/** SLD 终点列 */
</code_context>
<issue_to_address>
**issue (bug_risk):** Duration auto-property implementation is invalid and won’t compile.
`field` can’t be used in an auto-property setter in C#, so this code won’t compile. Introduce an explicit backing field and assign `CanonicalForm` in the setter, e.g.
```csharp
private Rational _duration;
public Rational Duration
{
get => _duration;
set => _duration = value.CanonicalForm;
}
```
If normalization on assignment isn’t required, keep a plain auto-property and normalize only at use sites.
</issue_to_address>
### Comment 2
<location path="parser/chu/C2sParser.cs" line_range="79" />
<code_context>
{
case "BPM":
- chart.BpmEvents.Add((Int(p, 1), Int(p, 2), Dbl(p, 3, 120.0)));
+ chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), decimal.Parse(p[3])));
break;
case "MET":
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using decimal.Parse without specifying culture or validation may mis-parse BPM values.
Elsewhere we parse numeric metadata with `TryParse` + `NumberStyles.Float` and `CultureInfo.InvariantCulture`. Here, `decimal.Parse(p[3])` is culture-dependent and will throw on bad input.
To keep behavior consistent and robust, consider using:
```csharp
if (decimal.TryParse(p[3], NumberStyles.Float, CultureInfo.InvariantCulture, out var bpm))
{
chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), bpm));
}
else
{
alerts.Add(new Alert(Warning, $"BPM 格式错误: {string.Join('\t', p)}") { Line = lineNum });
}
```
You may also want to pass `lineNum` into `ParseTiming` so `Alert.Line` is populated correctly.
Suggested implementation:
```csharp
}
```
```csharp
private static void ParseTiming(string[] p, ChuChart chart, int lineNum, List<Alert> alerts)
```
```csharp
case "BPM":
if (decimal.TryParse(p[3], NumberStyles.Float, CultureInfo.InvariantCulture, out var bpm))
{
chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), bpm));
}
else
{
alerts.Add(new Alert(Warning, $"BPM 格式错误: {string.Join('\t', p)}") { Line = lineNum });
}
break;
```
1. Ensure the following `using` is present at the top of `parser/chu/C2sParser.cs` (if it is not already there):
- `using System.Globalization;`
2. The method signature change to `ParseTiming(string[] p, ChuChart chart, int lineNum, List<Alert> alerts)` requires updating all call sites of `ParseTiming` to pass the current line number and the shared `List<Alert>` (or whatever collection type you use for alerts).
3. If `Alert.Line` is not a settable property or if `Alert`/`Warning` are in a different namespace, add the appropriate `using` or fully qualify those symbols.
4. For full consistency with the rest of the parser, you may also want to replace other `decimal.Parse` calls in this file (e.g. the `SFL` case) with `TryParse` + `NumberStyles.Float` + `CultureInfo.InvariantCulture` and report alerts similarly on failure.
</issue_to_address>
### Comment 3
<location path="parser/chu/SusParser.cs" line_range="210-214" />
<code_context>
}
- private static void ParseAirTarget(string dataStr, ChuNote note, List<Alert> alerts, int lineNum)
+ private static void ParseAirTarget(string dataStr, ChuNote note, int tpm, List<Alert> alerts, int lineNum)
{
- if (dataStr.Length >= 8)
+ if (dataStr.Length < 8)
{
- note.TargetNote = HexToInt(dataStr[6..8]).ToString();
- }
- else
- {
- alerts.Add(new Alert(Warning, $"AIR/ADW 音符缺少目标: {dataStr}") { Line = lineNum, RelevantNote = FormatNoteRef(note) });
+ alerts.Add(new Alert(Warning, $"AIR/ADW 音符缺少目标: {dataStr}") { Line = lineNum, RelevantNote = FormatNoteRef(note, tpm) });
}
}
</code_context>
<issue_to_address>
**issue (bug_risk):** SUS AIR/ADW parsing no longer assigns any target ID to the note.
In the previous implementation, this method decoded the target ID (hex → int → string) and stored it on the note (`note.TargetNote`). The new version only checks length and warns when missing, but never persists the target anywhere.
Since SUS AIR/ADW behavior and downstream generators depend on this ID, we’re now losing information needed to reproduce the original linkage. Once you reintroduce a stable field for the raw target (e.g. `ChuNote.TargetNote` or a new `RawTargetId`), this method should set it, for example:
```csharp
note.RawTargetId = HexToInt(dataStr[6..8]).ToString();
```
so generators can retain the original target ID when emitting SUS/C2S.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![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 unifies the CHUNITHM chart Intermediate Representation (IR) by replacing format-specific chart classes with a single ChuChart and updating ChuNote to use Rational types for precise timing. It refactors the C2S, SUS, and UGC parsers and generators to support this unified structure and introduces a base parser for shared note-linking logic. Key feedback includes addressing potential null reference exceptions in BPM statistics, ensuring culture-invariant formatting and parsing for decimal values to prevent locale-specific bugs, and investigating a potential regression in SUS target note parsing.
| sb.AppendLine($"BPM_DEF\t{Fmt(chart.DefBpm)}\t{Fmt(chart.DefBpm)}\t{Fmt(chart.DefBpm)}\t{Fmt(chart.DefBpm)}"); | ||
| sb.AppendLine($"CREATOR\t{chart.Designer}"); | ||
| var bpm_def = chart.BpmList.BPM_DEF(); | ||
| sb.AppendLine($"BPM_DEF\t{bpm_def.Item1}\t{bpm_def.Item2}\t{bpm_def.Item3}\t{bpm_def.Item4}"); |
There was a problem hiding this comment.
使用字符串插值格式化 decimal 值时，默认会使用当前线程的区域性设置。这可能导致在某些系统上小数点被格式化为逗号（,），从而生成无效的谱面文件。为了确保输出格式的统一性，建议使用 CultureInfo.InvariantCulture。此问题同样存在于第43行和第56行。
sb.AppendLine(FormattableString.Invariant($"BPM_DEF\t{bpm_def.Item1:0.000}\t{bpm_def.Item2:0.000}\t{bpm_def.Item3:0.000}\t{bpm_def.Item4:0.000}"));| if (!string.IsNullOrEmpty(sus.Designer)) sb.AppendLine($"#DESIGNER \"{sus.Designer}\""); | ||
| sb.AppendLine($"#BPM_DEF {sus.Bpm:F2}"); | ||
| sb.AppendLine($"#REQUEST \"{sus.TicksPerBeat}\""); | ||
| sb.AppendLine($"#BPM_DEF {sus.StartBpm:F2}"); |
There was a problem hiding this comment.
使用字符串插值格式化 decimal 值时，默认会使用当前线程的区域性设置。这可能导致在某些系统上小数点被格式化为逗号（,），从而生成无效的谱面文件。为了确保输出格式的统一性，建议使用 CultureInfo.InvariantCulture。
sb.AppendLine(FormattableString.Invariant($"#BPM_DEF {sus.StartBpm:F2}"));| if (!string.IsNullOrEmpty(ugc.Designer)) sb.AppendLine($"@DESIGN\t{ugc.Designer}"); | ||
| sb.AppendLine($"@DIFF\t{ugc.Difficulty}"); | ||
| sb.AppendLine($"@LEVEL\t{ugc.DisplayLevel}"); | ||
| sb.AppendLine($"@CONST\t{ugc.Level:F5}"); |
There was a problem hiding this comment.
使用字符串插值格式化 decimal 值时，默认会使用当前线程的区域性设置。这可能导致在某些系统上小数点被格式化为逗号（,），从而生成无效的谱面文件。为了确保输出格式的统一性，建议使用 CultureInfo.InvariantCulture。此问题同样存在于第40行和第47行。
sb.AppendLine(FormattableString.Invariant($"@CONST\t{ugc.Level:F5}"));| { | ||
| case "BPM": | ||
| chart.BpmEvents.Add((Int(p, 1), Int(p, 2), Dbl(p, 3, 120.0))); | ||
| chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), decimal.Parse(p[3]))); |
There was a problem hiding this comment.
decimal.Parse(p[3]) 默认使用当前线程的区域性设置，如果系统的小数点分隔符不是 .，可能会导致解析失败或结果不正确。考虑到C2S文件格式的普遍性，建议显式指定 CultureInfo.InvariantCulture 以确保解析的稳定性。此问题同样存在于第88行。
chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), decimal.Parse(p[3], CultureInfo.InvariantCulture)));
Collaborator
Author
|
关于不同版本UGC，在 Air-Crush （ v8文档https://gist.github.com/inonote/5c01e73781cab17765a1d93641d52298 结论摘要v4 / v6 / v7 ，也就是v8之前，都是相同的；然而遗憾的是，未能找到相关文档。我尽力找了，但网上只有v8的文档。
v8相比于前代，在Air Crush方面则引入了重大变更：•
|
Collaborator
Author
|
Request for @Applesaber review |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
14 issues found across 20 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="README.md">
<violation number="1" location="README.md:218">
P3: README now documents `ChuChart`, but nearby CHUNITHM usage comments still describe `IChuChart`, creating contradictory API guidance.</violation>
</file>
<file name="parser/chu/UgcParser.cs">
<violation number="1" location="parser/chu/UgcParser.cs:15">
P1: `RSL` is a mutable static field that retains its value between `Parse()` calls. Parsing a file with a non-default `@TICKS` will corrupt timing for all subsequently parsed files that don't explicitly declare `@TICKS`. Reset it at the start of `Parse()` or make it an instance field.</violation>
<violation number="2" location="parser/chu/UgcParser.cs:361">
P1: `AirColor[colorChar.ToString()]` will throw `KeyNotFoundException` if the last character of the code is not 'N' or 'I'. Use `GetValueOrDefault` or `TryGetValue` with a fallback (as done with `ChrExtras` in `ParseTapNote`).</violation>
<violation number="3" location="parser/chu/UgcParser.cs:466">
P1: `line[sepIdx+1]` will throw `IndexOutOfRangeException` if the separator is the last character in the line. Add a bounds check before accessing.</violation>
</file>
<file name="chart/chu/ChuChart.cs">
<violation number="1" location="chart/chu/ChuChart.cs:11">
P2: Using `int` for `Difficulty` loses non-numeric difficulty labels, which contradicts the intended robust IR behavior and breaks metadata round-tripping.</violation>
</file>
<file name="parser/chu/C2sParser.cs">
<violation number="1" location="parser/chu/C2sParser.cs:16">
P2: Avoid using a static mutable resolution field; keep resolution per parse instance/call to prevent cross-chart state leakage.</violation>
<violation number="2" location="parser/chu/C2sParser.cs:79">
P2: Use culture-invariant, validated decimal parsing for BPM/SFL values to avoid locale-dependent parse failures and unhandled exceptions.</violation>
</file>
<file name="generator/chu/C2sGenerator.cs">
<violation number="1" location="generator/chu/C2sGenerator.cs:22">
P2: `MUSIC` is silently rewritten to `0` when `chart.MusicId` is not parseable as `int`, causing data loss.</violation>
<violation number="2" location="generator/chu/C2sGenerator.cs:31">
P1: Use invariant-culture formatting for BPM_DEF numeric output to avoid locale-dependent decimal separators in generated C2S files.</violation>
<violation number="3" location="generator/chu/C2sGenerator.cs:43">
P1: Decimal output is locale-dependent; C2S serialization should use invariant formatting for BPM/SFL/BPM_DEF values.</violation>
</file>
<file name="parser/chu/SusParser.cs">
<violation number="1" location="parser/chu/SusParser.cs:16">
P1: Using a mutable `static` `RSL` leaks timing resolution across parse calls, so one chart’s `REQUEST` can corrupt timing in later charts.</violation>
<violation number="2" location="parser/chu/SusParser.cs:99">
P1: Validate `REQUEST` ticks as `> 0` before assigning `RSL`; zero/negative values can break rational time calculations.
(Based on your team's feedback about rejecting zero/negative SUS tick metadata instead of silently accepting it.) [FEEDBACK_USED]</violation>
</file>
<file name="generator/chu/UgcGenerator.cs">
<violation number="1" location="generator/chu/UgcGenerator.cs:29">
P1: Write `@CONST` using invariant-culture numeric formatting to prevent locale-specific decimal separators in UGC output.</violation>
</file>
<file name="generator/chu/SusGenerator.cs">
<violation number="1" location="generator/chu/SusGenerator.cs:26">
P1: Format `#BPM_DEF` with invariant culture so decimal output is stable across locales.</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
| public class UgcParser : IParser<UgcChart> | ||
| public class UgcParser: BaseChuParser | ||
| { | ||
| private static int RSL = 480 * 4; |
There was a problem hiding this comment.
P1: RSL is a mutable static field that retains its value between Parse() calls. Parsing a file with a non-default @TICKS will corrupt timing for all subsequently parsed files that don't explicitly declare @TICKS. Reset it at the start of Parse() or make it an instance field.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At parser/chu/UgcParser.cs, line 15:
<comment>`RSL` is a mutable static field that retains its value between `Parse()` calls. Parsing a file with a non-default `@TICKS` will corrupt timing for all subsequently parsed files that don't explicitly declare `@TICKS`. Reset it at the start of `Parse()` or make it an instance field.</comment>
<file context>
@@ -2,16 +2,18 @@
-public class UgcParser : IParser<UgcChart>
+public class UgcParser: BaseChuParser
{
+ private static int RSL = 480 * 4;
+
private static readonly Dictionary<string, string> AirDirections = new()
</file context>
| foreach (var b in chart.BpmList) | ||
| { | ||
| var (m, o) = Utils.BarAndTick(b.Time, RSL); | ||
| sb.AppendLine($"BPM\t{m}\t{o}\t{b.Bpm:0.000}"); |
There was a problem hiding this comment.
P1: Decimal output is locale-dependent; C2S serialization should use invariant formatting for BPM/SFL/BPM_DEF values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At generator/chu/C2sGenerator.cs, line 43:
<comment>Decimal output is locale-dependent; C2S serialization should use invariant formatting for BPM/SFL/BPM_DEF values.</comment>
<file context>
@@ -1,125 +1,119 @@
+ foreach (var b in chart.BpmList)
+ {
+ var (m, o) = Utils.BarAndTick(b.Time, RSL);
+ sb.AppendLine($"BPM\t{m}\t{o}\t{b.Bpm:0.000}");
+ }
+
</file context>
| public class SusParser : IParser<SusChart> | ||
| public class SusParser: BaseChuParser | ||
| { | ||
| private static int RSL = 480 * 4; |
There was a problem hiding this comment.
P1: Using a mutable static RSL leaks timing resolution across parse calls, so one chart’s REQUEST can corrupt timing in later charts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At parser/chu/SusParser.cs, line 16:
<comment>Using a mutable `static` `RSL` leaks timing resolution across parse calls, so one chart’s `REQUEST` can corrupt timing in later charts.</comment>
<file context>
@@ -10,8 +11,10 @@ namespace MuConvert.chu;
-public class SusParser : IParser<SusChart>
+public class SusParser: BaseChuParser
{
+ private static int RSL = 480 * 4;
+
private static readonly Dictionary<int, string> TypeMap = new()
</file context>
| { | ||
| case "BPM": | ||
| chart.BpmEvents.Add((Int(p, 1), Int(p, 2), Dbl(p, 3, 120.0))); | ||
| chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), decimal.Parse(p[3]))); |
There was a problem hiding this comment.
P2: Use culture-invariant, validated decimal parsing for BPM/SFL values to avoid locale-dependent parse failures and unhandled exceptions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At parser/chu/C2sParser.cs, line 79:
<comment>Use culture-invariant, validated decimal parsing for BPM/SFL values to avoid locale-dependent parse failures and unhandled exceptions.</comment>
<file context>
@@ -49,44 +54,47 @@ public class C2sParser : IParser<C2sChart>
{
case "BPM":
- chart.BpmEvents.Add((Int(p, 1), Int(p, 2), Dbl(p, 3, 120.0)));
+ chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), decimal.Parse(p[3])));
break;
case "MET":
</file context>
Suggested change
| chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), decimal.Parse(p[3]))); | |
| chart.BpmList.Add(new BPM(Int(p, 1) + new Rational(Int(p, 2), RSL), p.Length > 3 && decimal.TryParse(p[3], NumberStyles.Float, CultureInfo.InvariantCulture, out var bpm) ? bpm : 120m)); |
UMIGURI 2.0 已经做了 v1.0 的迁移，所以只支持 v8 应该就行了 一、高度值C2S 字段布局确认（12 字段）示例： 高度是浮点数，精度 0.1。ASD 绝大多数为 5.0，ALD 范围 1.0~22.5。 UGC v8 高度编码参考 inonote 规范（https://gist.github.com/inonote/5c01e73781cab17765a1d93641d52298）：
|
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.
本PR是 #2 ( #2 (comment) )的后续。
@Applesaber
重构中二谱面IR类，统一为
ChuChart首先，对Chunithm转谱模块的，IR中间表示(Chart)进行了整体的大重构。具体而言：
C2sChart、UgcChart、SusChart为统一的ChuChart。Rational分数（与maimai部分保持一致）ChuNote，也根据拿到的测试用紫谱C2S和这篇文章的内容进行了改写，使ChuNote可以全面、准确、无冗余的表示音符结构。注意：所设计的
ChuNote和ChuChart，主要以C2S的格式为基准。ChuNote是一一对应的。ChuNote，并不会合并到一起。Previous属性，用于标记某一段Slide的前驱段；实现了通用工具函数BaseChuParser.FillAllPrevious，自动为每一段Slide寻找前驱段、填充Previous属性。 见 9c13621ChuNote中的属性，如Tag，优先以C2S中的记法为基准。DEF（与C2S一致），而不是与UGC一致的NCHR的Tag也是UP/CE/DW（与C2S一致），而不是与UGC一致的U/D/C基于新的IR，对C2S和UGC的Parser和Generator进行修改，（尽己所能）使其变得正确
@SPDMOD变速指令的正确支持和实现当前版本的主要缺陷（也是TODO）：
Summary by Sourcery
围绕新的 ChuChart/ChuNote 中间表示（IR）统一 CHUNITHM 谱面处理，更新所有相关解析器/生成器以使用有理数计时并修正音符语义，并相应强化往返（round-trip）测试与文档。
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Original summary in English
Summary by Sourcery
Unify CHUNITHM chart handling around a new ChuChart/ChuNote IR, update all related parsers/generators to use rational timing and correct note semantics, and bolster round-trip tests and docs accordingly.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: