Funkin
Funkin copied to clipboard
[ENHANCEMENT] Prevent Stacked Notes
Linked Issues
Closes https://github.com/FunkinCrew/Funkin/issues/2555
What is this?
Inspired by a comment by Hundrec and the many issues about notes stacked on top of each other, this PR intends to add a way to prevent these pesky notes.
[!IMPORTANT] This PR requires https://github.com/FunkinCrew/funkin.assets/pull/108 to be merged.
Video Demonstration
https://github.com/user-attachments/assets/ef620bb4-f1e9-4dca-b2c4-c635b4959d54
To Do / Features
- [x] Visualization of stacked/overlapping notes.
- [x] Prevent creation of stacked notes when pasting.
- [x] Make stack note threshold customizable.
- [x] A hotkey that deletes every stacked note.
- [x] Opening a chart will warn user about stacked notes present in any difficulty/variation.
Suggestions are very welcome.
Special Thanks
@lemz1 for the fast chunked listStackedNotes implementation
I can see a warning being added, maybe with a button that allows you to see all instances of them? Something like CTRL+F in a web browser:
~~Though that's probably a bit out of scope...~~
I'd personally just either have a check, which is done every time you paste, that removes all stacked notes, or if you want to a button that does it for you. I don't see the reason for showing where stacked notes are, since you are going to remove them either way, right?
I'd say giving the charter the freedom to create stacked notes is important. Having an adjustable threshold for what constitutes as "stacked" (0-100ms, for example) could go a long way to enable that freedom
since you are going to remove them either way, right?
Unless you're like making a spam/joke chart, then yeah maybe?
I'd say giving the charter the freedom to create stacked notes is important. Having an adjustable threshold for what constitutes as "stacked" (0-100ms, for example) could go a long way to enable that freedom
I thought stacked meant exactly 0ms, but i guess it is nice to have a threshold for more control
I thought of having that customization feature because I've found that Funkin's chart editor would often place notes in an imprecise manner back in the legacy versions. I haven't really trusted note placements to be millisecond-precise since then, so a slider would ensure any stacked notes not visible to the naked eye would be shown.
I've been thinking of the behaviour when stacking notes when pasting/dragging and came up with these, using the criteria that hold notes have priority over tap notes, and I want opinions on it.
When Pasting
- If a tap note is pasted on top of another note
- Tap note: We just override the placed note.
- Hold note: In this case I think we allow it to be placed and a visual indicator is necessary, or we just don't allow for the pasted note to be placed at all.
- Hold note on top of another note
- Tap note: Override placed note.
- Hold note: If they've the same length we just override the placed note too, else we do like with the tap note on top of hold note.
When dragging
- Tap note
- On top of tap note: We do like with pasting and replace the overlapped note, but only if the dragged note is deselected, that way if many notes are being dragged the already placed notes don't just get deleted because a dragged note was on top of it.
- On top of hold note: Maybe similarly to the other but we instead don't allow the dragged note to be placed instead.
- Hold note
- On top of tap note: Just like for tap note on tap note
- On top of hold note: I don't know what could be done here, leave it up to the charter's decision or perhaps keep hold note with greater length?
We might need to take note kinds into account as well.
Finally, the messy commit history is no more. ~~Thank god for rebase~~
Just wanted to say thanks for working on this much-needed feature for months, Hyper!
Alright, it should be good to merge now. I've decided to discard the idea of preventing stacked notes when moving a note on top of another because it would be a bit complex to do and unlikely to happen when charting normally, and even if it happens there's the visual indicator.
I was experimenting with being able to visualize stacked/overlapping notes in the preview and found a bunch of them at section 75 in the opponent side of 2hot Easy difficulty lol
But should this be added too?
It'd be pretty useful since the user wouldn't have to scroll through the notes slowly to check for stacked notes methinks
Oh gosh, what a great example of the utility of this feature!
You should report this to funkin.assets when you get a chance!
This is pretty great! Some notes:
- I think the main critique is that the stack notes might be confusing if you don't know what it is you're looking at. It's not intutive/it might need some tooltips. EDIT: I got some feedback from other people and they said it's obvious what it's for so I think this point might be moot? Something to think about in the future though.
- Stack note threshold should be based on note snaps rather than on milliseconds. Say, anything closer than a 128th note is a stacked note, for example.
- Is stack detection for hold notes implemented?
- It'd be cool to have a "remove all stack notes" action/keybind. It should filter to just selected notes if there is a selection to match with other actions.
- I think the main critique is that the stack notes might be confusing if you don't know what it is you're looking at. It's not intutive/it might need some tooltips. EDIT: I got some feedback from other people and they said it's obvious what it's for so I think this point might be moot? Something to think about in the future though.
I see. The idea of reusing the select box for that might've not be the best idea, but honestly I can't think of a better one atm. If anyone has suggestions I would appreciate that.
- Stack note threshold should be based on note snaps rather than on milliseconds. Say, anything closer than a 128th note is a stacked note, for example.
~~In that case should I keep it on a separate threshold field or should it follow the chart editor's note snap level?~~ I may have misunderstood this. But all good now!
- Is stack detection for hold notes implemented?
I guess you could say so. I made it work for any note since a hold note is just a tap note with a non-zero length. The only time the length is considered is when you paste a note (a longer length note overrides a shorter length note but not the other way around). The way a stacked hold note is rendered is the same as the other stacked notes though.
- It'd be cool to have a "remove all stack notes" action/keybind. It should filter to just selected notes if there is a selection to match with other actions.
Done. Shift + Delete now deletes all stacked notes, and also filters to the note selection.
Merge conflicts? True...
Merge conflicts? True...
Merged conflicts solved? Also true...
I love that commit message: Assets submodule (for easier PR testing, remove in rebase before merging, if that ever happens)
Connection terminated. I'm sorry to interrupt you, Elizabeth, if you still even remember that name, But I'm afraid you've been misinformed. You are not here to receive a gift, nor have you been called here by the individual you assume, although, you have indeed been called. You have all been called here, into a labyrinth of sounds and smells, misdirection and misfortune. A labyrinth with no exit, a maze with no prize. You don't even realize that you are trapped. Your lust for blood has driven you in endless circles, chasing the cries of children in some unseen chamber, always seeming so near, yet somehow out of reach, but you will never find them. None of you will. This is where your story ends. And to you, my brave volunteer, who somehow found this job listing not intended for you, although there was a way out planned for you, I have a feeling that's not what you want. I have a feeling that you are right where you want to be. I am remaining as well. I am nearby. This place will not be remembered, and the memory of everything that started this can finally begin to fade away. As the agony of every tragedy should. And to you monsters trapped in the corridors, be still and give up your spirits. They don't belong to you. For most of you, I believe there is peace and perhaps more waiting for you after the smoke clears. Although, for one of you, the darkest pit of Hell has opened to swallow you whole, so don't keep the devil waiting, old friend. My daughter, if you can hear me, I knew you would return as well. It's in your nature to protect the innocent. I'm sorry that on that day, the day you were shut out and left to die, no one was there to lift you up into their arms the way you lifted others into yours, and then, what became of you. I should have known you wouldn't be content to disappear, not my daughter. I couldn't save you then, so let me save you now. It's time to rest - for you, and for those you have carried in your arms. This ends for all of us. End communication.
Huge!
RIP to whoever tries to go to the chart editor on develop without this PR's assets changes
RIP to whoever tries to go to the chart editor on
developwithout this PR's asset changes
i'll get those in as well just need to figure out how to
Easy! Just gotta update the assets submod commit 😁
⛄️