From fa67520be14c5a841fda4240f2377db30e15d82d Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 4 Sep 2022 15:44:32 +0800 Subject: [PATCH 1/4] Remove that damn "s". --- osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor.cs | 4 ++-- .../Edit/Utils/HitObjectWritableUtils.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor.cs b/osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor.cs index 8e2310d75..1a12ff320 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor.cs @@ -378,8 +378,8 @@ private class LocalScrollingInfo : IScrollingInfo return mode switch { LyricEditorMode.View => null, - LyricEditorMode.Texting => HitObjectWritableUtils.GetLyricPropertyLockedReasons(lyric, nameof(Lyric.Text), nameof(Lyric.RubyTags), nameof(Lyric.RomajiTags), nameof(Lyric.TimeTags)), - LyricEditorMode.Reference => HitObjectWritableUtils.GetLyricPropertyLockedReasons(lyric, nameof(Lyric.ReferenceLyric), nameof(Lyric.ReferenceLyricConfig)), + LyricEditorMode.Texting => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.Text), nameof(Lyric.RubyTags), nameof(Lyric.RomajiTags), nameof(Lyric.TimeTags)), + LyricEditorMode.Reference => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.ReferenceLyric), nameof(Lyric.ReferenceLyricConfig)), LyricEditorMode.Language => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.Language)), LyricEditorMode.EditRuby => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.RubyTags)), LyricEditorMode.EditRomaji => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.RomajiTags)), diff --git a/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs b/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs index 6a714a6c1..a4b23bb0d 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs @@ -19,7 +19,7 @@ public static bool IsWriteLyricPropertyLocked(Lyric lyric, params string[] prope public static bool IsWriteLyricPropertyLocked(Lyric lyric, string propertyName) => GetLyricPropertyLockedReason(lyric, propertyName) != null; - public static LockLyricPropertyBy? GetLyricPropertyLockedReasons(Lyric lyric, params string[] propertyNames) + public static LockLyricPropertyBy? GetLyricPropertyLockedReason(Lyric lyric, params string[] propertyNames) { var reasons = propertyNames.Select(x => GetLyricPropertyLockedReason(lyric, x)) .Where(x => x != null) From 91ceb78d5c908872071d9c3a8618a67fef196e0b Mon Sep 17 00:00:00 2001 From: andy840119 Date: Mon, 5 Sep 2022 22:55:13 +0800 Subject: [PATCH 2/4] Move this shit inside the function. Because user will only care about the is lockable or not. --- .../Edit/Utils/HitObjectWritableUtils.cs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs b/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs index a4b23bb0d..ed5ec102a 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs @@ -160,18 +160,17 @@ private static bool isWriteNotePropertyLockedByReferenceLyric(Lyric lyric, strin } #endregion - } - [Flags] - public enum LockLyricPropertyBy - { - ReferenceLyricConfig, + public enum LockLyricPropertyBy + { + ReferenceLyricConfig, - LockState, - } + LockState, + } - public enum LockNotePropertyBy - { - ReferenceLyricConfig, + public enum LockNotePropertyBy + { + ReferenceLyricConfig, + } } } From dc005b225d27aaede507ff472e0b07ec49577143 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Mon, 5 Sep 2022 23:36:47 +0800 Subject: [PATCH 3/4] Should be able to get the reason directly in the utils. Also, refactor the test case. --- .../Utils/HitObjectWritableUtilsTest.cs | 80 +++++++++++++----- .../Edit/Utils/HitObjectWritableUtils.cs | 83 ++++++++++++++++--- 2 files changed, 130 insertions(+), 33 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/Utils/HitObjectWritableUtilsTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/Utils/HitObjectWritableUtilsTest.cs index bda4109b0..0eacafc4a 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/Utils/HitObjectWritableUtilsTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/Utils/HitObjectWritableUtilsTest.cs @@ -44,17 +44,50 @@ public void TestIsWriteLyricPropertyLocked() }); void test(Lyric lyric) - => testEveryWritablePropertyInObject(lyric, (l, propertyName) => HitObjectWritableUtils.IsWriteLyricPropertyLocked(l, propertyName)); + { + testEveryWritablePropertiesInObjectAtTheSameTime(lyric, (l, propertyName) => HitObjectWritableUtils.IsWriteLyricPropertyLocked(l, propertyName)); + testEveryWritablePropertiesInObject(lyric, (l, propertyName) => HitObjectWritableUtils.IsWriteLyricPropertyLocked(l, propertyName)); + + testEveryWritablePropertiesInObjectAtTheSameTime(lyric, (l, propertyName) => HitObjectWritableUtils.GetLyricPropertyLockedReason(l, propertyName)); + testEveryWritablePropertiesInObject(lyric, (l, propertyName) => HitObjectWritableUtils.GetLyricPropertyLockedReason(l, propertyName)); + } } + #endregion + + #region Create or remove notes. + [Test] - public void TestGetLyricPropertyLockedReason() + public void TestIsCreateOrRemoveNoteLocked() { // standard. test(new Lyric()); + // test lock state. + foreach (var lockState in EnumUtils.GetValues()) + { + test(new Lyric + { + Lock = lockState + }); + } + + // reference lyric. + test(new Lyric + { + ReferenceLyricConfig = new ReferenceLyricConfig(), + }); + + test(new Lyric + { + ReferenceLyricConfig = new SyncLyricConfig(), + }); + void test(Lyric lyric) - => testEveryWritablePropertyInObject(lyric, (l, propertyName) => HitObjectWritableUtils.GetLyricPropertyLockedReason(l, propertyName)); + { + HitObjectWritableUtils.IsCreateOrRemoveNoteLocked(lyric); + HitObjectWritableUtils.GetCreateOrRemoveNoteLockedReason(lyric); + } } #endregion @@ -74,35 +107,38 @@ public void TestIsWriteNotePropertyLocked() }); void test(Note note) - => testEveryWritablePropertyInObject(note, (l, propertyName) => HitObjectWritableUtils.IsWriteNotePropertyLocked(l, propertyName)); - } - - [Test] - public void TestGetNotePropertyLockedReason() - { - // standard. - test(new Note()); + { + testEveryWritablePropertiesInObjectAtTheSameTime(note, (l, propertyName) => HitObjectWritableUtils.IsWriteNotePropertyLocked(l, propertyName)); + testEveryWritablePropertiesInObject(note, (l, propertyName) => HitObjectWritableUtils.IsWriteNotePropertyLocked(l, propertyName)); - void test(Note note) - => testEveryWritablePropertyInObject(note, (l, propertyName) => HitObjectWritableUtils.GetNotePropertyLockedReason(l, propertyName)); + testEveryWritablePropertiesInObjectAtTheSameTime(note, (l, propertyName) => HitObjectWritableUtils.GetNotePropertyLockedReason(l, propertyName)); + testEveryWritablePropertiesInObject(note, (l, propertyName) => HitObjectWritableUtils.GetNotePropertyLockedReason(l, propertyName)); + } } #endregion - private void testEveryWritablePropertyInObject(TProperty property, Action action) + private static void testEveryWritablePropertiesInObjectAtTheSameTime(THitObject hitObject, Action action) { // the purpose of this test case if focus on checking every property in the hit-object should be able to know the writable or not. // return value is not in the test scope. + string[] allWriteableProperties = typeof(THitObject).GetProperties() + .Where(x => x.CanRead && x.CanWrite) + .Where(x => x.CustomAttributes.All(customAttributeData => customAttributeData.AttributeType != typeof(JsonIgnoreAttribute))) + .Select(x => x.Name) + .ToArray(); + action(hitObject, allWriteableProperties); + } - var allWriteableProperties = typeof(THitObject).GetProperties().Where(x => x.CanRead && x.CanWrite); - - foreach (var propertyInfo in allWriteableProperties) + private static void testEveryWritablePropertiesInObject(THitObject hitObject, Action action) + { + testEveryWritablePropertiesInObjectAtTheSameTime(hitObject, (l, propertyNames) => { - if (propertyInfo.CustomAttributes.Any(x => x.AttributeType == typeof(JsonIgnoreAttribute))) - continue; - - action(property, propertyInfo.Name); - } + foreach (string propertyName in propertyNames) + { + action(hitObject, propertyName); + } + }); } } } diff --git a/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs b/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs index ed5ec102a..0c25e1d62 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs @@ -3,6 +3,7 @@ using System; using System.Linq; +using osu.Framework.Localisation; using osu.Game.Rulesets.Karaoke.Objects; using osu.Game.Rulesets.Karaoke.Objects.Properties; using osu.Game.Rulesets.Karaoke.Objects.Types; @@ -14,14 +15,31 @@ public static class HitObjectWritableUtils #region Lyric property public static bool IsWriteLyricPropertyLocked(Lyric lyric, params string[] propertyNames) - => propertyNames.All(x => IsWriteLyricPropertyLocked(lyric, x)); + => getLyricPropertyLockedBy(lyric, propertyNames) != null; public static bool IsWriteLyricPropertyLocked(Lyric lyric, string propertyName) - => GetLyricPropertyLockedReason(lyric, propertyName) != null; + => getLyricPropertyLockedBy(lyric, propertyName) != null; - public static LockLyricPropertyBy? GetLyricPropertyLockedReason(Lyric lyric, params string[] propertyNames) + public static LocalisableString? GetLyricPropertyLockedReason(Lyric lyric, params string[] propertyNames) + => getLyricPropertyLockedMessage(getLyricPropertyLockedBy(lyric, propertyNames)); + + public static LocalisableString? GetLyricPropertyLockedReason(Lyric lyric, string propertyName) + => getLyricPropertyLockedMessage(getLyricPropertyLockedBy(lyric, propertyName)); + + private static LocalisableString? getLyricPropertyLockedMessage(LockLyricPropertyBy? reasons) + { + return reasons switch + { + LockLyricPropertyBy.ReferenceLyricConfig => "Cannot modify this property due to this lyric is property is sync from another lyric.", + LockLyricPropertyBy.LockState => "This property is locked and not editable", + null => default(LocalisableString?), + _ => throw new ArgumentOutOfRangeException() + }; + } + + private static LockLyricPropertyBy? getLyricPropertyLockedBy(Lyric lyric, params string[] propertyNames) { - var reasons = propertyNames.Select(x => GetLyricPropertyLockedReason(lyric, x)) + var reasons = propertyNames.Select(x => getLyricPropertyLockedBy(lyric, x)) .Where(x => x != null) .OfType() .ToArray(); @@ -35,7 +53,7 @@ public static bool IsWriteLyricPropertyLocked(Lyric lyric, string propertyName) return null; } - public static LockLyricPropertyBy? GetLyricPropertyLockedReason(Lyric lyric, string propertyName) + private static LockLyricPropertyBy? getLyricPropertyLockedBy(Lyric lyric, string propertyName) { bool lockedByConfig = isWriteLyricPropertyLockedByConfig(lyric.ReferenceLyricConfig, propertyName); if (lockedByConfig) @@ -111,7 +129,21 @@ private static bool isWriteLyricPropertyLockedByConfig(IReferenceLyricPropertyCo public static bool IsCreateOrRemoveNoteLocked(Lyric lyric) => GetCreateOrRemoveNoteLockedReason(lyric) != null; - public static LockLyricPropertyBy? GetCreateOrRemoveNoteLockedReason(Lyric lyric) + public static LocalisableString? GetCreateOrRemoveNoteLockedReason(Lyric lyric) + => getCreateOrRemoveNoteLockedMessage(getCreateOrRemoveNoteLockedBy(lyric)); + + private static LocalisableString? getCreateOrRemoveNoteLockedMessage(LockLyricPropertyBy? reasons) + { + return reasons switch + { + LockLyricPropertyBy.ReferenceLyricConfig => "Cannot modify this property due to this lyric is property is sync from another lyric.", + LockLyricPropertyBy.LockState => "This property is locked and not editable", + null => default(LocalisableString?), + _ => throw new ArgumentOutOfRangeException() + }; + } + + private static LockLyricPropertyBy? getCreateOrRemoveNoteLockedBy(Lyric lyric, params string[] propertyNames) { bool lockedByConfig = isCreateOrRemoveNoteLocked(lyric.ReferenceLyricConfig); if (lockedByConfig) @@ -137,12 +169,41 @@ private static bool isCreateOrRemoveNoteLocked(IReferenceLyricPropertyConfig? co #region Note property public static bool IsWriteNotePropertyLocked(Note note, params string[] propertyNames) - => propertyNames.All(x => IsWriteNotePropertyLocked(note, x)); + => getNotePropertyLockedBy(note, propertyNames) != null; public static bool IsWriteNotePropertyLocked(Note note, string propertyName) - => GetNotePropertyLockedReason(note, propertyName) != null; + => getNotePropertyLockedBy(note, propertyName) != null; + + public static LocalisableString? GetNotePropertyLockedReason(Note note, params string[] propertyNames) + => getNotePropertyLockedMessage(getNotePropertyLockedBy(note, propertyNames)); + + public static LocalisableString? GetNotePropertyLockedReason(Note note, string propertyName) + => getNotePropertyLockedMessage(getNotePropertyLockedBy(note, propertyName)); + + private static LocalisableString? getNotePropertyLockedMessage(LockNotePropertyBy? reasons) + { + return reasons switch + { + LockNotePropertyBy.ReferenceLyricConfig => "Cannot modify this property due to this note's lyric is property is sync from another lyric.", + null => default(LocalisableString?), + _ => throw new ArgumentOutOfRangeException() + }; + } + + private static LockNotePropertyBy? getNotePropertyLockedBy(Note note, params string[] propertyNames) + { + var reasons = propertyNames.Select(x => getNotePropertyLockedBy(note, x)) + .Where(x => x != null) + .OfType() + .ToArray(); + + if (reasons.Contains(LockNotePropertyBy.ReferenceLyricConfig)) + return LockNotePropertyBy.ReferenceLyricConfig; + + return null; + } - public static LockNotePropertyBy? GetNotePropertyLockedReason(Note note, string propertyName) + private static LockNotePropertyBy? getNotePropertyLockedBy(Note note, string propertyName) { var lyric = note.ReferenceLyric; @@ -161,14 +222,14 @@ private static bool isWriteNotePropertyLockedByReferenceLyric(Lyric lyric, strin #endregion - public enum LockLyricPropertyBy + private enum LockLyricPropertyBy { ReferenceLyricConfig, LockState, } - public enum LockNotePropertyBy + private enum LockNotePropertyBy { ReferenceLyricConfig, } From b9258a10030c68f3748b9715da9b796308bf883e Mon Sep 17 00:00:00 2001 From: andy840119 Date: Mon, 5 Sep 2022 23:37:50 +0800 Subject: [PATCH 4/4] Remove this shit from the lyric editor. Because one lyric edit mode does not only contains only one check type. --- .../Edit/Lyrics/LyricEditor.cs | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor.cs b/osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor.cs index 1a12ff320..d7681fa2d 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor.cs @@ -13,7 +13,6 @@ using osu.Framework.Input; using osu.Framework.Input.Bindings; using osu.Framework.Input.Events; -using osu.Framework.Localisation; using osu.Game.Rulesets.Karaoke.Configuration; using osu.Game.Rulesets.Karaoke.Edit.ChangeHandlers.Lyrics; using osu.Game.Rulesets.Karaoke.Edit.Lyrics.Extends; @@ -26,9 +25,7 @@ using osu.Game.Rulesets.Karaoke.Edit.Lyrics.Extends.TimeTags; using osu.Game.Rulesets.Karaoke.Edit.Lyrics.States; using osu.Game.Rulesets.Karaoke.Edit.Lyrics.States.Modes; -using osu.Game.Rulesets.Karaoke.Edit.Utils; using osu.Game.Rulesets.Karaoke.Extensions; -using osu.Game.Rulesets.Karaoke.Objects; using osu.Game.Rulesets.Timing; using osu.Game.Rulesets.UI; using osu.Game.Rulesets.UI.Scrolling; @@ -359,35 +356,5 @@ private class LocalScrollingInfo : IScrollingInfo public IScrollAlgorithm Algorithm { get; } = new SequentialScrollAlgorithm(new List()); } - - public static LocalisableString? GetLyricPropertyLockedReason(Lyric lyric, LyricEditorMode mode) - { - var reasons = getLyricPropertyLockedReasons(lyric, mode); - - return reasons switch - { - LockLyricPropertyBy.ReferenceLyricConfig => "Cannot modify this property due to this lyric is property is sync from another lyric.", - LockLyricPropertyBy.LockState => "This property is locked and not editable", - null => null, - _ => throw new ArgumentOutOfRangeException() - }; - } - - private static LockLyricPropertyBy? getLyricPropertyLockedReasons(Lyric lyric, LyricEditorMode mode) - { - return mode switch - { - LyricEditorMode.View => null, - LyricEditorMode.Texting => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.Text), nameof(Lyric.RubyTags), nameof(Lyric.RomajiTags), nameof(Lyric.TimeTags)), - LyricEditorMode.Reference => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.ReferenceLyric), nameof(Lyric.ReferenceLyricConfig)), - LyricEditorMode.Language => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.Language)), - LyricEditorMode.EditRuby => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.RubyTags)), - LyricEditorMode.EditRomaji => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.RomajiTags)), - LyricEditorMode.EditTimeTag => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.TimeTags)), - LyricEditorMode.EditNote => HitObjectWritableUtils.GetCreateOrRemoveNoteLockedReason(lyric), - LyricEditorMode.Singer => HitObjectWritableUtils.GetLyricPropertyLockedReason(lyric, nameof(Lyric.Singers)), - _ => throw new ArgumentOutOfRangeException(nameof(mode), mode, null) - }; - } } }