Conversation
为之后其他类谱面接入我们的Alert类型做准备。
….mai命名空间下。 本项目正式启用基于逻辑和功能的命名空间，而不是基于文件夹的。相关文档TODO
审阅者指南重构项目以引入通用的 新 BaseChart 抽象与 Mai 谱面模型的类图classDiagram
class IBaseChart
class BaseChart_TNote {
List~TNote~ Notes
<<abstract>>
+StartBpm decimal
+StartTime decimal
+EndTime decimal
+TotalNotes int
}
IBaseChart <|.. BaseChart_TNote
class MaiChart {
+BPMList BpmList
+string DefaultTouchSize
+int ClockCount
+List~Rational~ ExplicitClocks
+decimal StartBpm
+decimal StartTime
+decimal EndTime
+int TotalNotes
+Statistics Statistics
+void Sort()
+Rational FirstNoteTime
+void Shift(Rational offset, decimal? bpm)
}
BaseChart_TNote <|-- MaiChart
class Note {
+MaiChart Chart
+Rational Time
+int Key
+Duration Duration
+virtual Rational EndTime
+string DebuggerDisplay()
}
MaiChart "1" --> "*" Note : Notes
class Tap
class Hold {
+override Duration Duration
}
class Touch {
+TouchSeries TouchSeries
}
class TouchHold {
+override Duration Duration
}
class Slide {
+int StartKey
+int EndKey
+List~SlideSegment~ segments
+Duration WaitTime
+override Rational EndTime
}
class SlideSegment
class Star
class PseudoNote
class Duration
class Statistics
Note <|-- Tap
Tap <|-- Hold
Note <|-- Touch
Touch <|-- TouchHold
Note <|-- Slide
Tap <|-- Star
Note <|-- PseudoNote
Slide --> "*" SlideSegment
Note "1" o-- "1" Duration : Duration
Slide "1" o-- "1" Duration : WaitTime
MaiChart --> Statistics : creates
Duration --> Note : owner
Statistics --> MaiChart : reads
SlideSegment --> Slide : owner
针对 MaiChart 的通用解析器与生成器类图classDiagram
class IBaseChart
class IParser_TChart {
<<interface>>
+(TChart,List~Alert~) Parse(string text)
}
class IGenerator_TChart {
<<interface>>
+(string,List~Alert~) Generate(TChart chart)
}
IParser_TChart ..> IBaseChart : where TChart
IGenerator_TChart ..> IBaseChart : where TChart
class MaiChart
class SimaiParser {
+StrictLevelEnum StrictLevel
+MaiChart chart
+List~Alert~ alerts
+(MaiChart,List~Alert~) Parse(string text)
}
class MA2Parser {
+MaiChart chart
+List~Alert~ alerts
+(MaiChart,List~Alert~) Parse(string text)
}
class SimaiGenerator {
+bool BigtouchIsAllowed
+bool UseExplicitTime
+MaiChart chart
+List~Alert~ alerts
+(string,List~Alert~) Generate(MaiChart chart)
}
class MA2Generator {
+bool IsUtage
+int MA2Version
+int RSL
+MaiChart chart
+List~Alert~ alerts
+(string,List~Alert~) Generate(MaiChart chart)
}
class MA2_103Generator
SimaiParser ..|> IParser_TChart
MA2Parser ..|> IParser_TChart
SimaiGenerator ..|> IGenerator_TChart
MA2Generator ..|> IGenerator_TChart
MA2_103Generator --|> MA2Generator
SimaiParser --> MaiChart : produces
MA2Parser --> MaiChart : produces
SimaiGenerator --> MaiChart : consumes
MA2Generator --> MaiChart : consumes
MA2_103Generator --> MaiChart : consumes
class Alert
SimaiParser --> Alert : collects
MA2Parser --> Alert : collects
SimaiGenerator --> Alert : collects
MA2Generator --> Alert : collects
MA2_103Generator --> Alert : collects
更新后的 Alert 时间元数据与本地化字符串类图classDiagram
class Alert {
+LEVEL Level
+Rational? TimeInBar
+double? TimeInSeconds
+int? Line
+string? RelevantNote
+string Description
+Alert(LEVEL level,string description)
+Alert(LEVEL level,string description,int? line,string? relevantNote)
+Alert(LEVEL level,string description,Rational? timeInBar,double? timeInSeconds,int? line,string? relevantNote)
+Alert(LEVEL level,string description,mai_MaiChart chart,Rational time,int? line,string? relevantNote)
+string ToString()
}
class LEVEL {
<<enumeration>>
Error
Warning
Info
Debug
}
class Locale {
+string MessageAt
+string MessageBar
+string MessageTime
+string MessageTimeAndBar
+string MessageParsing
+string MA2NoteSentenceTooManyParam
+string InvalidMA2Sentence
+string InvalidMA2SentenceWarning
}
Alert ..> Locale : formats_messages
class MaiChart
Alert ..> MaiChart : optional_barTime_input
Mai 谱面解析与生成流水线的流程图flowchart LR
subgraph Inputs
SimaiText[Simai text]
MA2Text[MA2 text]
end
subgraph Parsers
SP[SimaiParser]
MP[MA2Parser]
end
subgraph Model
MC[MaiChart]
end
subgraph Generators
SG[SimaiGenerator]
MG[MA2Generator]
MG103[MA2_103Generator]
end
subgraph Outputs
SimaiOut[Simai text output]
MA2Out[MA2 105 text output]
MA2103Out[MA2 103 text output]
end
Alerts[(Alert list)]
SimaiText --> SP
MA2Text --> MP
SP --> MC
MP --> MC
MC --> SG
MC --> MG
MG --> MG103
SG --> SimaiOut
MG --> MA2Out
MG103 --> MA2103Out
SP --> Alerts
MP --> Alerts
SG --> Alerts
MG --> Alerts
MG103 --> Alerts
文件级变更
提示与命令与 Sourcery 交互
自定义你的使用体验访问你的 控制面板 以：
获取帮助Original review guide in EnglishReviewer's GuideRefactors the project to introduce a generic BaseChart abstraction and a mai-specific MaiChart/Note pipeline, updates parsers and generators to be type-safe over chart types, and improves alert time metadata and testing/layout to prepare for future Chunithm chart support. Class diagram for new BaseChart abstraction and Mai chart modelclassDiagram
class IBaseChart
class BaseChart_TNote {
List~TNote~ Notes
<<abstract>>
+StartBpm decimal
+StartTime decimal
+EndTime decimal
+TotalNotes int
}
IBaseChart <|.. BaseChart_TNote
class MaiChart {
+BPMList BpmList
+string DefaultTouchSize
+int ClockCount
+List~Rational~ ExplicitClocks
+decimal StartBpm
+decimal StartTime
+decimal EndTime
+int TotalNotes
+Statistics Statistics
+void Sort()
+Rational FirstNoteTime
+void Shift(Rational offset, decimal? bpm)
}
BaseChart_TNote <|-- MaiChart
class Note {
+MaiChart Chart
+Rational Time
+int Key
+Duration Duration
+virtual Rational EndTime
+string DebuggerDisplay()
}
MaiChart "1" --> "*" Note : Notes
class Tap
class Hold {
+override Duration Duration
}
class Touch {
+TouchSeries TouchSeries
}
class TouchHold {
+override Duration Duration
}
class Slide {
+int StartKey
+int EndKey
+List~SlideSegment~ segments
+Duration WaitTime
+override Rational EndTime
}
class SlideSegment
class Star
class PseudoNote
class Duration
class Statistics
Note <|-- Tap
Tap <|-- Hold
Note <|-- Touch
Touch <|-- TouchHold
Note <|-- Slide
Tap <|-- Star
Note <|-- PseudoNote
Slide --> "*" SlideSegment
Note "1" o-- "1" Duration : Duration
Slide "1" o-- "1" Duration : WaitTime
MaiChart --> Statistics : creates
Duration --> Note : owner
Statistics --> MaiChart : reads
SlideSegment --> Slide : owner
Class diagram for generic parsers and generators over MaiChartclassDiagram
class IBaseChart
class IParser_TChart {
<<interface>>
+(TChart,List~Alert~) Parse(string text)
}
class IGenerator_TChart {
<<interface>>
+(string,List~Alert~) Generate(TChart chart)
}
IParser_TChart ..> IBaseChart : where TChart
IGenerator_TChart ..> IBaseChart : where TChart
class MaiChart
class SimaiParser {
+StrictLevelEnum StrictLevel
+MaiChart chart
+List~Alert~ alerts
+(MaiChart,List~Alert~) Parse(string text)
}
class MA2Parser {
+MaiChart chart
+List~Alert~ alerts
+(MaiChart,List~Alert~) Parse(string text)
}
class SimaiGenerator {
+bool BigtouchIsAllowed
+bool UseExplicitTime
+MaiChart chart
+List~Alert~ alerts
+(string,List~Alert~) Generate(MaiChart chart)
}
class MA2Generator {
+bool IsUtage
+int MA2Version
+int RSL
+MaiChart chart
+List~Alert~ alerts
+(string,List~Alert~) Generate(MaiChart chart)
}
class MA2_103Generator
SimaiParser ..|> IParser_TChart
MA2Parser ..|> IParser_TChart
SimaiGenerator ..|> IGenerator_TChart
MA2Generator ..|> IGenerator_TChart
MA2_103Generator --|> MA2Generator
SimaiParser --> MaiChart : produces
MA2Parser --> MaiChart : produces
SimaiGenerator --> MaiChart : consumes
MA2Generator --> MaiChart : consumes
MA2_103Generator --> MaiChart : consumes
class Alert
SimaiParser --> Alert : collects
MA2Parser --> Alert : collects
SimaiGenerator --> Alert : collects
MA2Generator --> Alert : collects
MA2_103Generator --> Alert : collects
Class diagram for updated Alert time metadata and Locale stringsclassDiagram
class Alert {
+LEVEL Level
+Rational? TimeInBar
+double? TimeInSeconds
+int? Line
+string? RelevantNote
+string Description
+Alert(LEVEL level,string description)
+Alert(LEVEL level,string description,int? line,string? relevantNote)
+Alert(LEVEL level,string description,Rational? timeInBar,double? timeInSeconds,int? line,string? relevantNote)
+Alert(LEVEL level,string description,mai_MaiChart chart,Rational time,int? line,string? relevantNote)
+string ToString()
}
class LEVEL {
<<enumeration>>
Error
Warning
Info
Debug
}
class Locale {
+string MessageAt
+string MessageBar
+string MessageTime
+string MessageTimeAndBar
+string MessageParsing
+string MA2NoteSentenceTooManyParam
+string InvalidMA2Sentence
+string InvalidMA2SentenceWarning
}
Alert ..> Locale : formats_messages
class MaiChart
Alert ..> MaiChart : optional_barTime_input
Flow diagram for Mai chart parsing and generation pipelineflowchart LR
subgraph Inputs
SimaiText[Simai text]
MA2Text[MA2 text]
end
subgraph Parsers
SP[SimaiParser]
MP[MA2Parser]
end
subgraph Model
MC[MaiChart]
end
subgraph Generators
SG[SimaiGenerator]
MG[MA2Generator]
MG103[MA2_103Generator]
end
subgraph Outputs
SimaiOut[Simai text output]
MA2Out[MA2 105 text output]
MA2103Out[MA2 103 text output]
end
Alerts[(Alert list)]
SimaiText --> SP
MA2Text --> MP
SP --> MC
MP --> MC
MC --> SG
MC --> MG
MG --> MG103
SG --> SimaiOut
MG --> MA2Out
MG103 --> MA2103Out
SP --> Alerts
MP --> Alerts
SG --> Alerts
MG --> Alerts
MG103 --> Alerts
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——我发现了两个问题，并留下了一些高层次的反馈：
Alert构造函数现在有点过载了，可能会导致调用歧义（尤其是当null同时可以匹配Rational?或int?参数时）；建议将其收敛为较小的一组构造函数，并通过静态辅助方法或 builder 风格的方法来补充，这样可以让 API 对未来的调用方更安全、更清晰。BaseChart<TNote>目前将Notes暴露为一个可变的公有字段；既然它已经成为通用抽象层，你可能会希望将其改为属性（或只暴露只读访问），以便在后续增加更多谱面类型（例如 Chunithm）时，更好地封装不变量。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- The `Alert` constructors are getting quite overloaded and may lead to ambiguous calls (especially where `null` could match either `Rational?` or `int?` arguments); consider consolidating to a smaller set of constructors plus static helpers or builder-style methods to keep the API safer and clearer for future callers.
- `BaseChart<TNote>` currently exposes `Notes` as a mutable public field; now that it’s the common abstraction, you might want to switch this to a property (or expose read-only access) to better encapsulate invariants as more chart types (e.g. Chunithm) are added.
## Individual Comments
### Comment 1
<location path="utils/Error.cs" line_range="17-23" />
<code_context>
public string Description;
- public Alert(LEVEL level, string description, (Chart, Rational)? barTime = null, int? line = null, string? relevantNote = null)
+ public Alert(LEVEL level, string description)
{
Level = level;
Description = description;
- BarTime = barTime;
+ }
+
+ public Alert(LEVEL level, string description, int? line = null, string? relevantNote = null)
+ : this(level, description)
+ {
</code_context>
<issue_to_address>
**issue (bug_risk):** The two Alert constructors are overload-ambiguous when called with just (LEVEL, string).
Because both signatures accept `(LEVEL, string)` and the extra parameters in the second ctor are optional, `new Alert(Error, "msg")` has two equally good matches and the code will not compile. To fix this, either remove the two-arg constructor, make the extra parameters non-optional (e.g. `int line` without defaults) and call that overload explicitly where needed, or move the more specific variants into a static factory method.
</issue_to_address>
### Comment 2
<location path="utils/Error.cs" line_range="37-40" />
<code_context>
+ TimeInSeconds = timeInSeconds;
+ }
+
+ public Alert(LEVEL level, string description, (mai.MaiChart, Rational) barTime, int? line = null, string? relevantNote = null)
+ : this(level, description, line, relevantNote)
+ {
+ var (chart, time) = barTime;
+ TimeInBar = time;
+ if (chart.BpmList.Count > 0) TimeInSeconds = (double)chart.ToSecond(time);
</code_context>
<issue_to_address>
**issue (bug_risk):** The `mai.MaiChart` type reference is likely invalid without a corresponding using-alias.
This constructor uses `(mai.MaiChart, Rational)` and then deconstructs it as `var (chart, time) = barTime;`, but `mai` is not defined in this file. Please either add a `using mai = MuConvert.mai;` alias or use the fully qualified `MuConvert.mai.MaiChart` type so the code compiles.
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会基于你的反馈改进后续评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The
Alertconstructors are getting quite overloaded and may lead to ambiguous calls (especially wherenullcould match eitherRational?orint?arguments); consider consolidating to a smaller set of constructors plus static helpers or builder-style methods to keep the API safer and clearer for future callers. BaseChart<TNote>currently exposesNotesas a mutable public field; now that it’s the common abstraction, you might want to switch this to a property (or expose read-only access) to better encapsulate invariants as more chart types (e.g. Chunithm) are added.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `Alert` constructors are getting quite overloaded and may lead to ambiguous calls (especially where `null` could match either `Rational?` or `int?` arguments); consider consolidating to a smaller set of constructors plus static helpers or builder-style methods to keep the API safer and clearer for future callers.
- `BaseChart<TNote>` currently exposes `Notes` as a mutable public field; now that it’s the common abstraction, you might want to switch this to a property (or expose read-only access) to better encapsulate invariants as more chart types (e.g. Chunithm) are added.
## Individual Comments
### Comment 1
<location path="utils/Error.cs" line_range="17-23" />
<code_context>
public string Description;
- public Alert(LEVEL level, string description, (Chart, Rational)? barTime = null, int? line = null, string? relevantNote = null)
+ public Alert(LEVEL level, string description)
{
Level = level;
Description = description;
- BarTime = barTime;
+ }
+
+ public Alert(LEVEL level, string description, int? line = null, string? relevantNote = null)
+ : this(level, description)
+ {
</code_context>
<issue_to_address>
**issue (bug_risk):** The two Alert constructors are overload-ambiguous when called with just (LEVEL, string).
Because both signatures accept `(LEVEL, string)` and the extra parameters in the second ctor are optional, `new Alert(Error, "msg")` has two equally good matches and the code will not compile. To fix this, either remove the two-arg constructor, make the extra parameters non-optional (e.g. `int line` without defaults) and call that overload explicitly where needed, or move the more specific variants into a static factory method.
</issue_to_address>
### Comment 2
<location path="utils/Error.cs" line_range="37-40" />
<code_context>
+ TimeInSeconds = timeInSeconds;
+ }
+
+ public Alert(LEVEL level, string description, (mai.MaiChart, Rational) barTime, int? line = null, string? relevantNote = null)
+ : this(level, description, line, relevantNote)
+ {
+ var (chart, time) = barTime;
+ TimeInBar = time;
+ if (chart.BpmList.Count > 0) TimeInSeconds = (double)chart.ToSecond(time);
</code_context>
<issue_to_address>
**issue (bug_risk):** The `mai.MaiChart` type reference is likely invalid without a corresponding using-alias.
This constructor uses `(mai.MaiChart, Rational)` and then deconstructs it as `var (chart, time) = barTime;`, but `mai` is not defined in this file. Please either add a `using mai = MuConvert.mai;` alias or use the fully qualified `MuConvert.mai.MaiChart` type so the code compiles.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+17
to
+23
| public Alert(LEVEL level, string description) | ||
| { | ||
| Level = level; | ||
| Description = description; | ||
| BarTime = barTime; | ||
| } | ||
|
|
||
| public Alert(LEVEL level, string description, int? line = null, string? relevantNote = null) |
There was a problem hiding this comment.
issue (bug_risk): 当仅以 (LEVEL, string) 调用时，这两个 Alert 构造函数存在重载歧义。
由于两个签名都接受 (LEVEL, string)，且第二个构造函数中的额外参数是可选的，所以 new Alert(Error, "msg") 会匹配到两个同样合适的重载，导致代码无法编译。为了解决这个问题，可以选择移除只带两个参数的构造函数，或者让额外参数变为必填（例如使用不带默认值的 int line），并在需要的地方显式调用该重载；或者将这些更具体的变体移到一个静态工厂方法中。
Original comment in English
issue (bug_risk): The two Alert constructors are overload-ambiguous when called with just (LEVEL, string).
Because both signatures accept (LEVEL, string) and the extra parameters in the second ctor are optional, new Alert(Error, "msg") has two equally good matches and the code will not compile. To fix this, either remove the two-arg constructor, make the extra parameters non-optional (e.g. int line without defaults) and call that overload explicitly where needed, or move the more specific variants into a static factory method.
Comment on lines
+37
to
+40
| public Alert(LEVEL level, string description, (mai.MaiChart, Rational) barTime, int? line = null, string? relevantNote = null) | ||
| : this(level, description, line, relevantNote) | ||
| { | ||
| var (chart, time) = barTime; |
There was a problem hiding this comment.
issue (bug_risk): 如果没有对应的 using 别名，mai.MaiChart 这个类型引用很可能是无效的。
这个构造函数使用 (mai.MaiChart, Rational)，并通过 var (chart, time) = barTime; 进行解构，但在这个文件中并未定义 mai。请添加 using mai = MuConvert.mai; 别名，或者直接使用完整限定名 MuConvert.mai.MaiChart，以确保代码可以正确编译。
Original comment in English
issue (bug_risk): The mai.MaiChart type reference is likely invalid without a corresponding using-alias.
This constructor uses (mai.MaiChart, Rational) and then deconstructs it as var (chart, time) = barTime;, but mai is not defined in this file. Please either add a using mai = MuConvert.mai; alias or use the fully qualified MuConvert.mai.MaiChart type so the code compiles.
![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 refactors the codebase to introduce a generic chart and parser/generator architecture, moving maimai-specific logic into a dedicated mai namespace and introducing BaseChart<TNote>. It also updates localization strings and refactors the Alert utility. Several critical issues were identified, including a syntax error in the IBaseChart declaration, potential runtime exceptions in EndTime calculations for both charts and notes, and significant performance inefficiencies in the TotalNotes property. Furthermore, the refactor introduced a circular dependency between the utils and mai namespaces, which compromises the architectural layering.
| @@ -0,0 +1,36 @@ | |||
| namespace MuConvert.chart; | |||
|
|
|||
| public interface IBaseChart; | |||
There was a problem hiding this comment.
In C#, interfaces must have a body enclosed in braces { }, even if they are empty. The semicolon-only syntax ; is not valid for interface declarations and will cause a compilation error.
public interface IBaseChart { }| } | ||
| } | ||
| public override decimal StartTime => (decimal)FirstNoteTime.Seconds; | ||
| public override decimal EndTime => (decimal)ToSecond(Notes.Select(x=>x.EndTime).Max()); |
There was a problem hiding this comment.
This property has two issues:
-
Correctness: If the
Noteslist is empty,Max()will throw anInvalidOperationException. You should handle the empty case (e.g., usingDefaultIfEmpty). -
Efficiency: This is an
$O(N)$ operation. IfEndTimeis accessed frequently, it could become a performance bottleneck. Consider caching this value or calculating it only when the chart is modified.
public override decimal EndTime => Notes.Count == 0 ? 0 : (decimal)ToSecond(Notes.Max(x => x.EndTime));| } | ||
| public override decimal StartTime => (decimal)FirstNoteTime.Seconds; | ||
| public override decimal EndTime => (decimal)ToSecond(Notes.Select(x=>x.EndTime).Max()); | ||
| public override int TotalNotes => Statistics.Total; |
There was a problem hiding this comment.
The TotalNotes property is extremely inefficient. It calls the Statistics property, which creates a new Statistics instance and iterates through all notes in the chart every time it is accessed. This makes TotalNotes an
public override int TotalNotes => Notes.Count;|
|
||
| internal virtual string DebuggerDisplay() => ""; | ||
|
|
||
| public virtual Rational EndTime => Time + Duration.Bar; |
There was a problem hiding this comment.
This will throw a NullReferenceException for note types that do not initialize the Duration property (such as Tap or Touch). Since Duration is a class and is only initialized in the Hold and TouchHold constructors, you should add a null check.
public virtual Rational EndTime => Time + (Duration?.Bar ?? 0);| /** | ||
| * 所有音符构成的列表 | ||
| */ | ||
| public List<TNote> Notes = []; |
There was a problem hiding this comment.
Exposing a List<T> as a public field is generally discouraged in C# as it breaks encapsulation. It is better to use a property with a private setter or an auto-property.
public List<TNote> Notes { get; set; } = [];
Comment on lines
+37
to
+43
| public Alert(LEVEL level, string description, (mai.MaiChart, Rational) barTime, int? line = null, string? relevantNote = null) | ||
| : this(level, description, line, relevantNote) | ||
| { | ||
| var (chart, time) = barTime; | ||
| TimeInBar = time; | ||
| if (chart.BpmList.Count > 0) TimeInSeconds = (double)chart.ToSecond(time); | ||
| } |
There was a problem hiding this comment.
This constructor introduces a circular dependency between the MuConvert.utils and MuConvert.mai namespaces, which breaks the architectural layering intended by the refactor. Additionally, it ties the general Alert class to a specific chart implementation (MaiChart). It would be better to pass the pre-calculated time in seconds or have IBaseChart provide a way to convert bar time to seconds.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 297 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed. cubic prioritises the most important files to review.
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="chart/mai/MaiChart.cs">
<violation number="1" location="chart/mai/MaiChart.cs:43">
P2: `StartTime` assumes at least one note; empty charts will throw when accessing `Notes[0]` through `FirstNoteTime`. Add an empty-check fallback.</violation>
<violation number="2" location="chart/mai/MaiChart.cs:44">
P2: `EndTime` calls `Max()` on `Notes` without guarding empty charts, which will throw at runtime.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| } | ||
| public override decimal StartTime => (decimal)FirstNoteTime.Seconds; | ||
| public override decimal EndTime => (decimal)ToSecond(Notes.Select(x=>x.EndTime).Max()); |
There was a problem hiding this comment.
P2: EndTime calls Max() on Notes without guarding empty charts, which will throw at runtime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At chart/mai/MaiChart.cs, line 44:
<comment>`EndTime` calls `Max()` on `Notes` without guarding empty charts, which will throw at runtime.</comment>
<file context>
@@ -33,13 +33,16 @@ public void Sort()
}
}
+ public override decimal StartTime => (decimal)FirstNoteTime.Seconds;
+ public override decimal EndTime => (decimal)ToSecond(Notes.Select(x=>x.EndTime).Max());
+ public override int TotalNotes => Statistics.Total;
</file context>
Suggested change
| public override decimal EndTime => (decimal)ToSecond(Notes.Select(x=>x.EndTime).Max()); | |
| public override decimal EndTime => Notes.Count == 0 ? 0m : (decimal)ToSecond(Notes.Select(x => x.EndTime).Max()); |
| return BpmList[0].Bpm; | ||
| } | ||
| } | ||
| public override decimal StartTime => (decimal)FirstNoteTime.Seconds; |
There was a problem hiding this comment.
P2: StartTime assumes at least one note; empty charts will throw when accessing Notes[0] through FirstNoteTime. Add an empty-check fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At chart/mai/MaiChart.cs, line 43:
<comment>`StartTime` assumes at least one note; empty charts will throw when accessing `Notes[0]` through `FirstNoteTime`. Add an empty-check fallback.</comment>
<file context>
@@ -33,13 +33,16 @@ public void Sort()
return BpmList[0].Bpm;
}
}
+ public override decimal StartTime => (decimal)FirstNoteTime.Seconds;
+ public override decimal EndTime => (decimal)ToSecond(Notes.Select(x=>x.EndTime).Max());
+ public override int TotalNotes => Statistics.Total;
</file context>
Suggested change
| public override decimal StartTime => (decimal)FirstNoteTime.Seconds; | |
| public override decimal StartTime => Notes.Count == 0 ? 0m : (decimal)FirstNoteTime.Seconds; |
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
1 participant
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.
Summary by Sourcery
引入通用的基础谱面抽象，并将现有所有与 Mai 谱面解析、生成以及 note/slide/statistics 相关的逻辑迁移到专用的
MuConvert.mai命名空间，同时相应更新测试和程序装配。增强内容：
BaseChart<TNote>接口和抽象基类，为未来支持多种游戏格式做准备。Chart→MaiChart，以及 note/slide/statistics/duration 类）重命名并迁移到专用的MuConvert.mai命名空间中。IParser、IGenerator）泛型化以支持不同谱面类型，使 MA2/Simai 的解析器和生成器与MaiChart对齐。文档：
测试：
tests/mai下，并添加用于定位 Mai 测试集目录的辅助方法。Original summary in English
Summary by Sourcery
Introduce a generic base chart abstraction and migrate all existing Mai chart parsing, generation, and note/slide/statistics logic to a dedicated MuConvert.mai namespace while updating tests and program wiring accordingly.
Enhancements:
Documentation:
Tests: