greenbagels / discpp Goto Github PK

A lot of functionality isn't yet included into the library, such as creating messages, diagnostic info, etc. We should aim to provide these features before we hit our first target version release.

This is related to #3 in that we need to not have the connection class manage all the parsing itself.

For now, I think we can settle on having the connection class parse all input to nlohmann::json objects, selectively respond to initial connection / keepalive data, and pass everything else along to the next library layer.

I fear we haven't properly thought about the scope of our project when we first made it, and coupled too much functionality into the discpp::connection class.

A lot of library functionality (like get_gateway) doesn't need to explicitly reference an active WebSocket connection, so let's cut out the extra stuff.

The connection::main_loop() function has a logic error; currently, it does this:

1. Read from the websocket
2. Execute the read handler
3. Wait until all outstanding threads finish their execution
4. Empty the write queue and repeat

The third step was a stop-gap solution to fix the blocking issue. To reproduce the issue, simply do the following:

1. Connect to guilds that are not currently sending any events (i.e. no server activity at all)
2. Wait a length of heartbeat_interval ms
3. Observe as the pending heartbeat is deadlocked by a blocking read().

Obviously, the way the loop is set up needs to change. Realistically, we can't be waiting on read() ever, or else any lack of incoming data will deadlock any recurring writes. Doesn't this mean async is our only other option?

Currently we just include the example copyright notice straight from GNU's website in each source file. Perhaps this is not a great system, as we can just switch to doxygen-style \copyright notices, which would also give us doxygen-based documentation?

We should aim to supply exception safety guarantees for each of our functions we offer in the class. Currently, we've supplied it for none of them, but we should definitely implement it for all of them (even if only a basic guarantee)