-
Notifications
You must be signed in to change notification settings - Fork 78
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.
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
fix duplicate difficulty & performance calculation when changing mods #222
Conversation
You should probably check that the list of mods is equal or fix the original bug instead of just checking the count, as substituting one mod doesnt change the count, and while I cant confirm that this is doable in the UI atm this doesnt quite feel right |
It is not doable, and I believe it is fine to me the way it's checked now. As I mentioned, I'm not aware of Lazer enough to evaluate whether this even is a bug, so I decided to go with a hotfix. |
@@ -511,6 +511,11 @@ protected override void Dispose(bool isDisposing) | |||
|
|||
private void modsChanged(ValueChangedEvent<IReadOnlyList<Mod>> mods) | |||
{ | |||
// Hotfix for preventing a difficulty and performance calculation from being trigger twice, | |||
// as the mod overlay for some reason triggers a ValueChanged twice per mod change. | |||
if (mods.OldValue.Count == mods.NewValue.Count) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't that disable recalculation when you're change mod settings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mod settings are unaffected, they are being tracked by a separate piece of code.
osu-tools/PerformanceCalculatorGUI/Screens/SimulateScreen.cs
Lines 519 to 528 in e387265
modSettingChangeTracker = new ModSettingChangeTracker(mods.NewValue); | |
modSettingChangeTracker.SettingChanged += m => | |
{ | |
debouncedStatisticsUpdate?.Cancel(); | |
debouncedStatisticsUpdate = Scheduler.AddDelayed(() => | |
{ | |
calculateDifficulty(); | |
calculatePerformance(); | |
}, 100); | |
}; |
When changing the mods on the simulation screen in perfcalc GUI, it for some reason triggers the value change event on the bindable twice. As far as I can tell, this behaviour is caused by the control itself in osu.Game, and not by osu-tools.
As I'm not sure about the circumstances under which this behaviour might be desired inside Lazer, I decided to hotfix it by checking the difference between the old and new value, which is the exact same on the second time the bindable's value changed is called.
Furthermore, there is another issue with performance calculation being triggered twice when entering a beatmap ID and calculating it for the first time, since the
updateCombo
method changes the value of the combo textbox, which's bindable is linked to triggering performance calculation. But I think this isn't too critical, as here only performance is recalculated and that's a very quick process, unlike the fix this PR does as this causes difficulty calculation to occur an unecessary extra time, and depending on the map that can take abit of time.osu-tools/PerformanceCalculatorGUI/Screens/SimulateScreen.cs
Line 870 in e387265