karashiiro / TextToTalk

Chat TTS plugin for Dalamud. Has support for triggers/exclusions, several TTS providers, and more!
MIT License
46 stars 30 forks source link

Lexicon <alias>'s aren't working #114

Closed ryankhart closed 2 years ago

ryankhart commented 2 years ago

This isn't a new issue. Aliases haven't been working for a while now, but I do recall them working at some point in Fall 2021. When I first noticed it, I shrugged it off and just converted my alias lexemes to phonemes instead. (https://github.com/karashiiro/TextToTalk/commit/74e5b1c9ad7d48f08ebeb06e675fd156424dc8f1 https://github.com/karashiiro/TextToTalk/commit/5b8e532122e9fe4f698772fc54f649e186655be6)

But now, as I'm currently building a lexicon specifically for chat acronyms, having an alias feature would make adding to this kind of lexicon so much faster than building a phoneme with obscure characters for each grapheme. I see that having aliases as a feature of TextToTalk appears to be an intended feature (as per below), so I can only assume that it is a bug that may hopefully be simple to fix. https://github.com/karashiiro/TextToTalk/blob/7f6376cc52a6f4545e97fb0393882c2d75acaf80/src/TextToTalk.Lexicons/LexiconManager.cs#L100

ryankhart commented 2 years ago

I've committed and pushed this new file to test with, but I've omitted the package.yml file I have locally until aliases are working again so that users can't access it through the GUI. https://github.com/karashiiro/TextToTalk/blob/1ae98b744029b81b72aa6620f9c7b5cd0a6eb413/lexicons/Chat-FFXIV-Acronyms/lexicon.pls

karashiiro commented 2 years ago

It probably broke in this commit, I'll look into it.

karashiiro commented 2 years ago

Fixed in v1.15.4

ryankhart commented 2 years ago

Hmm, what did you use to test this? I'm testing with /echo NIN with no other lexicons installed but my Chat FFXIV Acronym lexicon (not listed in the repo yet), and it doesn't work still even on 1.15.4.0.

Also, when the "Characters Locations" lexicon is installed and I type /echo NIN, it now gives this error message in the XL Log and does not speak anything.

--------------------------------
17:25:10.846 | INF [TextToTalk] GameObject is null; cannot check gender.
--------------------------------
17:25:10.948 | ERR Unobserved exception in Task.
System.AggregateException: A Task's exception(s) were not observed either by Waiting on the Task or accessing its Exception property. As a result, the unobserved exception was rethrown by the finalizer thread. (Object reference not set to an instance of an object.)
 ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at TextToTalk.Lexicons.LexiconManager.WrapGrapheme(String text, String grapheme, String phoneme) in K:\arashiiro\LexiconManager.cs:line 128
   at TextToTalk.Lexicons.LexiconManager.MakeSsml(String text, String langCode, Int32 playbackRate, Boolean includeSpeakAttributes) in K:\arashiiro\LexiconManager.cs:line 99
   at TextToTalk.Backends.Polly.PollyClient.Say(Engine engine, VoiceId voice, Int32 sampleRate, Int32 playbackRate, Single volume, TextSource source, String text) in K:\arashiiro\Backends\Polly\PollyClient.cs:line 55
   --- End of inner exception stack trace ---
{ } [ Send ]

Should this be a new issue or should this issue be reopened? Reopening for now.

karashiiro commented 2 years ago

I added a new test that specifically tests that aliases get replaced (there was no test for that before).

ryankhart commented 2 years ago

Oh I'm now noticing that now all TTS that includes a grapheme with a phoneme gives this error message and doesn't speak anything.

karashiiro commented 2 years ago

Sure enough, I didn't rerun any of the other tests after that one passed; the others are all failing, now.

karashiiro commented 2 years ago

Should be actually fixed with v1.15.5

ryankhart commented 2 years ago

Ok, now phonemes are working again, so that's great. But aliases still aren't working. It's just passing straight through unchanged. I'm testing with my Chat FFXIV Acronym lexicon with /echo WHM and it still says aloud "double you aitch em" not "white mage".

karashiiro commented 2 years ago

What's the lexeme that's failing? I just tossed together a lexicon for testing this and it worked fine.

<?xml version="1.0" encoding="UTF-8"?>
<lexicon version="1.0" 
      xmlns="http://www.w3.org/2005/01/pronunciation-lexicon"
      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
      xsi:schemaLocation="http://www.w3.org/2005/01/pronunciation-lexicon 
        http://www.w3.org/TR/2007/CR-pronunciation-lexicon-20071212/pls.xsd"
      alphabet="ipa" 
      xml:lang="en">
<lexeme>
<grapheme>white mage</grapheme>
<alias>WHM</alias>
<phoneme>wwww</phoneme>
</lexeme>
</lexicon>
ryankhart commented 2 years ago

I'm trying to do acronym expansion as mentioned with an example on this page: https://www.w3.org/TR/pronunciation-lexicon/#S4.7

Maybe I'm mistaken about how this is supposed to work, but I thought I remembered setting a grapheme to detect when that word is about to be spoken and then replacing it with an alias so that the TTS would speak that alias aloud instead. I'm remembering this based on my original lexicon contributions that used to work like this but now they don't.

This is the lexicon that I'm using to test.

<?xml version="1.0" encoding="UTF-8"?>
<lexicon version="1.0" 
      xmlns="http://www.w3.org/2005/01/pronunciation-lexicon"
      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
      xsi:schemaLocation="http://www.w3.org/2005/01/pronunciation-lexicon 
        http://www.w3.org/TR/2007/CR-pronunciation-lexicon-20071212/pls.xsd"
      alphabet="ipa" 
      xml:lang="en">
<lexeme>
<grapheme>NIN</grapheme>
<grapheme>nin</grapheme>
<alias>ninja</alias>
</lexeme>
<lexeme>
<grapheme>WHM</grapheme>
<grapheme>whm</grapheme>
<alias>white mage</alias>
</lexeme>
</lexicon>
ryankhart commented 2 years ago

I just went through GitHub's git blame tool, and I think I found the point in which it had been working before (at least as I expect it to). https://github.com/karashiiro/TextToTalk/blame/47654029b0a9cfd26c8d8368843a80443fc96a5a/src/TextToTalk.Lexicons/LexiconManager.cs#L100

if (!string.IsNullOrEmpty(lexeme.Alias))
{
    text = text.Replace(lexeme.Grapheme, lexeme.Alias);
}

I'm tinkering with the code now and running my own test builds since I'm starting to understand this side of the codebase a little more now, but I haven't had any success yet.

ryankhart commented 2 years ago

Oh it seems as if there is something preventing the MakeSsml method from running if there exists not . As soon as I put a dummy tag with a non-blank contents (it could be anything like "asdfasdf") and my tested with my local, uncommitted rewrite of MakeSsml (shown below), it worked.

public virtual string MakeSsml(string text, string langCode = null, int playbackRate = -1, bool includeSpeakAttributes = true)
{
    foreach (var lexicon in this.lexicons)
    {
        foreach (var lexeme in lexicon.Lexemes.Where(lexeme => text.Contains(lexeme.Grapheme) || (!string.IsNullOrEmpty(lexeme.Alias) && text.Contains(lexeme.Alias))))
        {
            if (!string.IsNullOrEmpty(lexeme.Alias))
            {
                text = text.Replace(lexeme.Grapheme, lexeme.Alias);
            }
            text = WrapGrapheme(text, lexeme.Grapheme, lexeme.Phoneme);
        }
    }
...
ryankhart commented 2 years ago

Correction to my previous guess: it is making it into the MakeSsml method. It's just not making it into the inner for loop here without a phoneme which is baffling to me because the conditional makes no mention of a phoneme here:

foreach (var lexeme in lexicon.Lexemes.Where(lexeme => text.Contains(lexeme.Grapheme) || (!string.IsNullOrEmpty(lexeme.Alias) && text.Contains(lexeme.Alias))))
ryankhart commented 2 years ago

I just made a breakthrough and got something working. I can submit a pull request when I work the kinks out.

It turns out that graphemes weren't being saved to the list of MicroLexemes unless they had a phoneme in it.

I've removed that restriction and only replaced with a phoneme in the the MakeSsml method if there is a phoneme that is not empty or null.

What I'm working on now is some unwanted recursion when "NIN" is replaced with "ninjaja". It probably got replaced with "ninja" and then the "nin" in "ninja" got replaced a second time.

karashiiro commented 2 years ago

It turns out that graphemes weren't being saved to the list of MicroLexemes unless they had a phoneme in it.

Oops, this is my bad, I thought that lexemes had to have both a grapheme and a phoneme to be valid, but according to the spec it can have either a phoneme or a grapheme. I'll have a look at #117.

karashiiro commented 2 years ago

Alright, should be actually actually actually fixed with v1.15.6