Skip to content

Conversation

@BjornGameDev
Copy link
Member

It is for the old TwitchClient, the newest TwitchClient has already been updated, but as it forwards the actual implementation to TwitchClient, and inherits ITwitchClient it should be low maintenance to keep up to date.

Copy link
Member

@swiftyspiffy swiftyspiffy left a comment

Choose a reason for hiding this comment

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

I know next to nothing about Unity, so I won't pretend to know what is going on in CoroutineHost.cs

My worry comes that we're going to constantly need to keep TwitchClientUnity up to date, and that worries me a bit. Is there absolutely no way to avoid needing to rewrite the whole client for unity?

@BjornGameDev
Copy link
Member Author

The client actually wraps the TwitchClient, so it only needs to be updated if the interface changes.

@BjornGameDev
Copy link
Member Author

Yes, it isn't beautiful. I wish I had found a cleaner way to do it. My goal was to keep the way you use TwitchLib the same, so the same docs can be used.

@swiftyspiffy
Copy link
Member

Just sucks to add another place that needs to be constantly kept up to date, but fair enough. I'm going to wait for either @Syzuna @LuckyNoS7evin or one of the others to review this just because I'm not super familiar with Unity.

Copy link
Member

@LuckyNoS7evin LuckyNoS7evin left a comment

Choose a reason for hiding this comment

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

Okay, approved this is Pulled in, I agree there may be a better way, I have an idea, but we should get there soon

@LuckyNoS7evin LuckyNoS7evin merged commit a1a48f6 into TwitchLib:master Feb 27, 2018
LuckyNoS7evin added a commit that referenced this pull request Mar 2, 2018
Update Unity Client to not need logger
LuckyNoS7evin added a commit that referenced this pull request Mar 2, 2018
Merge pull request #1 from TwitchLib/master
LuckyNoS7evin added a commit that referenced this pull request Mar 17, 2018
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.

3 participants