Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

A couple issues I've found while trying to build. #60

Closed
Warpten opened this issue Sep 25, 2020 · 3 comments
Closed

A couple issues I've found while trying to build. #60

Warpten opened this issue Sep 25, 2020 · 3 comments

Comments

@Warpten
Copy link

Warpten commented Sep 25, 2020

void guild::_load(const json & obj, shards::shard * _shard) noexcept

This function is noexcept, but throws.
Also, in one of the catch clauses:

spdlog::get("aegis")->error("Shard#{} : Error processing guild[{}] {}", _shard->get_id(), g_id, (std::string)e.what());

The cast to std::string feels redundant, on top of oncurring an unnecessary construction.


Consider allowing users to use Boost.Asio instead of standalone asio (It should pretty much be a drop-in replacement, at least for versions of Boost.Asio up to 1.73 (1.74 has some namespace changes that require aliases). This is what websocketpp does, which is one of your dependency. The most egregious change is asio::system_error, since it doesn't exist in Boost.Asio: boost::system::error_code is the counterpart. In turn, because websocketpp uses std::error_code, there are calls that will no longer compile (because they used asio::system_error), and need to be manually fixed.


There are missing includes in some places (I'm not using PCH headers, maybe that's why you're not seeing those). Unfortunately, since all my edits are currently unstaged, I can't provide a diff. If you wish me to, I can try to generate one, but I heavily hacked away at config.hpp (removing the polyfills for std::optional, among other things), so I'll have to filter some changes out.


                        // shard_mgr.cpp
                        _shard->_connection = websocket_o.get_connection(gateway_url, ecs);
                        _shard->_strand = _shard->_connection->get_strand();
                        if (ecs)
                            throw ecs;

Should probably become this?

                        _shard->_connection = websocket_o.get_connection(gateway_url, ecs);
                        if (ecs)
                            throw ecs;
                        _shard->_strand = _shard->_connection->get_strand();

On MSVC, i'm getting duplicate function bodies across multiple types, namely permissions's to_json and from_json. This was deeply confusing and I just moved the implementation back into permissions.hpp. Not a clean solution, but I'm not sure why this happens.

Cheers

@Warpten
Copy link
Author

Warpten commented Sep 25, 2020

Managed to get it to build successfully with Boost.Asio. This also unfortunately required changes on websocketpp's side:

  • Plugging its error system into boost instead of std.
  • Removing multiple calls to lib::ref(*m_io_service) or similar variants to avoid issues within Boost pertaining to comparing T == std::reference_wrapper<T>

Integrating Boost seems ... Daunting. Could be that websocketpp's code depends on an ancient boost version; from a quick glance at their CMakeLists, it appears to be at most 1.39.

@zeroxs
Copy link
Owner

zeroxs commented Sep 26, 2020

I use VS and do not use PCH so I'm not sure where the missing headers are. As for using Boost.Asio, it was never my intent to support it over standalone. If someone would like to PR a way to use both, I'd accept it. Your example of it not working well in 1.74 is exactly why I don't use it. I only need Asio so I went for standalone.

@Warpten
Copy link
Author

Warpten commented Sep 28, 2020

Sure, I just didn't feel like pulling standalone Asio when I already had Boost in my dependency tree :)

I saw you had a ticket open to update deps on Asio and websocketpp, I unfortunately cannot help in that regard since I've only started using Asio fairly recently ... No idea where I'd need to plug executors in Aegis

@zeroxs zeroxs closed this as completed Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants