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

Initial implementation of voice file output. #203

Closed
wants to merge 1 commit into from

Conversation

Cidan
Copy link
Contributor

@Cidan Cidan commented Apr 7, 2024

Fixes #202

@Cidan
Copy link
Contributor Author

Cidan commented Apr 7, 2024

This is the initial implementation, and there's pretty much nothing I like about this change. It feels like a quick hack, so I'm going to rethink this one. I'm also second-guessing it's utility, as externally, there's no good way to extract voice data, apply a DSP, then play it, without the wealth of tooling in the game client.

Copy link
Owner

@karashiiro karashiiro left a comment

Choose a reason for hiding this comment

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

Yeah, I feel a large part of the issue here is just how SoundHandler interacts with Addon(Battle)TalkHandler - it basically fakes a state change to populate the dedupe states early. I'm really not a fan of how that system works, but I haven't been able to come up with a good alternative.

Really, it should emit its own set of events which suppress Addon(Battle)TalkHandler events from being processed by the Say pipeline, but I haven't figured that out, yet. At that point, TextEmitEvent could be unassociated with voice lines altogether, and you could just process them without additional context as they're detected.

However, all of that does leave one remaining problem in that your WebSocket client needs to be able to extract the resulting sound file from sqpack, and there aren't many good libraries for that across programming languages (Lumina, Ironworks, Kobold).

At any rate, turning SoundHandler into an event emitter of some kind is probably the first step in cleaning this up.

/// <summary>
/// The in-game voice file for this line, if applicable.
/// </summary>
public required string VoiceFile { get; init; }
Copy link
Owner

Choose a reason for hiding this comment

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

Arguably shouldn't be required if it won't always exist (the same logic might apply to some existing properties though).

@karashiiro
Copy link
Owner

I'm also second-guessing it's utility, as externally, there's no good way to extract voice data, apply a DSP, then play it, without the wealth of tooling in the game client.

Yeah this is also a problem, since you'd presumably want to mute the in-game line for this, and that's a whole other discussion

@karashiiro
Copy link
Owner

From a UX perspective, I'd actually say that should maybe be its own plugin, unless it were to also apply to TTT's TTS.

@Cidan
Copy link
Contributor Author

Cidan commented Apr 7, 2024

From a UX perspective, I'd actually say that should maybe be its own plugin, unless it were to also apply to TTT's TTS.

Yeah, I came to that conclusion as well about 2 hours ago while mulling this over. It would actually be even better to just implement a DSP in C# and configure the whole thing in-game. I'll toy around with it. I'm going to close this PR as it's clear this isn't the form nor function that should be taken for this.

@Cidan Cidan closed this Apr 7, 2024
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.

Include the voice file name as a string for NPC voiced output in websocket JSON
2 participants