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

Fall/Jump Landing Audio #253

Merged
merged 17 commits into from
Sep 5, 2024
Merged

Conversation

niefia
Copy link
Collaborator

@niefia niefia commented Aug 22, 2024

Basic attempt at a Materials based Fall landing audio player, using the same logic as the Step audio to determine materials.

niefia added 5 commits August 22, 2024 12:39
Materials based landing audio player, uses same logic as the Step audio
Fall audio now only plays when it's intended fixing previous issues (Previously played when Jump button was pressed but Jump not trigger due to 0 Stamina)
Volume & Pitch of Falling sound is now affected the velocity of Falling player prior to hitting the ground
Updated LandingPlayer to match FootstepPlayer changes introduced by Fix FootstepSurfaceDetector raycast bug Phazorknight#248
@Phazorknight
Copy link
Owner

hey, this is pretty cool!
Default volume seems to be quite loud, so finding a way to dampen that would be good.
I appreciate you having this separate to the footstep system, but I'd probably combine it. I don't see a case where we want to have these two as separate components.

@niefia
Copy link
Collaborator Author

niefia commented Aug 30, 2024

There was a few reasons I did it that way but in retrospect it may not be necessary.

I put together a quick test using the FootstepPlayer only, and came across one major problem which is:
If the player goes from landing into a run, the step sounds will overwrite the landing sound

Can't think of a way to avoid this aside from using 2 nodes for it, but if you have any ideas feel free to suggest.
I'll work on cleaning up the code a bit in the meantime

niefia added 6 commits August 30, 2024 10:06
Refactored LandingPlayer to extend from FootstepSurfaceDetector class
Changed all landing variables to export variables in subgroup to audio
Changed the volume stat to use dB instead of linear volume to match the pre-existing Cogito variables
Created subgroup for Footstep audio to match the Landing Audio subgroup
…eneric

Refactored footstepSurfaceDetector _play_by_material function to be generic therefore allowing use of any of material_library on function call
Reworked into single AudioStreamPlayer3d node, fixed issue of sound overwrite using higher max polyphony on the node
@niefia
Copy link
Collaborator Author

niefia commented Aug 30, 2024

Ok code has been majorly cleaned up and with the last commit, it's been combined into a single AudioStreamPlayer3D node that avoids the sound overwrite problem I was having.

@Phazorknight
Copy link
Owner

I would've suggested to maybe use the included QuickAudio plug in that spawns/pools Audio players as necessary.
But will take a look at your solution in a bit.

Thanks!

@niefia
Copy link
Collaborator Author

niefia commented Aug 30, 2024

Oh a plugin wasn't even needed, I just missed a basic Godot feature. Just needed to set the AudioStreamPlayer3D to have polyphony to fix it, so set that to 8 max now instead of 1.

…allow for custom pitch/volume

Previous version still had issues with the volume and pitch of the audioplayer3d being overwritten mid sfx, so the landing sound would only partially play in the correct pitch/volume before changing to incorrect pitch/volume when footsteps started.

Fixed this by creating a QuickAudio play_sound_3d to play landing audio with custom parameters
Copy link
Owner

@Phazorknight Phazorknight left a comment

Choose a reason for hiding this comment

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

Nice job cleaning this up!
Let me know if you're still planning on working more on this PR or if it's ready to merge.

@niefia
Copy link
Collaborator Author

niefia commented Sep 3, 2024

It's ready to merge now. Functionality seems to be perfect from testing, and just put out a last commit to improve the readability of variables and comments before it gets merged.

@Phazorknight
Copy link
Owner

Amazing work, thanks so much for this! Merging.

@Phazorknight Phazorknight merged commit 505c429 into Phazorknight:main Sep 5, 2024
1 check passed
@niefia niefia deleted the landing-audio branch September 10, 2024 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants