Conversation
|
I don't know if it's worth implementing here, but on YouTube, when you press |
| new KeyBinding(InputKey.Period, GlobalAction.StepReplayForward), | ||
| new KeyBinding(new[] { InputKey.Control, InputKey.H }, GlobalAction.ToggleReplaySettings), | ||
|
|
||
| new KeyBinding(InputKey.Number0, GlobalAction.JumpReplay0), |
There was a problem hiding this comment.
Could you elaborate? My code looks like it's already hardcoded in some sense
There was a problem hiding this comment.
Use OnKeyDown and don't make these ;bindable. No user is going to rebind these.
| /// <summary> | ||
| /// "Jump replay to 0% progress" | ||
| /// </summary> | ||
| public static LocalisableString JumpReplay0 => new TranslatableString(getKey(@"jump_replay_0"), @"Jump replay to 0% progress"); |
There was a problem hiding this comment.
what on earth? check other cases where there's a variable in a string
There was a problem hiding this comment.
Do you mean condensing the 10 into something like below?
/// <summary>
/// "Jump replay to {0}0% progress"
/// </summary>
public static LocalisableString JumpReplay(int digit) => new TranslatableString(getKey(@"jump_replay"), @"Jump replay to {0}0% progress", digit);However, in GlobalActionContainer.cs, I need a LocalisableDescription for each JumpReplayN, and I don't think I'm able to call GlobalActionKeyBindingStrings.JumpReplay(N) since the wiki says: "the LocalisableStrings to use for the attribute must be stored statically in a public field or a property".
There was a problem hiding this comment.
Good thing you won't need to use global actions after seeing my other review comment 😉
| Seek(target); | ||
| } | ||
|
|
||
| public void JumpReplayTo(int digit) |
There was a problem hiding this comment.
this makes no sense as a method (nothing else will ever call this and the parameter is ambiguous). inline this at usage point.
|
Correct me if I'm wrong, but isn't it better to just use the existing skinnable song progress component and scroll to any point you want with mouse? I know it'd be nice to also have keyboard navigation, just wondering since there was no issue/discussion linked. |
it's a problem if you don't have (and don't want to) the progress bar |
Yeah that's definitely better if you're trying to find something specific in the replay. But e.g. to restart a replay, I would prefer to press 0 instead of dragging the song progress to the left corner |
5 participants
On YouTube, we can jump around videos using the 0-9 keys. It would be cool if we can do the same for osu replays and autoplays.
A noticeable problem is the smooth-seeking. When at the beginning of a map and pressing 9 to jump near the end, every intermediate hitcircle is displayed, which may take a second or two. I believe the desired behavior would be to jump directly to the timestamp, without displaying the intermediate circles. I'm not quite sure how to approach this. I personally don't find this too annoying, so I'm opening the PR anyway.
Also, I was wondering how jumping would behave for a replay of a failed run. Should the 0-9 jump be relative to the whole song, or just the failed run? Currently it jumps relative to the whole song, since I feel this is more consistent.
This is my first contribution, let me know if there are any concerns.