Implement client capability packet

[SkyzohKey]

Continuation of this thread #428 (comment) here to avoid spamming the original issue.

[isotoxin]

Ok. My suggestion.

As you know (may be), the first packet that one peer sends to another (or receives from another) is PACKET_ID_ONLINE. This packet is 1 byte length (including packet id). My suggestion is to send capabilities of client in this packet. Moreover, Isotoxin already doing this. Unfortunately, the current core ignores this package if its size is not 1 byte. That is why, Isotoxin sends this packet twice: 1 byte length for compatibility and full length with capability information.

What about capability format? It is just simple string with format: key:value\n

Current Isotoxin's capability string:
options.client_capabilities = "client:isotoxin/" SS(PLUGINVER) "\n"
"support_bbtags:b,s,u,i\n"
"support_viewsize:1\n"
"support_msg_chain:1\n"
"support_msg_cr_time:1\n"
"support_video_ex:1\n"
"support_folder_share:1\n";

If you do not mind such a method, I can do PR.
Or offer your solution. But remember that the client must know the capabilities of another client in the first packet, otherwise it will be difficult to understand whether the client supports the capability packet, or this packet has not yet been received.

[sudden6]

Good idea, I only see a problem when we get a lot of capabilities, the packet size might be a limit. This can probably be circumvented by sending more packets, but needs some thought.

[SkyzohKey]

client_capabilities should be a struct, like options one, for consistence. I'd like to see something like:

/**
 * We assume every bool to be false if not specified.
 **/
struct Tox_Client_Capabilities {
  /**
   * The client name, lower case alphanumeric, no spaces. Dash allowed.
   */
  const char *client_name;

  /**
   * The client version, using Semantic Versioning. ie. `0.1.7`, `3.8.0-beta`, etc.
   */
  const char *client_version;

  /**
   * True if the client supports ToxMe/ToxDNS/QNL lookups.
   */
  bool supports_lookup;

  /**
   * True if the client supports ToxID sharing.
   * @see Antox and Toxygen
   */
  bool supports_id_sharing;

  /**
   * True if the client supports ToxIdenticons.
   * @see Ricin
   */
  bool supports_tox_identicons;

  /**
   * True if the client supports BBCode rendering.
   */
  bool supports_bbcode;

  /**
   * True if the client supports Markdown rendering.
   */
  bool supports_markdown;

  /**
   * True if the client supports audio calls.
   */
  bool supports_audio;

  /**
   * True if the client supports video calls.
   */
  bool supports_video;

  /**
   * True if the client supports file transfers.
   */
  bool supports_files;

  /**
   * True if the client supports inline images transfers.
   */
  bool supports_inline_images;

  /**
   * True if the client supports avatars.
   */
  bool supports_avatars;

  /**
   * True if the client supports message splitting.
   */
  bool supports_messages_split;

  /**
   * True if the client supports whateveryouwantoaddtothatlist.
   */
  bool supports_whatever;
}

[isotoxin]

client_capabilities should be a struct

Bad idea. What if I come up with a new unique feature? Should I first make PR to the core to expand this structure? No! Only a set of strings with gradual standardization. Otherwise it will not work.

[SkyzohKey]

So please use a standard format. JSON, Yaml, TOML, whatever you want but something standard. :)

[dvor]

I would prefer format be an implementation details. It would be nice if toxcore would be able to parse it and provide some key/value-like API (e.g. all_capability_keys, value_for_key).

[sudden6]

I agree with @dvor toxcore shouldn't need to parse the keys and values, this way clients can implement own stuff.

My proposal for this kind of packet:

[Fixed length binary Header] ... for client capability messages longer than one packet
key=value\n
...

The Header will be something like number of segments as uint16 and segment number as uint16.

Allowed chars for the key will be [a-zA-Z0-9_]. Allowed chars for the value will be everything but\n and =. Binary data will be encoded in base64.

[isotoxin]

Current core accepts only 1-byte length PACKET_ID_ONLINE.
My proposal is use this fact as end of capability sequence.
Old core sends 1-byte PACKET_ID_ONLINE and new core interprets it as end of sequence, so no capability info received.
New core sends number of PACKET_ID_ONLINE with capabilities with last 1-byte length PACKET_ID_ONLINE. Old core accepts only 1-byte PACKET_ID_ONLINE and works normally. New core concatenates all >1 byte PACKET_ID_ONLINE and provides complete capability info to client.

pros:

• Full compatibility between old and new cores
• Client can access to capability info just in tox_callback_friend_connection_status
• Isotoxin already uses this scheme (except concatenation)

cons:

• ?