Support sending messages over IRC

[kevinrpb]

Hi there!

I have been working for a while on an app to interact with Twitch chat and subsequently implementing pretty much the same as this library (for IRC and Helix, at least). It was a neat surprise to come across the repo just today, since it looks really neat and something I could perhaps help a bit with!

My first attempt at a contribution will be allowing sending chat messages through IRC, which I believe was missing. This should also solve #10, since the IRC client can be independently instantiated.

Please, let me know what you think about the change, or if you had something else in mind.

Cheers!

[kevinrpb]

@LosFarmosCTL hey, friendly ping in case you missed this. I have other changes I would like to contribute (adding Sendable conformances and possibly unifying Emote models), let me know if I should go ahead and send PRs for those 😄

[LosFarmosCTL]

@LosFarmosCTL hey, friendly ping in case you missed this.

Oh whoops yup, thanks for the ping and PR in general, I did totally miss this 😄 Sorry for the general inactivity, I've injured my shoulder about 2 months back and couldn't really type until ~1 week ago, once my exams this month are over there will be some major progress here again (targeting a first stable v1 release at the end of this or early next year).

You're totally right, sending messages over IRC is indeed missing, never got around to implementing it, since I always use the Helix sendMessage endpoint now. It is definitely something that the IRC client should be able to do though, however there are some more details that have to be considered:

The biggest thing is that the connection used to send messages has to be be separate from the one to receive them. This is because Twitch has no way of notifying you whether or not your message was dropped via IRC (e.g. due to spam in the channel or your account being shadowbanned), the only way to do so is to check if that message was actually received in a joined channel. And, since Twitch does not send your own messages back to you on the same connection, we need 2 separate ones (this is also what projects like https://github.com/chatterino/chatterino2 have to do)
Since Swifts main target is still mobile platforms, I would like there to be an option to disable creating that write-only connection. When the sendMessage function is called in that case, throw an exception. The same goes for trying to send a message using an anonymous connection, since that obviously won't work either.

Also, the public interface for the IRC client should use the IRCAuthenticationStyle enum that is passed to TwitchClient.ircClient right now, modified to include the TwitchCredentials inside of the authenticated case. TwitchClient.ircClient() can use the authenticated case by default (also, rename to something like createIRCClient while we're at it).

And the last thing, the public interface of the IRC client should not expose OutgoingMessage directly, instead the options of .privmsg should be provided as arguments to the sendMessage function, which can pass it on to a general function in the connection.

So, to summarize, the general interface I imagine would be like this:

actor TwitchIRCClient {
  private writeConnection: IRCConnection
  private readConnectionPool: IRCConnectionPool

  init(
    _ authenticationStyle: IRCAuthenticationStyle, 
    options: IRCClientOptions = .init(), // don't require options to be customized
    urlSession: URLSession = URLSession(configuration.default)
  )

  // throws if write connection is disabled or connection is anonymous
  func sendMessage(
    _ message: String, in channel: String,
    replyTo messageId: String? = nil,
    clientNonce: String? = nil
  )
}
 
// there will be more options in the future, I have some things that I want to make configurable
// so an options struct makes sense
struct IRCClientOptions {
  let enableWriteConnection: Bool = true // default to true
}

enum IRCAuthenticationStyle {
  case anonymous
  case authenticated(using: TwitchCredentials)
}

extension TwitchClient {
  public func createIRCClient() // always return an authenticated client
}

I hope everything was understandable, if you have questions feel free to just ask 😄

---

I have other changes I would like to contribute (adding Sendable conformances and possibly unifying Emote models), let me know if I should go ahead and send PRs for those 😄

Regarding this, I'm working on Sendable conformances myself, want to redesign some stuff around that as well, but unifying emote models is something that should be done and I'd be happy to accept PRs, if you're interested in working on it.

[kevinrpb]

Sorry for the general inactivity, I've injured my shoulder about 2 months back and couldn't really type until ~1 week ago

Haha, no worries, hope all is good now 👍🏽 .

RE: Needing two connections, yes, I am aware of that. I was actually waiting on an answer here to ask you whether that should be handled directly in the TwitchIRCClient or if users should do it themselves if needed (this is what I'm currently doing in my app).

Given your answer I think I can spend some time integrating it directly into the client and will report back once I get to it 😄 .

Also, the public interface for the IRC client should use the IRCAuthenticationStyle enum [...]

This makes sense, will take it into consideration.

And the last thing, the public interface of the IRC client should not expose OutgoingMessage directly, instead the options of .privmsg should be provided as arguments to the sendMessage function, which can pass it on to a general function in the connection.

Hmm. Not sure about this one. The API already exposes IncomingMessage when receiving the messages, why do you think it's best not to expose OutgoingMessage?

[LosFarmosCTL]

Haha, no worries, hope all is good now 👍🏽 .

Yup, doing a lot better now, thanks 😄

Hmm. Not sure about this one. The API already exposes IncomingMessage when receiving the messages, why do you think it's best not to expose OutgoingMessage?

Exposing the type itself isn't the issue, but OutgoingMessage adds the additional step of having to specify what outgoing message to send. The only 3 that have to be handled by the user are JOINs, PARTs and PRIVMSGs, while stuff like NICK, PASS, CAP REQ, PING etc. are all handled internally and never have to be supplied manually. It would also create unexpected behavior unless manually checking every type of message passed into it, if for example the user decides to send a JOIN into the write connection, which is not intended to be possible.

I think I've actually meant to create a new enum to translate IncomingMessage into as well, since at the point of handling the returned message, there is no reason for the user to expect e.g. a CAP ACK and include it in some switch case, since those are fully handled internally anyways.

[LosFarmosCTL]

Ignore the linting error, unrelated to this PR, there was a rule change in SwiftLint.

Fixed in main, makes more sense to return raw data if JSON parsing fails anyways, if Twitch returns a response that isn't valid UTF-8 there is no reasonable way to handle that at the library scope.

[kevinrpb]

Hey there, had some time to put into this 😄 .

I have changed what we discussed:

• IRC client init uses the existing IRCAuthenticationStyle (I moved it to TwitchIRCClient.AuthenticationStyle because it made sense to me, LMK what you think). TwitchClient always returns an authenticated IRC client.
• IRC client receives an options struct (TwitchIRCClient.Options, again LMK).
• IRC client handles a write connection according to the options.
• IRC client doesn't expose OutgoingMessage, but receives the parameters instead.

---

Now, a couple things from your message that I didn't answer to before:

Exposing the type itself isn't the issue, but OutgoingMessage adds the additional step of having to specify what outgoing [...]

Ah, I hear you. Changed it to receive the message information 👍🏽 .

I think I've actually meant to create a new enum to translate IncomingMessage into as well [...]

Hmm not sure what I think about this one. It seems like copying something that exists already, but I also understand where you're coming from.

---

Other than that, I'll try to spend some time this week unifying the emotes. It will need some thinking because the responses are ever so slightly different. Will get back to you when I have something that I think makes sense.

Cheers!

[LosFarmosCTL]

There's one leftover method from the earlier version of this PR and the formatting check failed, other than that this looks good to me!

[kevinrpb]

Ah, missed that. I'll try to update it later today 👍🏽

[kevinrpb]

@LosFarmosCTL thank you for the merge 😄

I'll try to get something going for the emotes Soon ™️ .