FEAT(client): Introduced new mumble API to link and unlink

[mryamac]

This patch introduces a new API to link and unlink the given channels to the given link ids.

Checks

• My commits follow the commit guidelines

[Krzmbrzl]

Thinking about the feature introduced here, I believe it would benefit from completing the interface w.r.t. linked channels:

• Allow to link/unlink an arbitrary amount of channels with a single function call (e.g. take an array of channel IDs instead of a pair of them)
• Add function to get the set of linked channels a given channel is in
• Optionally, add a convenience function to check whether a given pair of channels is linked with each other.
  What do you think?

[mryamac]

Thinking about the feature introduced here, I believe it would benefit from completing the interface w.r.t. linked channels:

• Allow to link/unlink an arbitrary amount of channels with a single function call (e.g. take an array of channel IDs instead of a pair of them)
• Add function to get the set of linked channels a given channel is in
• Optionally, add a convenience function to check whether a given pair of channels is linked with each other.
  What do you think?

I think we don't need such functions for the first case, multiple link and unlink channels. Let me explain why I am feeling like this. How can we handle this in case of an exception? If one of the channels does not exist, what should I do? Should I link others, or should I send an error? This decision belongs to people who call API. Am I right?

For the second case, the get function is ok for me. How about the set function if the same case happens like the above?
Instead of a set function, we may write a clear function to remove all linked channels. How about this one?

In the third case, checking whether the pair of channels is linked or not is also ok for me.

[Krzmbrzl]

If one of the channels does not exist, what should I do? Should I link others, or should I send an error? This decision belongs to people who call API. Am I right?

No - if the user provides an invalid channel, the API call returns an error and does nothing.

How about the set function if the same case happens like the above?

I'm I afraid I can't quite follow your meaning here 👀

Instead of a set function, we may write a clear function to remove all linked channels. How about this one?

I wasn't proposing a set function, I was proposing a function that returns the set (in the mathematical sense) of channels that are all linked together. Specifically that set to which a given channel belongs (which can be the empty set - aka an empty array).
The clear-function that you propose should be equivalent to the unlink function that I suggested, no?

[mryamac]

If one of the channels does not exist, what should I do? Should I link others, or should I send an error? This decision belongs to people who call API. Am I right?

No - if the user provides an invalid channel, the API call returns an error and does nothing.

Ok

How about the set function if the same case happens like the above?

I'm I afraid I can't quite follow your meaning here 👀

Sorry about that. I need to be clearer. I thought the set was another function, so, I mean that channel does not exist for a given channel list. This question is no more valid with your explanation.

Instead of a set function, we may write a clear function to remove all linked channels. How about this one?

I wasn't proposing a set function, I was proposing a function that returns the set (in the mathematical sense) of channels that are all linked together. Specifically that set to which a given channel belongs (which can be the empty set - aka an empty array). The clear-function that you propose should be equivalent to the unlink function that I suggested, no?

My proposed method is not exactly the same as the unlink function. The proposed method, clear, won't take the channels list. It unlinks all linked channels. Actually, this can be done using get and unlink methods, but it's simpler usage. It is up to you, the clear method is not necessary.

[Krzmbrzl]

My proposed method is not exactly the same as the unlink function. The proposed method, clear, won't take the channels list. It unlinks all linked channels. Actually, this can be done using get and unlink methods, but it's simpler usage. It is up to you, the clear method is not necessary.

I have the feeling that this function would probably be unusable on general servers as there might be linked channels that are completely unrelated to what the plugin is doing but if that plugin would call this clear function, those links would be gone as well. That doesn't seem desirable to me 🤔

[Krzmbrzl]

All new API functions are missing the mumble_connection_t argument (as the second argument - see other functions)

[Krzmbrzl]

All new API functions are missing the mumble_connection_t argument (as the second argument - see other functions)

☝️
that seems to have gone missing in the mass of review comments :)

This one too:

Finally, there should be a new function inside ServerHandler that can transmit all to-be-linked channels in a single message. Otherwise, we risk running into the server's (per default rather strict) rate limiter and request messages will get dropped.

[mryamac]

All new API functions are missing the mumble_connection_t argument (as the second argument - see other functions)

☝️ that seems to have gone missing in the mass of review comments :)

This one too:

Finally, there should be a new function inside ServerHandler that can transmit all to-be-linked channels in a single message. Otherwise, we risk running into the server's (per default rather strict) rate limiter and request messages will get dropped.

Could you check the latest changes?

[Krzmbrzl]

@mryamac why did you delete the branch?

[mryamac]

I will add two more API functions, and I thought a fresh start would be nice.

[Krzmbrzl]

Ah okay - so a new PR will be coming at some point in the future? Then ignore my comment :D