重构修复谱面的逻辑,支持绿黄谱大号TouchSize#51
Conversation
…没有了行尾、注释匹配的区间就会一直延申到谱面末尾。
例子:
```
(120){1}1,1,
// 这是一个注释
2,3,4,5,6,
```
按之前的逻辑,FixChartSimaiSharp后会变成一行:
```
(120){1}1,1,// 这是一个注释2,3,4,5,6,
```
再尝试去除注释的话,就变成了
```
(120){1}1,1,
```
注释后面的notes`2,3,4,5,6`也一块全被干掉了。
增加了对于`Ch[2:1]f`或是`1h[2:1]xb`这种情况的支持(之前只支持`1h[2:1]b`)。并整合谱面修复的逻辑到一处。另有其他小优化去除冗余代码。
审阅者指南重构 Simai 谱面修复与解析逻辑,以正确处理非标准音符模式和注释,扩展对更多畸形模式和非飞星的支持,在绿/黄谱面上启用大触摸尺寸,调整谱面导入的难度(level) 处理,并将 MaiLib 固定到包含已知转换 bug 修复的版本。 更新后的 Simai 谱面解析与修复顺序图sequenceDiagram
actor User
participant MaidataImportService
participant SimaiTokenizer
participant SimaiParser
User->>MaidataImportService: TryParseChartSimaiSharp(chartText, level, errors)
activate MaidataImportService
note over MaidataImportService: 第一轮尝试:仅移除注释后解析
MaidataImportService->>MaidataImportService: SimaiCommentRegex().Replace(chartText, "")
MaidataImportService->>MaidataImportService: SimaiCommentRegex2().Replace(chartText, "")
MaidataImportService->>SimaiTokenizer: TokensFromText(chartText_without_comments)
SimaiTokenizer-->>MaidataImportService: tokens1
MaidataImportService->>SimaiParser: ChartOfToken(tokens1)
SimaiParser-->>MaidataImportService: maiLibChart1
MaidataImportService-->>User: maiLibChart1
deactivate MaidataImportService
rect rgb(235, 235, 235)
note over MaidataImportService,SimaiParser: 如果第一次解析抛出异常,则运行回退修复逻辑
end
User->>MaidataImportService: TryParseChartSimaiSharp(chartText, level, errors) fallback
activate MaidataImportService
MaidataImportService->>MaidataImportService: FixChartSimaiSharp(chartText)
note over MaidataImportService: 应用 SimaiError1-6 正则修复
MaidataImportService->>SimaiTokenizer: TokensFromText(fixed_chart)
SimaiTokenizer-->>MaidataImportService: tokens2
MaidataImportService->>SimaiParser: ChartOfToken(tokens2)
SimaiParser-->>MaidataImportService: maiLibChart2
MaidataImportService->>MaidataImportService: errors.Add(ImportChartMessage)
MaidataImportService-->>User: maiLibChart2
deactivate MaidataImportService
更新后的 MaidataImportService 及相关类型类图classDiagram
class MaidataImportService {
+ImportChartResult ImportMaidata(Dictionary~string,string~ maiData, bool isUtage, bool debug)
-MaiChart TryParseChartSimaiSharp(string chartText, int level, List~ImportChartMessage~ errors)
-string FixChartSimaiSharp(string chart)
-static string Add1Bar(string maidata, float bpm)
-static Dictionary~int,AllChartsEntry~ allCharts
-GeneratedRegex SimaiCommentRegex()
-GeneratedRegex SimaiCommentRegex2()
-GeneratedRegex SimaiError1()
-GeneratedRegex SimaiError2()
-GeneratedRegex SimaiError3()
-GeneratedRegex SimaiError4()
-GeneratedRegex SimaiError5()
-GeneratedRegex SimaiError6()
}
class AllChartsEntry {
+string originalText
+MaiChart simaiSharpChart
+AllChartsEntry(string originalText, MaiChart simaiSharpChart)
}
class MaiChart {
+List~Note~ Notes
+string Compose(ChartEnum_ChartVersion version)
}
class Note {
+NoteEnum_NoteType NoteType
+string TouchSize
}
class ImportChartResult {
+List~ImportChartMessage~ messages
+bool success
}
class ImportChartMessage {
+string message
+MessageLevel level
}
class SimaiTokenizer {
+string[] TokensFromText(string text)
}
class SimaiParser {
+MaiChart ChartOfToken(string[] tokens)
}
class ChartEnum_ChartVersion {
}
class NoteEnum_NoteType {
<<enumeration>>
TTP
THO
}
MaidataImportService --> AllChartsEntry : uses
MaidataImportService --> SimaiTokenizer : tokenizes
MaidataImportService --> SimaiParser : parses
MaidataImportService --> ImportChartResult : returns
MaiChart "1" --> "*" Note : contains
ImportChartResult "1" --> "*" ImportChartMessage : aggregates
Note --> NoteEnum_NoteType : typed
MaiChart --> ChartEnum_ChartVersion : composes with version
更新后的 MaidataImportService.ImportMaidata 流程图flowchart TD
A[Start ImportMaidata] --> B[构建 allCharts 字典
针对关卡 level i=0..8
跳过 i==1]
B --> C[对每个非空 inote_i
调用 TryParseChartSimaiSharp]
C --> D[计算谱面补白
并在需要时调用 Add1Bar]
D --> E[遍历 allCharts 中的谱面]
E --> F[判定 isUtage]
F --> G[从 targetLevelMap
查找 targetLevel]
G --> H{isUtage?}
H -- yes --> I[将 targetLevel 设为 0]
H -- no --> J[保持映射后的 targetLevel]
I --> K[计算 touchSizeBig = !isUtage 且 level 在 2,3]
J --> K
K --> L[加载对应 level 的 maiLibChart]
L --> M{touchSizeBig?}
M -- yes --> N[遍历 maiLibChart.Notes
若 NoteType 为 TTP 或 THO
则设 TouchSize = L1]
M -- no --> O[保持音符不变]
N --> P[以 Ma2_104 版本输出谱面]
O --> P
P --> Q[写入谱面文件和元数据]
Q --> R[完成所有 level]
R --> S[返回 ImportChartResult]
文件级改动
技巧与命令与 Sourcery 交互
自定义你的体验访问你的 控制面板 以:
获取帮助Original review guide in EnglishReviewer's GuideRefactors Simai chart fixing and parsing to correctly handle non-standard note patterns and comments, expands support for additional malformed patterns and non-flying stars, enables large touch size on green/yellow charts, adjusts chart import level handling, and bumps MaiLib to a fixed version for a known conversion bug. Sequence diagram for updated Simai chart parsing and fixingsequenceDiagram
actor User
participant MaidataImportService
participant SimaiTokenizer
participant SimaiParser
User->>MaidataImportService: TryParseChartSimaiSharp(chartText, level, errors)
activate MaidataImportService
note over MaidataImportService: First attempt: parse after removing comments only
MaidataImportService->>MaidataImportService: SimaiCommentRegex().Replace(chartText, "")
MaidataImportService->>MaidataImportService: SimaiCommentRegex2().Replace(chartText, "")
MaidataImportService->>SimaiTokenizer: TokensFromText(chartText_without_comments)
SimaiTokenizer-->>MaidataImportService: tokens1
MaidataImportService->>SimaiParser: ChartOfToken(tokens1)
SimaiParser-->>MaidataImportService: maiLibChart1
MaidataImportService-->>User: maiLibChart1
deactivate MaidataImportService
rect rgb(235, 235, 235)
note over MaidataImportService,SimaiParser: If first parse throws exception, run fallback fix logic
end
User->>MaidataImportService: TryParseChartSimaiSharp(chartText, level, errors) fallback
activate MaidataImportService
MaidataImportService->>MaidataImportService: FixChartSimaiSharp(chartText)
note over MaidataImportService: Applies SimaiError1-6 regex fixes
MaidataImportService->>SimaiTokenizer: TokensFromText(fixed_chart)
SimaiTokenizer-->>MaidataImportService: tokens2
MaidataImportService->>SimaiParser: ChartOfToken(tokens2)
SimaiParser-->>MaidataImportService: maiLibChart2
MaidataImportService->>MaidataImportService: errors.Add(ImportChartMessage)
MaidataImportService-->>User: maiLibChart2
deactivate MaidataImportService
Updated class diagram for MaidataImportService and related typesclassDiagram
class MaidataImportService {
+ImportChartResult ImportMaidata(Dictionary~string,string~ maiData, bool isUtage, bool debug)
-MaiChart TryParseChartSimaiSharp(string chartText, int level, List~ImportChartMessage~ errors)
-string FixChartSimaiSharp(string chart)
-static string Add1Bar(string maidata, float bpm)
-static Dictionary~int,AllChartsEntry~ allCharts
-GeneratedRegex SimaiCommentRegex()
-GeneratedRegex SimaiCommentRegex2()
-GeneratedRegex SimaiError1()
-GeneratedRegex SimaiError2()
-GeneratedRegex SimaiError3()
-GeneratedRegex SimaiError4()
-GeneratedRegex SimaiError5()
-GeneratedRegex SimaiError6()
}
class AllChartsEntry {
+string originalText
+MaiChart simaiSharpChart
+AllChartsEntry(string originalText, MaiChart simaiSharpChart)
}
class MaiChart {
+List~Note~ Notes
+string Compose(ChartEnum_ChartVersion version)
}
class Note {
+NoteEnum_NoteType NoteType
+string TouchSize
}
class ImportChartResult {
+List~ImportChartMessage~ messages
+bool success
}
class ImportChartMessage {
+string message
+MessageLevel level
}
class SimaiTokenizer {
+string[] TokensFromText(string text)
}
class SimaiParser {
+MaiChart ChartOfToken(string[] tokens)
}
class ChartEnum_ChartVersion {
}
class NoteEnum_NoteType {
<<enumeration>>
TTP
THO
}
MaidataImportService --> AllChartsEntry : uses
MaidataImportService --> SimaiTokenizer : tokenizes
MaidataImportService --> SimaiParser : parses
MaidataImportService --> ImportChartResult : returns
MaiChart "1" --> "*" Note : contains
ImportChartResult "1" --> "*" ImportChartMessage : aggregates
Note --> NoteEnum_NoteType : typed
MaiChart --> ChartEnum_ChartVersion : composes with version
Flow diagram for updated MaidataImportService.ImportMaidata logicflowchart TD
A[Start ImportMaidata] --> B[Build allCharts dictionary
for levels i=0..8
skip i==1]
B --> C[TryParseChartSimaiSharp for each
non empty inote_i]
C --> D[CalcChartPadding and
Add1Bar if needed]
D --> E[Iterate charts in allCharts]
E --> F[Determine isUtage]
F --> G[Lookup targetLevel from
targetLevelMap]
G --> H{isUtage?}
H -- yes --> I[Set targetLevel = 0]
H -- no --> J[Keep mapped targetLevel]
I --> K[Compute touchSizeBig = !isUtage and level in 2,3]
J --> K
K --> L[Load maiLibChart for level]
L --> M{touchSizeBig?}
M -- yes --> N[For each note in maiLibChart.Notes
if NoteType is TTP or THO
set TouchSize = L1]
M -- no --> O[Leave notes unchanged]
N --> P[Compose chart to Ma2_104]
O --> P
P --> Q[Write chart file and metadata]
Q --> R[Finish all levels]
R --> S[Return ImportChartResult]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 本次拉取请求主要致力于提升Simai谱面解析和修复的健壮性与准确性。通过重构核心修复逻辑,解决了多种不标准格式的兼容性问题以及因注释处理不当导致的数据丢失风险。此外,还引入了针对特定难度谱面的视觉优化,并更新了关键依赖库以修复已知的解析缺陷,从而全面提升了谱面导入和处理的质量。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并提供了一些整体性的反馈:
- 在
TryParseChartSimaiSharp中,你在第一次解析前通过去除注释来修改了chartText,因此后备调用FixChartSimaiSharp(chartText)时已经拿不到原始的原始文本;建议使用一个单独的局部变量(例如var normalizedText = ...),这样第二次尝试就能按照预期基于未修改的输入运行。 - 在对绿谱/黄谱应用
TouchSize = "L1"时,你现在会覆盖 TTP/THO 音符上已有的触碰大小;如果谱面本身可以指定触碰大小,那么只在TouchSize为 null/空时才设置为L1可能会更安全。 ImportMaidata中的循环for (var i = 0; i < 9; i++) { if (i == 1) continue; ... }略微不直观;可以考虑改为遍历一个明确列出的支持等级列表(例如new[]{0,2,3,4,5,6,7,8}),让预期范围更加清晰。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `TryParseChartSimaiSharp`, you mutate `chartText` when stripping comments before the first parse, so the fallback `FixChartSimaiSharp(chartText)` no longer sees the original raw text; consider using a separate local (e.g. `var normalizedText = ...`) so the second attempt operates on the unmodified input as intended.
- When applying `TouchSize = "L1"` for green/yellow charts, you currently overwrite any existing touch size on TTP/THO notes; if charts can specify explicit touch sizes, it might be safer to only set `L1` when `TouchSize` is null/empty.
- The loop `for (var i = 0; i < 9; i++) { if (i == 1) continue; ... }` in `ImportMaidata` is a bit non-obvious; consider iterating over an explicit list of supported levels (e.g. `new[]{0,2,3,4,5,6,7,8}`) to make the intended range clearer.
## Individual Comments
### Comment 1
<location path="MaiChartManager/Services/MaidataImportService.cs" line_range="329-331" />
<code_context>
var allCharts = new Dictionary<int, AllChartsEntry>();
- for (var i = 2; i < 9; i++)
+ for (var i = 0; i < 9; i++)
{
+ if (i == 1) continue;
if (!string.IsNullOrWhiteSpace(maiData.GetValueOrDefault($"inote_{i}")))
{
</code_context>
<issue_to_address>
**question (bug_risk):** Changing the inote iteration range changes insertion order in `allCharts`, which may affect downstream logic relying on dictionary iteration order.
Previously `allCharts` inserted `inote_2..8` first and `inote_0` last; now it inserts `0..8` (skipping `1`) in numeric order. Since .NET `Dictionary` preserves insertion order when iterating `Values`, this will change the sequence passed into `CalcChartPadding(allCharts.Values.Select(...))`. Please confirm that `CalcChartPadding` (and any callers) are order‑independent; if not, consider explicitly sorting by level at the call site or enforcing the required order when building `allCharts`.
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
TryParseChartSimaiSharp, you mutatechartTextwhen stripping comments before the first parse, so the fallbackFixChartSimaiSharp(chartText)no longer sees the original raw text; consider using a separate local (e.g.var normalizedText = ...) so the second attempt operates on the unmodified input as intended. - When applying
TouchSize = "L1"for green/yellow charts, you currently overwrite any existing touch size on TTP/THO notes; if charts can specify explicit touch sizes, it might be safer to only setL1whenTouchSizeis null/empty. - The loop
for (var i = 0; i < 9; i++) { if (i == 1) continue; ... }inImportMaidatais a bit non-obvious; consider iterating over an explicit list of supported levels (e.g.new[]{0,2,3,4,5,6,7,8}) to make the intended range clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `TryParseChartSimaiSharp`, you mutate `chartText` when stripping comments before the first parse, so the fallback `FixChartSimaiSharp(chartText)` no longer sees the original raw text; consider using a separate local (e.g. `var normalizedText = ...`) so the second attempt operates on the unmodified input as intended.
- When applying `TouchSize = "L1"` for green/yellow charts, you currently overwrite any existing touch size on TTP/THO notes; if charts can specify explicit touch sizes, it might be safer to only set `L1` when `TouchSize` is null/empty.
- The loop `for (var i = 0; i < 9; i++) { if (i == 1) continue; ... }` in `ImportMaidata` is a bit non-obvious; consider iterating over an explicit list of supported levels (e.g. `new[]{0,2,3,4,5,6,7,8}`) to make the intended range clearer.
## Individual Comments
### Comment 1
<location path="MaiChartManager/Services/MaidataImportService.cs" line_range="329-331" />
<code_context>
var allCharts = new Dictionary<int, AllChartsEntry>();
- for (var i = 2; i < 9; i++)
+ for (var i = 0; i < 9; i++)
{
+ if (i == 1) continue;
if (!string.IsNullOrWhiteSpace(maiData.GetValueOrDefault($"inote_{i}")))
{
</code_context>
<issue_to_address>
**question (bug_risk):** Changing the inote iteration range changes insertion order in `allCharts`, which may affect downstream logic relying on dictionary iteration order.
Previously `allCharts` inserted `inote_2..8` first and `inote_0` last; now it inserts `0..8` (skipping `1`) in numeric order. Since .NET `Dictionary` preserves insertion order when iterating `Values`, this will change the sequence passed into `CalcChartPadding(allCharts.Values.Select(...))`. Please confirm that `CalcChartPadding` (and any callers) are order‑independent; if not, consider explicitly sorting by level at the call site or enforcing the required order when building `allCharts`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Ch[2:1]f或是1h[2:1]xb这种情况的修复支持(之前只能修复1h[2:1]b`,其他字母未支持)。一个例子:
按之前的逻辑,FixChartSimaiSharp后会变成一行:
再尝试去除注释的话,就变成了
注释后面的notes
2,3,4,5,6也一块全被干掉了。1p2p3p4p5p6这种一串p或一串q的星星会触发。那边已经PR了,这里bump MaiLib的版本到我修复后的版本上来。Summary by Sourcery
优化 Simai 谱面导入与解析逻辑,更好地处理非标准记谱、注释以及触摸大小设置,同时将 MaiLib 更新到固定版本。
New Features:
Bug Fixes:
Enhancements:
Build:
Original summary in English
Summary by Sourcery
Refine Simai chart import and parsing to better handle non-standard notation, comments, and touch sizes, while updating MaiLib to a fixed version.
New Features:
Bug Fixes:
Enhancements:
Build: