redbluegames / unity-text-typer

Text typing effect for Unity uGUI Text components
MIT License
307 stars 28 forks source link

Switch to using text mesh pro #14

Closed timminkov closed 6 years ago

timminkov commented 6 years ago

I tested this a bit and it seemed to work fine. TMPro has basically the same API as a text component so I don't think much else needs to change (I could be wrong though).

edwardrowe commented 6 years ago

Thanks for the pull request, and for researching integrating TMP. I like this, but I don't want the core package to depend on users downloading the TextMeshPro package.

What I'd like to do is depend on a ITextDisplay interface, that has one property getter .Text. Then figure out which type of component they linked up. I'd want it to still compile if the users don't have TMP installed. But I'm not sure of an easy way to do that, especially at compile time.

So the TextComponent property here would be something like:

private ITextDisplay TextComponent
{
   get 
   {
      if (this.textComponent != null)
      {
         return this.textComponent;
      }
      #if TEXT_MESH_PRO
      else if (this.textMeshProComponent != null)
      {
         return this.textMeshProComponent
      }
      #endif
      else { return null; }
   }
}

And Text : ITextDisplay, TextMeshProUGUI : ITextDisplay.. But that's obviously not possible.

So I'll have to keep thinking on this, unless you have other ideas?

timminkov commented 6 years ago

That makes sense! I actually searched the forks for someone trying the same thing as me with no success.

One note is that TMPro is integrated as a default package in Unity 2018. Text is still there though - I just have no idea why anyone would ever use a normal text component at this point.

edwardrowe commented 6 years ago

If TMP is integrated into 2018 by default, we could use #if UNITY_2018 or something, I'm sure. I'll have to verify it's installed with a specific version. I'm still trying to figure out the best way to let users choose which type of component they'd like to use.

edwardrowe commented 6 years ago

So I've finally had some time to look at this. It looks like TMP has a property specifically designed for this: maxVisibleCharacters. So I really need to rework the codebase so that it doesn't have more bloat than people need.

That said, as you suggested using TMP is really the only way to go, so I think we should just take this commit and branch or tag an old version of this for users who don't want or can't use TMP.

Thanks for the pull request and encouraging me to finally look into this!

edwardrowe commented 6 years ago

This resolves Issue #15

timminkov commented 6 years ago

I realize I'm a few months late but thanks for the merge 👍