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

Add: Typing Sound Audio Bus in Character Settings #2425

Conversation

anis-mahsoume
Copy link

@anis-mahsoume anis-mahsoume commented Oct 3, 2024

Add a new field "Audio Bus" into the character settings typing sounds section:
image
The Audio Bus is an OptionButton field which values are populated from AudioServer buses.
I decided to make the change after opening the issue:
#2420

Copy link
Collaborator

@CakeVR CakeVR left a comment

Choose a reason for hiding this comment

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

Just some minor improvements, you can simply apply all of them.

Thanks for your Pull Request.

@anis-mahsoume anis-mahsoume requested a review from CakeVR October 10, 2024 14:56
@anis-mahsoume
Copy link
Author

Just some minor improvements, you can simply apply all of them.

Thanks for your Pull Request.

Thanks @CakeVR for your review: I have applied the changes you suggested.

@CakeVR
Copy link
Collaborator

CakeVR commented Oct 11, 2024

The problem I have with this solution is that you use _on_started_revealing_text to set the Audio Bus which gets triggered every single text message.

I don't think this is a good solution.

@Jowan-Spooner
Copy link
Collaborator

I don't think this should be a setting in the character editor, rather a global setting or a style setting. I cannot see a use-case where you'd need to change this per type sound mood.

@CakeVR
Copy link
Collaborator

CakeVR commented Nov 11, 2024

Hello, is there any interest in continuing on this PR?
As I agree with Jowan, there would have to be Global Setting in the Settings tab instead.

@anis-mahsoume
Copy link
Author

Hi,
This issue has been fixed in
#2456
So closing this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants