Check if an Event is handled or not?
Closed this issue · 13 comments
Hi there, i am completely new to this library, but it looks very promising and i love it so far!
Sorry if this is the wrong place to ask, but the Readme suggested to ask here:
Basically I want to check if a postponed event is handled by someone else. Example:
struct WindowCloseEvent{};
void listen() {
bus->listen([](const WindowCloseEvent& event) {
// Custom handling code
});
}
void update() {
bus->postpone(WindowCloseEvent{});
bus->process();
if (!bus->handled()) { // <- Does not exist
window.close();
}
}listen() may or may not be called, depending on how the user uses the library.
The update part of the code should dispatch the event. If there is no custom listener attached it should execute a default handler, which just closes the window. But if a custom handler for this event is attached, it should execute the user implementation and not the default code.
My intuition would have been to call if (bus->postpone(...)) or similar, but none of these functions seem to return the correct information. I also tried to override the same event again with another implementation, but this throws an exception.
Thank you for any help!
Hello, thank you for question
There is no way currently to determine if someone/something handled the event. Also there is no way to add a "default" handler for any event or unhandled event. I'm thinking about it for debug purposes.
bus->process(); return only number of processed events (no matter if there was a listener for such an event or not)
bus->postpone() just return true/false if the event was successfully added to the queue
I will update documentation for that. (I will keep open this issue)
In my opinion you should store your state on your own. I mean the state of your listener as a listener could be called by "Event" or by simply callback in your code. Another approach would be to create 2 listeners. First one would handle logic as in your example, other will just be by some kind of state handler which will collect information but I would not recommend this approach.
Handling state by this library if something was called isn't a good idea (I think but we may argue). Simply you need information about if it was called, someone wants information how many times it was called and another person needs information how long ago it was called, so as you can see there are many use cases for handling such a state.
Thank you for your response!
I also tried this library: https://github.com/DeveloperPaul123/eventbus, but it also has currently no way of doing that, and because it is slightly more complicated i'm back to your library. The only difference i noticed is that this library allows to register multiple handlers for the same type which are then called in order, while your library throws an exception, but still no way to check if it was handled.
It works fine as i only call it in a wrapper function and never expose it directly, so i can do whatever i want in the wrapper.
Actually I don't need any of the information you listed, the only information i need is if a handler was attached or not, so i can decide wether or not to execute the default behaviour. My current workaround is:
Have a std::vector<std::string> as a list of registered handlers and whenever a handler is registered in the wrapper, i call listen and store typeid(T).name() in the list. When an event is called in the second wrapper, i check if typeid(T).name() is in the list and if so, call postpone and process, if not, not.
I think the easiest and most beautiful solution to this challenge would be to add an EventBus function that simply checks if the handler is registered, since you surely must have an internal representation of all the registered handlers. It would be called like if (bus->is_listening<WindowCloseEvent>()), maybe a similar name.
I think this would work amazingly well since you have the exceptions in place that allow only a single handler per type, meaning you can have zero or one handlers per type, which aligns perfectly with the is_listening function, making the library more complete.
Thank you for your consideration!
1 listener ID can have only 1 event handler thats because you need to distinguish which listen + unlisten pair you would like to dismiss. Of course you can have multiple listeners for same event. But this is only for your safety to have 1 listener that can listen for 1 event type.
Remember that you can register lambda as listener. Then you need a way to unregister such lambda. Thats why listenerID is returned.
You shouldn't relay on order of calling of your handlers this is some kind "UB" for library. Currently it works that way.
You should be using Listener so it will handle for you when to listen and unlisten in RAII style.
For now I may suggest such approach:
template <typename Event>
class MyListener
{
public:
MyListener(std::shared_ptr<EventBus> bus)
: _listener(bus)
{
}
void listen(std::function<void(const Event&)> callback)
{
assert(isListening() == false);
_listener.listen(callback);
_isListening = true;
}
void unlisten()
{
_listener.unlistenAll();
_isListening = false;
}
~MyListener()
{
_isListening = false;
}
bool isListening() const
{
return _isListening;
}
private:
EventBus::Listener _listener;
bool _isListening = false;
};
struct WindowCloseEvent{};
int main(int argc, char* argv[])
{
auto eventBus = std::make_shared<EventBus>();
MyListener<WindowCloseEvent> listener{eventBus};
if(false /*you decide when*/ && listener.isListening() == false)
{
listener.listen([](const auto& event)
{
//...
});
}
if(true && listener.isListening() == false)
{
listener.listen([](const auto& event)
{
// other listener logic
});
}
if(listener.isListening())
{
//...
}
...
}
This is just simple implementation approach. Maybe better is to implement "protocol" of your listener in such way to not use lambda and use just methods. Eg:
struct WindowCloseEvent{};
template <typename Base, typename Event>
class MyListener
{
public:
MyListener(std::shared_ptr<EventBus> bus)
: _listener(bus)
{
}
void listen()
{
assert(isListening() == false);
_listener.listen([self = static_cast<Base*>(this)](const Event& e)
{ self->callback(e); });//better than std::bind
_isListening = true;
}
void callback(const Event& event)
{
//Generic callback for any event not implemented
std::cout << "generic" << __PRETTY_FUNCTION__ << std::endl;
}
void unlisten()
{
_listener.unlistenAll();
_isListening = false;
}
~MyListener()
{
_isListening = false;
}
bool isListening() const
{
return _isListening;
}
private:
EventBus::Listener _listener;
bool _isListening = false;
};
class WindowCloseListener : public MyListener<WindowCloseListener, WindowCloseEvent>
{
public:
void callback(const WindowCloseEvent& event)
{
// Logic specific for this event
std::cout << "WindowClose" << std::endl;
}
};
int main(int argc, char* argv[])
{
auto eventBus = std::make_shared<EventBus>();
WindowCloseListener listener{eventBus};
eventBus->postpone(WindowCloseEvent{});
eventBus->process();
if(false /*you decide when*/ && listener.isListening() == false)
{
listener.listen();
}
eventBus->postpone(WindowCloseEvent{});
eventBus->process();
if(true && listener.isListening() == false)
{
listener.listen();
}
eventBus->postpone(WindowCloseEvent{});
eventBus->process();
if(listener.isListening())
{
//...
}
...
}
Just example.
And of course you may add some bool flag was it called or not. You are right that I might expose isListening as public interface. Thank you for suggestion.
So as example above, you may still create multiple "MyListener or WindowCloseListener" and have multiple handlers.
Thanks! This is almost what i am currently using, but not quite. It would be pretty much impossible to have a listener class for every event i have as this defies the point of having the event bus in the first place. I currently have one bus and one listener attached to each other, and then I simply call listen() on the listener object for every event i want to catch. This is why i can't set a bool variable as the listener is used for an arbitrary amount of events. This is why i use typeid(T).name(), as this is a string-representation of the type, allowing me to differentiate between types without having a variable for each and every one.
Here are a few code snippets from my usage:
std::vector<b::string> m_registeredEvents;
std::shared_ptr<b::event_bus> m_eventbus;
b::event_listener m_eventListener;b::event_listener and b::event_bus are aliases to EventBus, to better fit into my library.
template<typename T, typename... TArgs>
bool dispatchEvent(TArgs&&... args) {
if (std::find(m_registeredEvents.begin(), m_registeredEvents.end(), typeid(T).name()) == m_registeredEvents.end()) {
return false;
}
m_eventbus->postpone<T>({ std::forward<TArgs>(args)... });
m_eventbus->process();
return true;
}
template<typename T>
void listenEvent(std::function<void(const T&)> listener) {
m_eventListener.listenToCallback(listener);
m_registeredEvents.emplace_back(typeid(T).name());
}sf::Event event {};
while (m_sfmlWindow.pollEvent(event)) {
ImGui::SFML::ProcessEvent(m_sfmlWindow, event);
switch (event.type) { // a gazillion events are dispatched here
case sf::Event::Closed:
if (!dispatchEvent<b::events::WindowCloseEvent>()) {
m_sfmlWindow.close();
}
break;
.....As you can see, this is what allows me to call m_sfmlWindow.close() if no handler is attached, and just the handler if one is attached (where the user can call m_sfmlWindow.close() or not, based on a condition). This is done for like 30 different event types with one event bus and one listener.
It does work perfectly like this, but having isListening() would make everything a lot more conscise.
Checkout ProtectedEventStream this is why there is limitation 1 listener can have 1 event handler.
Each Listener just holds single ID.
Of course this lib allows to add your own implementation of EventStream sure no wants to do this :).
In my opinion your approach is correct and I should expose isListening in future.
For now what I could just recommend to inherit/compose from EventBus::Listener and just add there your map of tracked state of listened events. In future it will be more easy to upgrade as such interface such show up in Listener (Listener::isListening()).
Thank you that you describe here your use case. I will add it as example and consider best approach I can find.
Just hint as typeid(T).name() may be not unique if I remember correctly. So just be safe with that and double check. Maybe use just hash_code?
No problem, i did not mean that the limitation of 1 listener is bad, i just said it because it was the most apparent difference i found when comparing it to the other library. It's a difference that i noticed, that unfortunately does not help in my situation. The rest is mostly identical, the other one is just another implementation and a bit more complicated.
Anyways, i just looked into it and per cppreference.com, both typeid(T).name() and hash_code have the exact same implications: Both only guarantee that an invocation with the same type always produces the same result. But both might produce the same result with different types and are completely implementation defined, and both might produce different results between invocations of the same program, it is only the same throughout 1 lifetime of 1 process. MSVC produces human-readable names, GCC does not. So, actually both are flawed for this purpose.
As i currently use mainly MSVC which prints human-readable type names (eg. "struct WindowCloseEvent") and i use them with distinct unique names, i think it should not be an issue most of the time. No need to change anything on my side, i will keep my implementation just like it is until isListening is supported ;)
(This is currently the only place where this library is used)
Thank you for your time, i really apprechiate it!
Limitation is only because I wanted to make it simple with RAII and avoid mistakes by someone with listen-unlisten match count. I will improve soon. Thanks. Give ma 1-2 weeks for that.
This maybe will interest you: https://github.com/gelldur/gcpp/blob/master/src/gcpp/type_name.h
Ah, very interesting, i saw such an implementation many times but never wanted to use it. is it cross-platform on MSVC and on GCC/Clang on Linux? I assume it if does work on all platforms and compilers, it does not produce equivalent results, just unique ones, right?
In the best case i would like perfectly reproducable results on any platform or compiler, but i don't think this is possible without complete compiler-like parsing
I mostly used it for debugging so far it was ok.
Thank you, i still have to try this when i get back to it
This works perfectly, it simplifies my code significantly. Thank you so much for the change!