Skip to content
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

[Enhancement] Prevent Stacked Notes #3574

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

NotHyper-474
Copy link
Contributor

@NotHyper-474 NotHyper-474 commented Oct 4, 2024

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 FunkinCrew/funkin.assets#108 to be merged.

Video Demonstration

demo.mp4

To Do

  • Visualization of stacked/overlapping notes.
  • Prevent creation of stacked notes when pasting.
  • Make stack note threshold customizable.

Suggestions are very welcome.

Special Thanks

@lemz1 for the fast chunked listStackedNotes implementation

@AbnormalPoof
Copy link
Collaborator

AbnormalPoof commented Oct 4, 2024

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:
Screenshot 2024-10-04 at 11 37 22 AM
Though that's probably a bit out of scope...

@lemz1
Copy link
Contributor

lemz1 commented Oct 4, 2024

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?

@NotHyper-474 NotHyper-474 changed the title [FEATURE] Visual Indication of Stacked Notes [FEATURE] Prevent Stacked Notes Oct 4, 2024
@Hundrec
Copy link
Collaborator

Hundrec commented Oct 4, 2024

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

@AbnormalPoof
Copy link
Collaborator

AbnormalPoof commented Oct 4, 2024

since you are going to remove them either way, right?

Unless you're like making a spam/joke chart, then yeah maybe?

@lemz1
Copy link
Contributor

lemz1 commented Oct 4, 2024

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

@Hundrec
Copy link
Collaborator

Hundrec commented Oct 4, 2024

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.

@NotHyper-474 NotHyper-474 changed the title [FEATURE] Prevent Stacked Notes [Enhancement] Prevent Stacked Notes Oct 5, 2024
@NotHyper-474

This comment was marked as outdated.

@github-actions github-actions bot added size: large A large pull request with more than 100 changes. pr: haxe PR modifies game code. pr: documentation PR modifies documentation or README files. size: medium A medium pull request with 100 or fewer changes. and removed size: large A large pull request with more than 100 changes. size: medium A medium pull request with 100 or fewer changes. labels Oct 6, 2024
@EliteMasterEric EliteMasterEric added the status: pending triage Awaiting review. label Oct 8, 2024
@NotHyper-474 NotHyper-474 force-pushed the feature/stacked-notes-vis branch from 24f1dd9 to 5834279 Compare October 8, 2024 19:41
@NotHyper-474
Copy link
Contributor Author

Finally, the messy commit history is no more. Thank god for rebase

@NotHyper-474 NotHyper-474 force-pushed the feature/stacked-notes-vis branch from d2111cd to 40f3373 Compare October 8, 2024 20:01
@NotHyper-474 NotHyper-474 force-pushed the feature/stacked-notes-vis branch from b061f1d to 77e9cf2 Compare January 15, 2025 22:02
@NotHyper-474 NotHyper-474 force-pushed the feature/stacked-notes-vis branch 5 times, most recently from 2862c99 to 2be69fd Compare January 17, 2025 21:16
@Hundrec Hundrec added type: enhancement Involves an enhancement or new feature. and removed pr: documentation PR modifies documentation or README files. labels Jan 22, 2025
@AbnormalPoof AbnormalPoof added the topic: chart editor Related to the operation of the Chart Editor. label Jan 22, 2025
@NotHyper-474
Copy link
Contributor Author

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
image

But should this be added too?

@AbnormalPoof
Copy link
Collaborator

It'd be pretty useful since the user wouldn't have to scroll through the notes slowly to check for stacked notes methinks

@Hundrec
Copy link
Collaborator

Hundrec commented Jan 23, 2025

Oh gosh, what a great example of the utility of this feature!
image

You should report this to funkin.assets when you get a chance!

@NotHyper-474 NotHyper-474 force-pushed the feature/stacked-notes-vis branch 3 times, most recently from 4fc9737 to db1500b Compare January 23, 2025 22:54
@NotHyper-474 NotHyper-474 force-pushed the feature/stacked-notes-vis branch from 89cb8d5 to c98b254 Compare February 4, 2025 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: haxe PR modifies game code. size: large A large pull request with more than 100 changes. status: pending triage Awaiting review. topic: chart editor Related to the operation of the Chart Editor. type: enhancement Involves an enhancement or new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants