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

Customizable note hit sounds & effects #4112

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

Conversation

FlooferLand
Copy link
Contributor

@FlooferLand FlooferLand commented Feb 6, 2025

This PR adds note hit sound effects, as well as a little sound system for playing general note-related sounds.
It also adds a settings toggle for note splash particles, as requested by a // TODO I saw.

See FunkinCrew/funkin.assets/preload/sounds/noteSounds/how to.txt to see how to add new sounds

Video showcase

https://www.youtube.com/watch?v=K-nh51LEa7U
(or a catbox.moe mirror)


Associated assets PR: FunkinCrew/funkin.assets#36

This PR was previously #2793.
Made a new PR as I rewrote everything and the old PR contained remnants of the new settings widget system

@github-actions github-actions bot added status: pending triage Awaiting review. pr: haxe PR modifies game code. size: large A large pull request with more than 100 changes. labels Feb 6, 2025
@Hundrec Hundrec added status: needs revision Cannot be approved because it is awaiting some work by the contributor. type: enhancement Involves an enhancement or new feature. and removed status: pending triage Awaiting review. labels Feb 6, 2025
@Hundrec
Copy link
Collaborator

Hundrec commented Feb 6, 2025

Please change the base branch to develop at the top of this PR!

@amyspark-ng
Copy link
Contributor

🔥🔥🔥🔥🔥

@FlooferLand FlooferLand changed the base branch from main to develop February 7, 2025 06:26
@FlooferLand
Copy link
Contributor Author

FlooferLand commented Feb 7, 2025

I always forget about the develop branch, my bad.

Not sure how to change my fork / local repo over to the develop branch without risking accidentally nuking the entire PR again, I think I give the lead maintainers a heart attack any time they see me do anything with git lmao

Also, I solved a conflict and someone added a showNotesplash to Strumline.hx, but it doesn't seem to be used by anything?

@Hundrec Hundrec added status: pending triage Awaiting review. and removed status: needs revision Cannot be approved because it is awaiting some work by the contributor. labels Feb 7, 2025
@Lasercar
Copy link
Contributor

Lasercar commented Feb 8, 2025

I always forget about the develop branch, my bad.

Not sure how to change my fork / local repo over to the develop branch without risking accidentally nuking the entire PR again, I think I give the lead maintainers a heart attack any time they see me do anything with git lmao

Just start the branch using the develop one as a base, rather than main. Then, when you make the PR, make sure you're requesting to merge into the develop branch and you should be fine!

Also, if you click the dropdown of the conflict fix thing below when there's a conflict, you can choose to rebase it rather than merge to keep the commits clean.

If you plan to continue working on the PR/branch after that, make sure that you pull the rebased branch from your remote origin of the branch first.

@AbnormalPoof
Copy link
Collaborator

AbnormalPoof commented Feb 8, 2025

I always forget about the develop branch, my bad.

Not sure how to change my fork / local repo over to the develop branch without risking accidentally nuking the entire PR again, I think I give the lead maintainers a heart attack any time they see me do anything with git lmao

Also, I solved a conflict and someone added a showNotesplash to Strumline.hx, but it doesn't seem to be used by anything?

showNotesplash is for mods. It's so that mods can disable/enable note splashes at will. (See #4009)

@AbnormalPoof
Copy link
Collaborator

This PR appears to have merge conflicts. Please fix them!

@AbnormalPoof AbnormalPoof added status: needs revision Cannot be approved because it is awaiting some work by the contributor. and removed status: pending triage Awaiting review. labels Feb 15, 2025
Copy link

@JackXson-Real JackXson-Real left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works good! There are 2 changes that should be made, I will approve this once they are:

  1. Remove vine boom sound effect. It's a joke hitsound that does not have any place in the base game + the sound is not public domain, so it would technically be illegal for them to put it in here without permission
  2. Add a preference for toggling the strumline arrow highlight animation. (Basically make it function the same as how the opponent used to hit notes pre-0.3.0.
    This Animation
    image

It is sad to see a loved one go but it was sadly inevitable, may the vine boom rest in peace
@AbnormalPoof AbnormalPoof added status: pending triage Awaiting review. and removed status: needs revision Cannot be approved because it is awaiting some work by the contributor. labels Feb 23, 2025
@FlooferLand
Copy link
Contributor Author

Should be good now!
Sad to see the vine boom gone but I suspected it wouldn't make it in xD
More on the copyright of things if you're interested:

  • The Pool Ball sound is an edited segment from a CC0 (public domain) sample I found on FreeSound.
  • I have no idea where Ping Pong sound is but it's been circulating around and being included in several rhythm games without seemingly anyone knowing where it's originally from. I'm pretty sure its a legacy sound effect from the rhythm game Osu, but I doubt Peppy would issue a copyright strike for a decade old sound effect that is less than half a second long

Do note I made the highlight animation toggle turn off the highlight animations for both the player and the opponent's notes when its off; I could potentially make it so the player can chose whenever the highlight animation plays for them and not their opponent but ehh. This should be fine.

@JackXson-Real
Copy link

Should be good now! Sad to see the vine boom gone but I suspected it wouldn't make it in xD More on the copyright of things if you're interested:

  • The Pool Ball sound is an edited segment from a CC0 (public domain) sample I found on FreeSound.
  • I have no idea where Ping Pong sound is but it's been circulating around and being included in several rhythm games without seemingly anyone knowing where it's originally from. I'm pretty sure its a legacy sound effect from the rhythm game Osu, but I doubt Peppy would issue a copyright strike for a decade old sound effect that is less than half a second long

Do note I made the highlight animation toggle turn off the highlight animations for both the player and the opponent's notes when its off; I could potentially make it so the player can chose whenever the highlight animation plays for them and not their opponent but ehh. This should be fine.

Yeah don't worry I meant to have it done on both player and opponent so it's fine. I will test this in a bit.

Copy link

@JackXson-Real JackXson-Real left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for removing the vine boom! Also the note highlight preference looks perfect, thanks for adding it! I hope this gets merged for 0.6.0

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. type: enhancement Involves an enhancement or new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants