Skip to content

Commit

Permalink
Merge pull request #586 from andy840119/clean-up-code
Browse files Browse the repository at this point in the history
Clean up code
  • Loading branch information
andy840119 authored May 1, 2021
2 parents 50ce4eb + 86b48cd commit e14c9f5
Show file tree
Hide file tree
Showing 32 changed files with 82 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void TestCheckText(string text, bool hasIssue)
Assert.AreEqual(issueTemplate != null, hasIssue);
}

[TestCase(new[] { 1, 2, 3}, false)]
[TestCase(new[] { 1, 2, 3 }, false)]
[TestCase(new[] { 1 }, false)]
[TestCase(new[] { 100 }, false)] // although singer is not exist, but should not check in this test case.
[TestCase(new int[] { }, true)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ public void TestCheckParentLyric(int? lyricIndex, bool hasIssue)
var notInBeatmapLyric = new Lyric();

var note = new Note();

switch (lyricIndex)
{
case 0:
note.ParentLyric = lyric;
break;
break;

case 1:
note.ParentLyric = notInBeatmapLyric;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) andy840119 <[email protected]>. Licensed under the GPL Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Collections.Generic;
using System.Linq;
using NUnit.Framework;
Expand Down Expand Up @@ -43,7 +44,7 @@ public void TestCheckInvalidRubyTags(string text, string[] rubies, RubyTagInvali
};

var issue = run(lyric).OfType<RubyTagIssue>().FirstOrDefault();
var invalidRubyTagDictionaryKeys = issue?.InvalidRubyTags.Keys.ToArray() ?? new RubyTagInvalid[] { };
var invalidRubyTagDictionaryKeys = issue?.InvalidRubyTags.Keys.ToArray() ?? Array.Empty<RubyTagInvalid>();
Assert.AreEqual(invalidRubyTagDictionaryKeys, invalids);
}

Expand All @@ -62,7 +63,7 @@ public void TestCheckInvalidRomajiTags(string text, string[] romajies, RomajiTag
};

var issue = run(lyric).OfType<RomajiTagIssue>().FirstOrDefault();
var invalidRomajiTagDictionaryKeys = issue?.InvalidRomajiTags.Keys.ToArray() ?? new RomajiTagInvalid[] { };
var invalidRomajiTagDictionaryKeys = issue?.InvalidRomajiTags.Keys.ToArray() ?? Array.Empty<RomajiTagInvalid>();
Assert.AreEqual(invalidRomajiTagDictionaryKeys, invalids);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) andy840119 <[email protected]>. Licensed under the GPL Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Collections.Generic;
using System.Linq;
using NUnit.Framework;
Expand Down Expand Up @@ -38,7 +39,7 @@ public void TestCheckInvalidLyricTime(string lyricText, string[] timeTags, TimeI
lyric.TimeTags = TestCaseTagHelper.ParseTimeTags(timeTags);

var issue = run(lyric).OfType<LyricTimeIssue>().FirstOrDefault();
var invalidTimeTagDictionaryKeys = issue?.InvalidLyricTime ?? new TimeInvalid[] { };
var invalidTimeTagDictionaryKeys = issue?.InvalidLyricTime ?? Array.Empty<TimeInvalid>();
Assert.AreEqual(invalidTimeTagDictionaryKeys, invalid);
}

Expand All @@ -56,7 +57,7 @@ public void TestCheckInvalidTimeTags(string text, string[] timeTags, TimeTagInva
};

var issue = run(lyric).OfType<TimeTagIssue>().FirstOrDefault();
var invalidTimeTagDictionaryKeys = issue?.InvalidTimeTags.Keys.ToArray() ?? new TimeTagInvalid[] { };
var invalidTimeTagDictionaryKeys = issue?.InvalidTimeTags.Keys.ToArray() ?? Array.Empty<TimeTagInvalid>();
Assert.AreEqual(invalidTimeTagDictionaryKeys, invalids);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void TestNoLyricAndNoLanguage()
public void TestNoLyricButHaveLanguage()
{
// test no lyric and have language. (should not show alert)
var translateLanguages = new CultureInfo[] { new CultureInfo("Ja-jp") };
var translateLanguages = new[] { new CultureInfo("Ja-jp") };
var beatmap = createTestingBeatmap(translateLanguages, null);
var result = check.Run(beatmap, new TestWorkingBeatmap(beatmap));
Assert.AreEqual(result.Count(), 0);
Expand All @@ -48,7 +48,7 @@ public void TestNoLyricButHaveLanguage()
public void TestHaveLyricButNoLanguage()
{
// test have lyric and no language. (should not show alert)
var lyrics = new Lyric[] { new Lyric() };
var lyrics = new[] { new Lyric() };
var beatmap = createTestingBeatmap(null, lyrics);
var result = check.Run(beatmap, new TestWorkingBeatmap(beatmap));
Assert.AreEqual(result.Count(), 0);
Expand All @@ -58,40 +58,40 @@ public void TestHaveLyricButNoLanguage()
public void TestHaveLyricAndLanguage()
{
// no lyric with translate string. (should have issue)
var translateLanguages = new CultureInfo[] { new CultureInfo("Ja-jp") };
var beatmap = createTestingBeatmap(translateLanguages, new Lyric[]
var translateLanguages = new[] { new CultureInfo("Ja-jp") };
var beatmap = createTestingBeatmap(translateLanguages, new[]
{
createLyric(),
createLyric(),
});
Assert.AreEqual(check.Run(beatmap, new TestWorkingBeatmap(beatmap)).Count(), 1);

// no lyric with translate string. (should have issue)
var beatmap2 = createTestingBeatmap(translateLanguages, new Lyric[]
var beatmap2 = createTestingBeatmap(translateLanguages, new[]
{
createLyric(new CultureInfo("Ja-jp")),
createLyric(),
});
Assert.AreEqual(check.Run(beatmap2, new TestWorkingBeatmap(beatmap2)).Count(), 1);

// no lyric with translate string. (should have issue)
var beatmap3 = createTestingBeatmap(translateLanguages, new Lyric[]
var beatmap3 = createTestingBeatmap(translateLanguages, new[]
{
createLyric(new CultureInfo("Ja-jp")),
createLyric(new CultureInfo("Ja-jp"), ""),
});
Assert.AreEqual(check.Run(beatmap3, new TestWorkingBeatmap(beatmap3)).Count(), 1);

// some lyric with translate string. (should have issue)
var beatmap4 = createTestingBeatmap(translateLanguages, new Lyric[]
var beatmap4 = createTestingBeatmap(translateLanguages, new[]
{
createLyric(new CultureInfo("Ja-jp"), "translate1"),
createLyric(new CultureInfo("Ja-jp")),
});
Assert.AreEqual(check.Run(beatmap4, new TestWorkingBeatmap(beatmap4)).Count(), 1);

// every lyric with translate string. (should not have issue)
var beatmap5 = createTestingBeatmap(translateLanguages, new Lyric[]
var beatmap5 = createTestingBeatmap(translateLanguages, new[]
{
createLyric(new CultureInfo("Ja-jp"), "translate1"),
createLyric(new CultureInfo("Ja-jp"), "translate2"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public void TestMoveUp(string sourceName, int lyricIndex, int newLyricIndex)
TestMoveUp(lyrics, caretPosition, newCaretPosition);
}

[TestCase(nameof(singleLyric), 0, NOT_EXIST)] // cannot move down if at bottom index.
[TestCase(nameof(singleLyric), 0, NOT_EXIST)] // cannot move down if at bottom index.
[TestCase(nameof(singleLyricWithNoText), 0, NOT_EXIST)]
[TestCase(nameof(twoLyricsWithText), 0, 1)]
[TestCase(nameof(threeLyricsWithSpacing), 0, 1)]
Expand Down
3 changes: 3 additions & 0 deletions osu.Game.Rulesets.Karaoke.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,7 @@ private void load()
<s:Boolean x:Key="/Default/UserDictionary/Words/=Bindables/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Catmull/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Coronavirus/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=cuttable/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Demibold/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Drawables/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=dropdown_0027s/@EntryIndexedValue">True</s:Boolean>
Expand Down Expand Up @@ -947,6 +948,7 @@ private void load()
<s:Boolean x:Key="/Default/UserDictionary/Words/=Naka/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Nicokara/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=nkmproj/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Noto/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=pasokonn/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Playfield/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Playfields/@EntryIndexedValue">True</s:Boolean>
Expand All @@ -971,4 +973,5 @@ private void load()
<s:Boolean x:Key="/Default/UserDictionary/Words/=Taiko/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Unranked/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=upppppdate/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Venera/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=vocaloid/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Karaoke/Beatmaps/KaraokeBeatmap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public override IEnumerable<BeatmapStatistic> GetStatistics()

if (scorable)
{
int notes = HitObjects.Count(s => s is Note note && note.Display);
int notes = HitObjects.Count(s => s is Note { Display: true });
defaultStatistic.Add(new BeatmapStatistic
{
Name = @"Note",
Expand Down
4 changes: 2 additions & 2 deletions osu.Game.Rulesets.Karaoke/Bindables/BindableDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ public BindableDictionary(IDictionary<TKey, TValue> items = null)
{
if (items != null)
{
foreach (var item in items)
foreach (var (key, value) in items)
{
collection.Add(item.Key, item.Value);
collection.Add(key, value);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ private void load(EditorBeatmap beatmap, KaraokeRulesetEditCheckerConfigManager
public class LyricVerifier : IBeatmapVerifier
{
private readonly List<ICheck> checks;

public LyricVerifier(LyricCheckerConfig config)
{
checks = new List<ICheck>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ public IEnumerable<Issue> Run(IBeatmap playableBeatmap, IWorkingBeatmap workingB
{
foreach (var lyric in playableBeatmap.HitObjects.OfType<Lyric>())
{
if(lyric.Language == null)
if (lyric.Language == null)
yield return new IssueTemplateNotFillLanguage(this).Create(lyric);

// todo : check lyric layout.

if(string.IsNullOrWhiteSpace(lyric.Text))
if (string.IsNullOrWhiteSpace(lyric.Text))
yield return new IssueTemplateNoText(this).Create(lyric);

if(lyric.Singers == null || lyric.Singers.Length == 0)
if (lyric.Singers == null || lyric.Singers.Length == 0)
yield return new IssueTemplateNoSinger(this).Create(lyric);

// todo : check is singer in singer list.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,18 @@ public class CheckInvalidPropertyNotes : ICheck
public IEnumerable<Issue> Run(IBeatmap playableBeatmap, IWorkingBeatmap workingBeatmap)
{
var lyrics = playableBeatmap.HitObjects.OfType<Lyric>();

foreach (var note in playableBeatmap.HitObjects.OfType<Note>())
{
if(note.ParentLyric == null || !lyrics.Contains(note.ParentLyric))
if (note.ParentLyric == null || !lyrics.Contains(note.ParentLyric))
yield return new IssueTemplateInvalidParentLyric(this).Create(note);
}
}

public class IssueTemplateInvalidParentLyric : IssueTemplate
{
public IssueTemplateInvalidParentLyric(ICheck check)
: base(check, IssueType.Problem, "Note must have its parent lyric. If see this issue, please contact to developrt.")
: base(check, IssueType.Problem, "Note must have its parent lyric. If see this issue, please contact to ruleset developer.")
{
}

Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Karaoke/Edit/Checks/CheckTranslate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public IEnumerable<Issue> Run(IBeatmap playableBeatmap, IWorkingBeatmap workingB
{
yield return new IssueTemplateMissingTranslate(this).Create(language);
}
else if(notTranslateLyrics.Count() > 0)
else if (notTranslateLyrics.Any())
{
yield return new IssueTemplateMissingPartialTranslate(this).Create(language);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void createTimeInvalidMessage(TimeInvalid timeInvalid)
break;

case TimeInvalid.StartTimeInvalid:
invalidMessage.AddAlertParagraph("Start time is larger than minimux time tag's time.");
invalidMessage.AddAlertParagraph("Start time is larger than minimum time tag's time.");
break;

case TimeInvalid.EndTimeInvalid:
Expand Down
7 changes: 5 additions & 2 deletions osu.Game.Rulesets.Karaoke/Edit/Layout/LayoutManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,12 @@ private void load()

var skinLookups = source.GetConfig<KaraokeIndexLookup, IDictionary<int, string>>(KaraokeIndexLookup.Style)?.Value;

foreach (var skinLookup in skinLookups)
if (skinLookups == null)
return;

foreach (var (key, value) in skinLookups)
{
PreviewFontSelections.Add(skinLookup.Key, skinLookup.Value);
PreviewFontSelections.Add(key, value);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private bool moveItemToTargetPosition(Lyric newLyric, Lyric oldLyric, int skippi
if (scrollPosition < spacing)
return false;

// do not scroll if posiiton is too large and not able to move to target position.
// do not scroll if position is too large and not able to move to target position.
var itemHeight = newItem.Height + newItem.OverlayHeight;
var contentHeight = ScrollContainer.ScrollContent.Height;
var containerHeight = ScrollContainer.DrawHeight;
Expand All @@ -98,7 +98,7 @@ float getOffsetPosition(DrawableLyricEditListItem newItem, DrawableLyricEditList
if (oldItemPosition > scrollPosition)
return 0;

// if previous lyric is in front of current lyirc row, due to overlay in previous row has been removed.
// if previous lyric is in front of current lyric row, due to overlay in previous row has been removed.
// it will cause offset from previous row overlay.
return -newItem.OverlayHeight;
}
Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor_State.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public enum Mode
TypingMode,

/// <summary>
/// Aboe to create/delete/mode/split/combine note.
/// Able to create/delete/mode/split/combine note.
/// </summary>
EditNoteMode,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ private void load(OsuColour colours)
ColumnDimensions = new[]
{
new Dimension(GridSizeMode.Absolute, info_part_spacing),
new Dimension(GridSizeMode.Distributed)
new Dimension()
},
RowDimensions = new[] { new Dimension(GridSizeMode.AutoSize, minSize: min_height, maxSize: max_height) },
Content = new[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ protected override bool OnMouseMove(MouseMoveEvent e)
break;

case Mode.TimeTagEditMode:

state.MoveHoverCaretToTargetPosition(new TimeTagIndexCaretPosition(Lyric, index));
break;

Expand Down Expand Up @@ -149,10 +149,8 @@ protected override bool OnDoubleClick(DoubleClickEvent e)
lyricManager?.SplitLyric(Lyric, textCaretPosition.Index);
return true;
}
else
{
throw new NotSupportedException(nameof(position));
}

throw new NotSupportedException(nameof(position));
}

[BackgroundDependencyLoader]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using osu.Game.Rulesets.Karaoke.Edit.Blueprints.Notes;
using osu.Game.Rulesets.Karaoke.Objects;
using osu.Game.Rulesets.Karaoke.Objects.Drawables;
using osu.Game.Rulesets.Karaoke.UI;
using osu.Game.Rulesets.Karaoke.UI.Scrolling;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Drawables;
Expand Down Expand Up @@ -46,7 +45,7 @@ protected override Drawable CreateContent(Lyric lyric)

internal class EditNoteHitObjectComposer : OverlayHitObjectComposer
{
protected Lyric TargetLyric { get; private set; }
protected Lyric TargetLyric { get; }

public EditNoteHitObjectComposer(Lyric lyric)
{
Expand All @@ -58,6 +57,7 @@ private void load()
{
// add all matched notes into playfield
var notes = EditorBeatmap.HitObjects.OfType<Note>().Where(x => x.ParentLyric == TargetLyric).ToList();

foreach (var note in notes)
{
// todo : should support pooling.
Expand All @@ -79,17 +79,17 @@ protected override ComposeBlueprintContainer CreateBlueprintContainer()

public override void BeginPlacement(HitObject hitObject)
{
throw new System.NotImplementedException();
throw new NotImplementedException();
}

public override void Delete(HitObject hitObject)
{
throw new System.NotImplementedException();
throw new NotImplementedException();
}

public override void EndPlacement(HitObject hitObject, bool commit)
{
throw new System.NotImplementedException();
throw new NotImplementedException();
}

#endregion
Expand Down Expand Up @@ -120,7 +120,7 @@ internal class EditNoteSelectionHandler : KaraokeSelectionHandler
[Resolved]
private HitObjectComposer composer { get; set; }

protected override ScrollingNotePlayfield NotePlayfield => (composer as EditNoteHitObjectComposer).Playfield as ScrollingNotePlayfield;
protected override ScrollingNotePlayfield NotePlayfield => (composer as EditNoteHitObjectComposer)?.Playfield as ScrollingNotePlayfield;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public override IEnumerable<DrawableHitObject> HitObjects
/// <summary>
/// Retrieve the relevant <see cref="Playfield"/> at a specified screen-space position.
/// In cases where a ruleset doesn't require custom logic (due to nested playfields, for example)
/// this will return the ruleset's main playfield.
/// this will return the ruleset main playfield.
/// </summary>
/// <param name="screenSpacePosition">The screen-space position to query.</param>
/// <returns>The most relevant <see cref="Playfield"/>.</returns>
Expand Down
Loading

0 comments on commit e14c9f5

Please sign in to comment.