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

Added check for a valid AudioStreamPlayer3D node. #506

Merged
merged 1 commit into from
Aug 18, 2023

Conversation

jkankiewicz
Copy link
Contributor

This PR fixes issue #505:

  • Added check for a valid AudioStreamPlayer3D node.

@Malcolmnixon
Copy link
Collaborator

The original code appears to be written assuming users will:

  • Create their own snap-zone scene by extending from snap_zone.tscn, or enabling "Editable Children"
  • Edit the AudioStreamPlayer3D to configure the (numerous) sound/bus/volume settings

The original code isn't so friendly to instantiation via "Add New Node" as it requires the user create the CollisionShape3D and the AudioStreamPlayer3D. This PR relaxes this slightly by allowing the user to omit creating the AudioStreamPlayer3D if they don't want sounds.

After we get this PR in, I think we should continue to improve the "Add New Node" behavior by:

  • Allowing the user to provide their own CollisionShape3D
  • If the user doesn't provide their own CollisionShape3D then programmatically creating a sphere CollisionShape3D and exposing the Grab Distance property
  • Allowing the user to provide their own AudioStreamPlayer3D
  • Hiding the "Stash Sound" if the user doesn't provide their own AudioStreamPlayer3D

Note: It's simple for us to create a sphere CollisionShape3D programmatically; but we don't want to create an AudioStreamPlayer3D as there are so many audio properties - it's far easier to let the user manipulate via having them put in an AudioStreamPlayer3D node.

Copy link
Collaborator

@Malcolmnixon Malcolmnixon left a comment

Choose a reason for hiding this comment

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

This looks good

Copy link
Member

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

lgtm

@BastiaanOlij BastiaanOlij added the bug Something isn't working label Aug 18, 2023
@BastiaanOlij BastiaanOlij added this to the 4.2.0 milestone Aug 18, 2023
@BastiaanOlij BastiaanOlij merged commit 5bb35f3 into GodotVR:master Aug 18, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants