break: remove co_attach & enforce event handler call semantics (retur…

…n void/dpp::job, take const event)

docpages/advanced_reference/coroutines.md

@@ -19,7 +19,7 @@ int main() {
 
     /* Message handler to look for a command called !file */
     /* Make note of passing the event by value, this is important (explained below) */
-    bot.on_message_create.co_attach([](dpp::message_create_t event) -> dpp::job {
+    bot.on_message_create([](dpp::message_create_t event) -> dpp::job {
 		dpp::cluster *cluster = event.from->creator;
 
         if (event.msg.content == "!file") {
@@ -47,11 +47,11 @@ int main() {
 
 Coroutines can make commands simpler by eliminating callbacks, which can be very handy in the case of complex commands that rely on a lot of different data or steps.
 
-In order to be a coroutine, a function has to return a special type with special functions; D++ offers dpp::job, dpp::task<R>, and dpp::coroutine<R>, which are designed to work seamlessly with asynchronous calls through dpp::async, which all the functions starting with `co_` such as dpp::cluster::co_message_create return. Event routers can have a dpp::job attached to them, as this object allows to create coroutines that can execute on their own, asynchronously. More on that and the difference between it and the other two types later. To turn a function into a coroutine, simply make it return dpp::job as seen in the example at line 10, then use `co_await` on awaitable types or `co_return`. The moment the execution encounters one of these two keywords, the function is transformed into a coroutine.
+In order to be a coroutine, a function has to return a special type with special functions; D++ offers dpp::job, dpp::task, and dpp::coroutine, which are designed to work seamlessly with asynchronous calls through dpp::async, which all the functions starting with `co_` such as dpp::cluster::co_message_create return. Event routers can have a dpp::job attached to them, as this object allows to create coroutines that can execute on their own, asynchronously. More on that and the difference between it and the other two types later. To turn a function into a coroutine, simply make it return dpp::job as seen in the example at line 10, then use `co_await` on awaitable types or `co_return`. The moment the execution encounters one of these two keywords, the function is transformed into a coroutine. Coroutines that use dpp::job can be used for event handlers, they can be attached to an event router just the same way as regular event handlers.
 
 When using a `co_*` function such as `co_message_create`, the request is sent immediately and the returned dpp::async can be `co_await`-ed, at which point the coroutine suspends (pauses) and returns back to its caller; in other words, the program is free to go and do other things while the data is being retrieved and D++ will resume your coroutine when it has the data you need, which will be returned from the `co_await` expression.
 
-\attention You may hear that coroutines are "writing async code as if it was sync", while this is sort of correct, it may limit your understanding and especially the dangers of coroutines. I find **they are best thought of as a shortcut for a state machine**. If you've ever written one, you know what this means. Think of the lambda as *its constructor*, in which captures are variable parameters. Think of the parameters passed to your lambda as data members in your state machine. References are kept as references, and by the time the state machine is resumed, the reference may be dangling : [this is not good](/lambdas-and-locals.html)! As a rule of thumb when making coroutines, **always prefer taking parameters by value and avoid lambda capture**. 
+\attention You may hear that coroutines are "writing async code as if it was sync", while this is sort of correct, it may limit your understanding and especially the dangers of coroutines. I find **they are best thought of as a shortcut for a state machine**, if you've ever written one, you know what this means. Think of the lambda as *its constructor*, in which captures are variable parameters. Think of the parameters passed to your lambda as data members in your state machine. References are kept as references, and by the time the state machine is resumed, the reference may be dangling : [this is not good](/lambdas-and-locals.html)! As a rule of thumb when making coroutines, **always prefer taking parameters by value and avoid lambda capture**. Keep in mind they may also be resumed in another thread than the one they started on, so avoid thread local variables.
 
 ### Several steps in one
 
@@ -67,7 +67,7 @@ int main() {
 
     bot.on_log(dpp::utility::cout_logger());
 
-    bot.on_slashcommand.co_attach([](dpp::slashcommand_t event) -> dpp::job {
+    bot.on_slashcommand([](dpp::slashcommand_t event) -> dpp::job {
         if (event.command.get_command_name() == "addemoji") {
             dpp::cluster *cluster = event.from->creator;
             // Retrieve parameter values
@@ -130,9 +130,9 @@ int main() {
 
 \note This next example is fairly advanced and makes use of many of both C++ and D++'s advanced features.
 
-Earlier we mentioned two other types of coroutines provided by dpp: dpp::coroutine<R> and dpp::task<R>. They both take their return type as a template parameter, which may be void. Both dpp::job and dpp::task<R> start on the constructor for asynchronous execution, however only the latter can be `co_await`-ed, this allows you to retrieve its return value. If a dpp::task<R> is destroyed before it ends, it is cancelled and will stop when it is resumed from the next `co_await`. dpp::coroutine<R> also has a return value and can be `co_await`-ed, however it only starts when `co_await`-ing, meaning it is executed synchronously.
+Earlier we mentioned two other types of coroutines provided by dpp: dpp::coroutine and dpp::task. They both take their return type as a template parameter, which may be void. Both dpp::job and dpp::task start on the constructor for asynchronous execution, however only the latter can be `co_await`-ed, this allows you to retrieve its return value. If a dpp::task is destroyed before it ends, it is cancelled and will stop when it is resumed from the next `co_await`. dpp::coroutine also has a return value and can be `co_await`-ed, however it only starts when `co_await`-ing, meaning it is executed synchronously.
 
-Here is an example of a command making use of dpp::task<R> to retrieve the avatar of a specified user, or if missing, the sender:
+Here is an example of a command making use of dpp::task to retrieve the avatar of a specified user, or if missing, the sender:
 
 ~~~~~~~~~~{.cpp}
 #include <dpp/dpp.h>
@@ -142,7 +142,7 @@ int main() {
 
     bot.on_log(dpp::utility::cout_logger());
 
-    bot.on_slashcommand.co_attach([](dpp::slashcommand_t event) -> dpp::job {
+    bot.on_slashcommand([](dpp::slashcommand_t event) -> dpp::job {
         if (event.command.get_command_name() == "avatar") {
             // Make a nested coroutine to fetch the guild member requested, that returns it as an optional
             constexpr auto resolve_member = [](const dpp::slashcommand_t &event) -> dpp::task<std::optional<dpp::guild_member>> {

include/dpp/event_router.h

@@ -93,6 +93,7 @@ template<class T> class event_router_t {
 	 * @brief Thread safety mutex
 	 */
 	mutable std::shared_mutex lock;
+
 	/**
 	 * @brief Container of event listeners keyed by handle,
 	 * as handles are handed out sequentially they will always
@@ -101,24 +102,6 @@ template<class T> class event_router_t {
 	 */
 	std::map<event_handle, std::function<void(const T&)>> dispatch_container;
 
-
-#ifdef DPP_CORO
-	/**
-	 * @brief Container for event listeners (coroutines only)
-	 *
-	 * Note: keep a listener's parameter as a value type, the event passed can die while a coroutine is suspended
-	 */
-	std::map<event_handle, std::function<dpp::job(const T&)>> coroutine_container;
-#else
-#ifndef _DOXYGEN_
-	/**
-	 * @brief Dummy container to keep the struct size same
-	 */
-	std::map<event_handle, std::function<void(T)>> dummy_container;
-#endif /* _DOXYGEN_ */
-#endif /* DPP_CORO */
-
-
 	/**
 	 * @brief A function to be called whenever the method is called, to check
 	 * some condition that is required for this event to trigger correctly.
@@ -163,13 +146,6 @@ template<class T> class event_router_t {
 				listener(event);
 			}
 		};
-#ifdef DPP_CORO
-		for (const auto& [_, listener] : coroutine_container) {
-			if (!event.is_cancelled()) {
-				listener(event);
-			}
-		}
-#endif  /* DPP_CORO */
 	};
 
 	/**
@@ -181,11 +157,7 @@ template<class T> class event_router_t {
 	 */
 	bool empty() const {
 		std::shared_lock l(lock);
-#ifdef DPP_CORO
-		return dispatch_container.empty() && coroutine_container.empty();
-#else
 		return dispatch_container.empty();
-#endif /* DPP_CORO */
 	}
 
 	/**
@@ -199,69 +171,112 @@ template<class T> class event_router_t {
 		return !empty();
 	}
 
+#ifdef _DOXYGEN_
 	/**
-	 * @brief Attach a lambda to the event, adding a listener.
-	 * The lambda should follow the signature specified when declaring
-	 * the event object and should take exactly one parameter derived
-	 * from event_dispatch_t.
-	 * 
-	 * @param func Function lambda to attach to event
+	 * @brief Attach a callable to the event, adding a listener.
+	 * The callable should either be of the form `void(const T &)` or
+	 * `dpp::job(T)` (the latter requires DPP_CORO to be defined),
+	 * where T is the event type for this event router.
+	 *
+	 * This has the exact same behavior as using attach.
+	 *
+	 * @see attach
+	 * @param fun Callable to attach to event
 	 * @return event_handle An event handle unique to this event, used to
 	 * detach the listener from the event later if necessary.
 	 */
-	event_handle operator()(std::function<void(const T&)> func) {
-		return this->attach(func);
+	template <typename F>
+	event_handle operator()(F&& fun);
+
+	/**
+	 * @brief Attach a callable to the event, adding a listener.
+	 * The callable should either be of the form `void(const T &)` or
+	 * `dpp::job(T)` (the latter requires DPP_CORO to be defined),
+	 * where T is the event type for this event router.
+	 *
+	 * @param fun Callable to attach to event
+	 * @return event_handle An event handle unique to this event, used to
+	 * detach the listener from the event later if necessary.
+	 */
+	template <typename F>
+	event_handle attach(F&& fun);
+#else /* not _DOXYGEN_ */
+#  ifdef DPP_CORO
+	/**
+	 * @brief Attach a callable to the event, adding a listener.
+	 * The callable should either be of the form `void(const T &)` or
+	 * `dpp::job(T)`, where T is the event type for this event router.
+	 *
+	 * @param fun Callable to attach to event
+	 * @return event_handle An event handle unique to this event, used to
+	 * detach the listener from the event later if necessary.
+	 */
+	template <typename F>
+	requires (utility::callable_returns<F, dpp::job, const T&> || utility::callable_returns<F, void, const T&>)
+	event_handle operator()(F&& fun) {
+		return this->attach(std::forward<F>(fun));
 	}
 
 	/**
-	 * @brief Attach a lambda to the event, adding a listener.
-	 * The lambda should follow the signature specified when declaring
-	 * the event object and should take exactly one parameter derived
-	 * from event_dispatch_t.
+	 * @brief Attach a callable to the event, adding a listener.
+	 * The callable should either be of the form `void(const T &)` or
+	 * `dpp::job(T)`, where T is the event type for this event router.
 	 *
-	 * @param func Function lambda to attach to event
+	 * @param fun Callable to attach to event
 	 * @return event_handle An event handle unique to this event, used to
 	 * detach the listener from the event later if necessary.
 	 */
-	event_handle attach(std::function<void(const T&)> func) {
+	template <typename F>
+	requires (utility::callable_returns<F, dpp::job, const T&> || utility::callable_returns<F, void, const T&>)
+	event_handle attach(F&& fun) {
 		std::unique_lock l(lock);
 		event_handle h = next_handle++;
-		dispatch_container.emplace(h, func);
+		dispatch_container.emplace(h, std::forward<F>(fun));
 		return h;
 	}
+#  else
+	/**
+	 * @brief Attach a callable to the event, adding a listener.
+	 * The callable should be of the form `void(const T &)`
+	 * where T is the event type for this event router.
+	 *
+	 * @param fun Callable to attach to event
+	 * @return event_handle An event handle unique to this event, used to
+	 * detach the listener from the event later if necessary.
+	 */
+	template <typename F>
+	std::enable_if_t<utility::callable_returns_v<F, void, const T&>, event_handle> operator()(F&& fun) {
+		return this->attach(std::forward<F>(fun));
+	}
 
-#ifdef DPP_CORO
 	/**
-	 * @brief Attach a coroutine task to the event, adding a listener.
-	 * The coroutine should follow the signature specified when declaring
-	 * the event object and should take exactly one parameter derived
-	 * from event_dispatch_t.
+	 * @brief Attach a callable to the event, adding a listener.
+	 * The callable should be of the form `void(const T &)`
+	 * where T is the event type for this event router.
 	 *
-	 * @param func Coroutine task to attack to the event. <b>It MUST take the event by value.</b>
+	 * @param fun Callable to attach to event
 	 * @return event_handle An event handle unique to this event, used to
 	 * detach the listener from the event later if necessary.
 	 */
-	event_handle co_attach(std::function<job(T)> func) {
+	template <typename F>
+	std::enable_if_t<utility::callable_returns_v<F, void, const T&>, event_handle> attach(F&& fun) {
 		std::unique_lock l(lock);
 		event_handle h = next_handle++;
-		coroutine_container.emplace(h, func);
+		dispatch_container.emplace(h, std::forward<F>(fun));
 		return h;
 	}
-#endif /* DPP_CORO */
+#  endif /* DPP_CORO */
+#endif /* _DOXYGEN_ */
 	/**
 	 * @brief Detach a listener from the event using a previously obtained ID.
-	 * 
+	 *
 	 * @param handle An ID obtained from event_router_t::operator()
 	 * @return true The event was successfully detached
 	 * @return false The ID is invalid (possibly already detached, or does not exist)
 	 */
 	bool detach(const event_handle& handle) {
 		std::unique_lock l(lock);
-#ifdef DPP_CORO
-		return this->dispatch_container.erase(handle) || this->coroutine_container.erase(handle);
-#else
 		return this->dispatch_container.erase(handle);
-#endif /* DPP_CORO */
 	}
 };
 

include/dpp/utility.h

@@ -603,5 +603,41 @@ namespace dpp {
 		 */
 		void DPP_EXPORT set_thread_name(const std::string& name);
 
+#ifdef __cpp_concepts // if c++20
+		/**
+		 * @brief Concept satisfied if a callable F can be called using the arguments Args, and that its return value is convertible to R.
+		 *
+		 * @tparam F Callable object
+		 * @tparam R Return type to check for convertibility to
+		 * @tparam Args... Arguments to use to resolve the overload
+		 * @return Whether the expression `F(Args...)` is convertible to R
+		 */
+		template <typename F, typename R, typename... Args>
+		concept callable_returns = std::convertible_to<std::invoke_result_t<F, Args...>, R>;
+
+		/**
+		 * @brief Type trait to check if a callable F can be called using the arguments Args, and that its return value is convertible to R.
+		 *
+		 * @deprecated In C++20 mode, prefer using the concept `callable_returns`.
+		 * @tparam F Callable object
+		 * @tparam R Return type to check for convertibility to
+		 * @tparam Args... Arguments to use to resolve the overload
+		 * @return Whether the expression `F(Args...)` is convertible to R
+		 */
+		template <typename F, typename R, typename... Args>
+		inline constexpr bool callable_returns_v = callable_returns<F, R, Args...>;
+#else
+		/**
+		 * @brief Type trait to check if a callable F can be called using the arguments Args, and that its return value is convertible to R.
+		 *
+		 * @tparam F Callable object
+		 * @tparam R Return type to check for convertibility to
+		 * @tparam Args... Arguments to use to resolve the overload
+		 * @return Whether the expression `F(Args...)` is convertible to R
+		 */
+		template <typename F, typename R, typename... Args>
+		inline constexpr bool callable_returns_v = std::is_convertible_v<std::invoke_result_t<F, Args...>, R>;
+#endif
+
 	} // namespace utility
 } // namespace dpp

src/unittest/coro.cpp

@@ -389,7 +389,7 @@ void coro_offline_tests()
 }
 
 void event_handler_test(dpp::cluster *bot) {
-	bot->on_message_create.co_attach([](dpp::message_create_t event) -> dpp::job {
+	bot->on_message_create([](dpp::message_create_t event) -> dpp::job {
 		if (event.msg.content == "coro test") {
 			dpp::cluster *bot = event.from->creator;
 

src/unittest/test.cpp

@@ -574,7 +574,7 @@ Markdown lol \\|\\|spoiler\\|\\| \\~\\~strikethrough\\~\\~ \\`small \\*code\\* b
 		 * are sending audio later, this way if the audio receive code is plain unstable
 		 * the test suite will crash and fail.
 		 */
-		bot.on_voice_receive_combined([&](auto& event) {
+		bot.on_voice_receive_combined([&](const auto& event) {
 		});
 
 		std::promise<void> ready_promise;