-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add "ManiaModAccelerate" #21696
base: master
Are you sure you want to change the base?
Add "ManiaModAccelerate" #21696
Conversation
Co-Authored-By: Alleyne-Angel <[email protected]>
Co-Authored-By: Alleyne-Angel <[email protected]>
DK about the name, since upscroll (direction) is a thing and its the first thing this mod made me think its about |
I originally thought about speedup, but this might conflict with windup |
public BindableDouble ScoreSpeed = new BindableDouble(); | ||
|
||
private double scoreSpeed | ||
{ | ||
get | ||
{ | ||
if (ScoreSpeed.Disabled) | ||
return configTimeRange.Value; | ||
|
||
return ScoreSpeed.Value; | ||
} | ||
} |
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.
no clue why this is called "score speed" when it's a time range. also no xmldoc.
alternative implementations that would not require this member would be looked upon much more favourably
{ | ||
public class ManiaModScrollUp : Mod, IApplicableToDrawableRuleset<ManiaHitObject>, IApplicableToScoreProcessor, IApplicableToPlayer | ||
{ | ||
public override string Name => "Scroll up"; |
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.
name needs to change as it is confusing. "Accelerate" maybe
public override string Acronym => "SU"; | ||
public override LocalisableString Description => @"Key will become faster..., until you miss"; | ||
public override double ScoreMultiplier => 1; | ||
public override IconUsage? Icon => FontAwesome.Solid.AngleDoubleUp; |
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.
remove icon for now. can't find it now but it was decided in one of these that new mods should not be choosing their own icons
|
||
double speed = MinScoreSpeed.Value - (MinScoreSpeed.Value - MaxScoreSpeed.Value) * Math.Log(s.NewValue + 1, MaxComboCount.Value + 1); | ||
|
||
scoreProcessor.TransformBindableTo(scrollTime, speed, 500, Easing.OutQuint); |
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.
inefficient, this is going to likely cause tens to hundreds of transforms to be running at any given time. i'm also not even sure that this is going to work correctly with multiple overlapping notes
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.
I refer to this
osu/osu.Game/Rulesets/Mods/ModMuted.cs
Line 89 in b35e796
scoreProcessor.TransformBindableTo(metronomeVolumeAdjust, dimFactor, 500, Easing.OutQuint); |
Because this value needs use transform instead of set directly, otherwise it will cause weird visual effects, and I didn't find a better solution either.
If there is a better way please let me know
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.
see mods that implement IUpdatableByPlayfield
, i.e. magnetised
MinScoreSpeed.BindValueChanged(s => | ||
{ | ||
MaxScoreSpeed.MaxValue = s.NewValue; | ||
}); | ||
|
||
MaxScoreSpeed.BindValueChanged(s => | ||
{ | ||
MinScoreSpeed.MinValue = s.NewValue; | ||
}); |
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.
so touching either value sets the other to the same value too? very weird. compare ModWind{Up,Down}
for how this should actually be done
"Accelerate" maybe? |
I feel like this mod fits the "Fun" category better than the conversion one |
bug Co-Authored-By: Alleyne-Angel <[email protected]>
Unexpected things will happen
Now combo and scroll speed are positively correlated
Now it can test if the mod work, not just if it can load
ScrollSpeed has been used and will cause mislead
It should be within the acceptable range of normal players. Co-Authored-By: Hidden is fun <[email protected]>
if combo is lower then BaseComboCount, speed will not change.
but still, I don't think this abbreviation is appropriate
use interface to adjust scroll speed is better than add a separate member in drawable mania ruleset
I might be stupid
This is a mania mod, scroll speed will change with current combo.
If you miss, scroll speed will reduce to minspeed which can set in mod settings
- [ ] Might need a better icon, I don't think this one is good enough, but I'm out of ideasno icon for nowNow combo and scroll speed are positively correlated
2022-12-18_23-58-08.mp4