Skip to content

Commit

Permalink
Merge pull request #1565 from andy840119/refactor-the-lock-utils
Browse files Browse the repository at this point in the history 
Refactor the lock utils and remove the utils in the lyric editor.
  • Loading branch information
@andy840119
andy840119 authored Sep 5, 2022
2 parents 902f62c + b9258a1 commit 5e91cd7
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 74 deletions.
80 changes: 58 additions & 22 deletions osu.Game.Rulesets.Karaoke.Tests/Editor/Utils/HitObjectWritableUtilsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,50 @@ public void TestIsWriteLyricPropertyLocked()
});

void test(Lyric lyric)
=> testEveryWritablePropertyInObject<Lyric, Lyric>(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<LockState>())
{
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, Lyric>(lyric, (l, propertyName) => HitObjectWritableUtils.GetLyricPropertyLockedReason(l, propertyName));
{
HitObjectWritableUtils.IsCreateOrRemoveNoteLocked(lyric);
HitObjectWritableUtils.GetCreateOrRemoveNoteLockedReason(lyric);
}
}

#endregion
Expand All @@ -74,35 +107,38 @@ public void TestIsWriteNotePropertyLocked()
});

void test(Note note)
=> testEveryWritablePropertyInObject<Note, Note>(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, Note>(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<THitObject, TProperty>(TProperty property, Action<TProperty, string> action)
private static void testEveryWritablePropertiesInObjectAtTheSameTime<THitObject>(THitObject hitObject, Action<THitObject, string[]> 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>(THitObject hitObject, Action<THitObject, string> 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);
}
});
}
}
}
33 changes: 0 additions & 33 deletions osu.Game.Rulesets.Karaoke/Edit/Lyrics/LyricEditor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -359,35 +356,5 @@ private class LocalScrollingInfo : IScrollingInfo

public IScrollAlgorithm Algorithm { get; } = new SequentialScrollAlgorithm(new List<MultiplierControlPoint>());
}

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.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.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)
};
}
}
}
98 changes: 79 additions & 19 deletions osu.Game.Rulesets.Karaoke/Edit/Utils/HitObjectWritableUtils.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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? GetLyricPropertyLockedReasons(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<LockLyricPropertyBy>()
.ToArray();
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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));

public static LockNotePropertyBy? GetNotePropertyLockedReason(Note note, string 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<LockNotePropertyBy>()
.ToArray();

if (reasons.Contains(LockNotePropertyBy.ReferenceLyricConfig))
return LockNotePropertyBy.ReferenceLyricConfig;

return null;
}

private static LockNotePropertyBy? getNotePropertyLockedBy(Note note, string propertyName)
{
var lyric = note.ReferenceLyric;

Expand All @@ -160,18 +221,17 @@ private static bool isWriteNotePropertyLockedByReferenceLyric(Lyric lyric, strin
}

#endregion
}

[Flags]
public enum LockLyricPropertyBy
{
ReferenceLyricConfig,
private enum LockLyricPropertyBy
{
ReferenceLyricConfig,

LockState,
}
LockState,
}

public enum LockNotePropertyBy
{
ReferenceLyricConfig,
private enum LockNotePropertyBy
{
ReferenceLyricConfig,
}
}
}

0 comments on commit 5e91cd7

Please sign in to comment.