Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add change handler callbacks from timing screen / components #10330

Merged
merged 9 commits into from
Oct 5, 2020
Merged

Add change handler callbacks from timing screen / components #10330

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
436cc57
Expose ChangeHandler.SaveState via interface
peppy Oct 2, 2020
c1c5b5d
Push state change on control point group addition / removal
peppy Oct 2, 2020
98fd661
Add change handling for timing section
peppy Oct 2, 2020
693a4ff
Add change handling for effects section
peppy Oct 2, 2020
08faef6
Add change handling for difficulty section
peppy Oct 2, 2020
9fc9009
Add change handling for sample section
peppy Oct 2, 2020
519c3ac
Change SliderWithTextBoxInput to transfer on commit
peppy Oct 2, 2020
b14b160
Merge branch 'master' into editor-timing-screen-change-handling
smoogipoo Oct 2, 2020
d3e1da5
Merge branch 'master' into editor-timing-screen-change-handling
smoogipoo Oct 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions osu.Game/Screens/Edit/EditorChangeHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@ public void EndChange()
SaveState();
}

/// <summary>
/// Saves the current <see cref="Editor"/> state.
/// </summary>
public void SaveState()
{
if (bulkChangesStarted > 0)
Expand Down
6 changes: 6 additions & 0 deletions osu.Game/Screens/Edit/IEditorChangeHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,11 @@ public interface IEditorChangeHandler
/// This should be invoked as soon as possible after <see cref="BeginChange"/> to cause a state change.
/// </remarks>
void EndChange();

/// <summary>
/// Immediately saves the current <see cref="Editor"/> state.
/// Note that this will be a no-op if there is a change in progress via <see cref="BeginChange"/>.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant in the context of this PR, as the behaviour described hasn't been changed here, but I find dropping SaveState() calls entirely a bit risky, maybe? Not sure if I'm being overly paranoid but either always persisting or throwing if a change is in progress seem like better choices from an API standpoint. That said this is probably OK for now until it becomes a problem.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go down that path, we'll have to re-think how some of these operations are handled. Right now, batch operations surround smaller ones and reduce the state overhead by calling Begin/End.

/// </summary>
void SaveState();
}
}
1 change: 1 addition & 0 deletions osu.Game/Screens/Edit/Timing/DifficultySection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ protected override void OnControlPointChanged(ValueChangedEvent<DifficultyContro
if (point.NewValue != null)
{
multiplierSlider.Current = point.NewValue.SpeedMultiplierBindable;
multiplierSlider.Current.BindValueChanged(_ => ChangeHandler?.SaveState());
}
}

Expand Down
3 changes: 3 additions & 0 deletions osu.Game/Screens/Edit/Timing/EffectSection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ protected override void OnControlPointChanged(ValueChangedEvent<EffectControlPoi
if (point.NewValue != null)
{
kiai.Current = point.NewValue.KiaiModeBindable;
kiai.Current.BindValueChanged(_ => ChangeHandler?.SaveState());

omitBarLine.Current = point.NewValue.OmitFirstBarLineBindable;
omitBarLine.Current.BindValueChanged(_ => ChangeHandler?.SaveState());
}
}

Expand Down
3 changes: 3 additions & 0 deletions osu.Game/Screens/Edit/Timing/SampleSection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ protected override void OnControlPointChanged(ValueChangedEvent<SampleControlPoi
if (point.NewValue != null)
{
bank.Current = point.NewValue.SampleBankBindable;
bank.Current.BindValueChanged(_ => ChangeHandler?.SaveState());

volume.Current = point.NewValue.SampleVolumeBindable;
volume.Current.BindValueChanged(_ => ChangeHandler?.SaveState());
}
}

Expand Down
3 changes: 3 additions & 0 deletions osu.Game/Screens/Edit/Timing/Section.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ internal abstract class Section<T> : CompositeDrawable
[Resolved]
protected Bindable<ControlPointGroup> SelectedGroup { get; private set; }

[Resolved(canBeNull: true)]
protected IEditorChangeHandler ChangeHandler { get; private set; }

[BackgroundDependencyLoader]
private void load(OsuColour colours)
{
Expand Down
1 change: 1 addition & 0 deletions osu.Game/Screens/Edit/Timing/SliderWithTextBoxInput.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public SliderWithTextBoxInput(string labelText)
},
slider = new SettingsSlider<T>
{
TransferValueOnCommit = true,
RelativeSizeAxes = Axes.X,
}
}
Expand Down
4 changes: 4 additions & 0 deletions osu.Game/Screens/Edit/Timing/TimingScreen.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ public class ControlPointList : CompositeDrawable
[Resolved]
private Bindable<ControlPointGroup> selectedGroup { get; set; }

[Resolved(canBeNull: true)]
private IEditorChangeHandler changeHandler { get; set; }

[BackgroundDependencyLoader]
private void load(OsuColour colours)
{
Expand Down Expand Up @@ -146,6 +149,7 @@ protected override void LoadComplete()
controlGroups.BindCollectionChanged((sender, args) =>
{
table.ControlGroups = controlGroups;
changeHandler.SaveState();
}, true);
}

Expand Down
7 changes: 7 additions & 0 deletions osu.Game/Screens/Edit/Timing/TimingSection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,13 @@ protected override void OnControlPointChanged(ValueChangedEvent<TimingControlPoi
if (point.NewValue != null)
{
bpmSlider.Bindable = point.NewValue.BeatLengthBindable;
bpmSlider.Bindable.BindValueChanged(_ => ChangeHandler?.SaveState());

bpmTextEntry.Bindable = point.NewValue.BeatLengthBindable;
// no need to hook change handler here as it's the same bindable as above

timeSignature.Bindable = point.NewValue.TimeSignatureBindable;
timeSignature.Bindable.BindValueChanged(_ => ChangeHandler?.SaveState());
}
}

Expand Down Expand Up @@ -117,6 +122,8 @@ public BPMSlider()
bpmBindable.BindValueChanged(bpm => beatLengthBindable.Value = beatLengthToBpm(bpm.NewValue));

base.Bindable = bpmBindable;

TransferValueOnCommit = true;
}

public override Bindable<double> Bindable
Expand Down