search

ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15k stars 2.21k forks source link

Always inherit the volume from the previous hit object on placement #28728

Closed OliBomby closed 2 months ago

OliBomby commented 2 months ago

Before this PR the next hit object you place would only inherit volume from the previous hit object if you have the 'Auto' bank selected. If you had any other bank selected, the volume would default to 100. Now I made it always inherit volume from the previous hit object. I think this better matches the user's expectation and mimics stable behaviour.

bdach commented 2 months ago

Behaviourally, I agree that this is the correct change.

From a code quality standpoint, I refactored this slightly to reduce code duplication.

From a maintenance standpoint, if you submit multiple PRs that touch the same file, I'd ask that each individual change include test coverage to ensure no interference between them breaks them in the process. I have added tests and will continue to do so as I go through this series, but something to note for the future I guess.

OliBomby commented 2 months ago

From a maintenance standpoint, if you submit multiple PRs that touch the same file, I'd ask that each individual change include test coverage to ensure no interference between them breaks them in the process. I have added tests and will continue to do so as I go through this series, but something to note for the future I guess.

Alright I'll remind myself to add tests even for small changes