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

ChatThrottleLib: Add Battle.net addon message support #14

Merged
merged 2 commits into from
May 7, 2024

Conversation

Meorawr
Copy link
Contributor

@Meorawr Meorawr commented May 5, 2024

This adds the necessary hooks and public APIs to support sending comms over the BNSendGameData API.

There's a few differences to note up-front about CTL's provided interface for this functionality.

  • The BNSendGameData API supports sending messages up to 4078 bytes in length, but our implementation limits it to 255 bytes.
  • The BNSendGameData API has a different parameter ordering and no 'chattype' parameter, whereas our implementation has a parameter ordering consistent with SendAddonMessage and requires a chattype.

On message size, our round-robin message selection effectively pauses when it reaches a message for which there isn't yet enough accrued bandwidth to send. In the case of 4078 byte messages that this API supports, this could mean with the current CPS value that a priority blocks for up to 5 seconds before enough bandwidth is available to send it off. This would be extremely unfair to everything else sending data, so limiting it to the usual 255 bytes sounds more reasonable.

On parameter ordering, there's an inconsistency between the API's parameters and the data supplied in its event; the API doesn't have a chattype parameter but its receipt event fires with one always set to 'WHISPER'.

If the API changed to require a chattype parameter down the line it would require a backwards compatibility break in our interface if we hadn't accommodated it, so it feels sensible to just require it up-front even if it must always be 'WHISPER'.

With both the message size and added chattype parameter in mind, it then also makes sense to just make the interface have the same parameter ordering as SendAddonMessage to make it a bit easier to generically use these functions. BNSendGameData requires an order of (target, prefix, data), whereas our interface is the standard (prio, prefix, data, chattype, target, ...).

@Meorawr Meorawr force-pushed the feature/add-bngamedata-support branch from 3f13060 to da734cb Compare May 5, 2024 20:25
@Meorawr Meorawr marked this pull request as ready for review May 5, 2024 20:25
@Meorawr Meorawr force-pushed the feature/add-bngamedata-support branch 2 times, most recently from 2e45279 to 7c934ff Compare May 6, 2024 07:38
@Meorawr Meorawr changed the title Add Battle.net addon message support ChatThrottleLib: Add Battle.net addon message support May 6, 2024
@Meorawr
Copy link
Contributor Author

Meorawr commented May 6, 2024

Note that this is good to go and tested, but I'd recommend also merging in #17 because this API can error if you attempt to send to a game account ID that the client doesn't know about.

This adds the necessary hooks and public APIs to support sending comms
over the BNSendGameData API.

There's a few differences to note up-front about CTL's provided
interface for this functionality.

- The BNSendGameData API supports sending messages up to 4078 bytes in
  length, but our implementation limits it to 255 bytes.

- The BNSendGameData API has a different parameter ordering and no
  'chattype' parameter, whereas our implementation is consistent with
  SendAddonMessage.

On message size, our round-robin message selection effectively pauses
when it reaches a message for which there isn't yet enough accrued
bandwidth to send. In the case of 4078 byte messages that this API
supports, this could mean with the current CPS value that a priority blocks
for up to 5 seconds before enough bandwidth is available to send it off.

This would be extremely unfair to everything else sending data, so we limit
it to the usual 255 bytes.

On parameter ordering, there's an inconsistency between the API's
parameters and the data supplied in its event; the API doesn't have a
chattype parameter but its event fires with one always set to 'WHISPER'.

If the API changed to require a chattype parameter down the line it
would require a backwards compatibility break in our interface if we
hadn't accomodated it, so it feels sensible to just require it up-front
even if it must always be 'WHISPER'.

With both the message size and added chattype parameter in mind, it then
also makes sense to just make the interface have the same parameter
ordering as SendAddonMessage to make it a bit easier to generically use
these functions. BNSendGameData requires an order of (target, prefix,
data), whereas our interface is the standard (prio, prefix, data,
chattype, target).
@Nevcairiel Nevcairiel merged commit b8d1722 into WoWUIDev:master May 7, 2024
1 check passed
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.

2 participants