How should I rebuild this event handling code?
I've been reading a few C ++ books lately (Sutters, Meyers) that encouraged me to use smart pointers (and object destruction in general) more efficiently. But now I don't know how to fix what I have. Specifically, I now have an IntroScene class that inherits from both Scene and InputListener.
The scene is really not up to date, but the InputListener subscribes to the InputManager on build, and fails again to destroy.
class IntroScene : public sfg::Scene, public sfg::InputListener {
/*structors, inherited methods*/
virtual bool OnEvent(sf::Event&) override; //inputlistener
}
But now, if the input dispatcher dispatches events to the scene and the scene decides to replace itself because of this, I have a function working on an object that no longer exists.
bool IntroScene::OnEvent(sf::Event& a_Event) {
if (a_Event.type == sf::Event::MouseButtonPressed) {
sfg::Game::Get()->SceneMgr()->Replace(ScenePtr(new IntroScene()));
} //here the returned smartpointer kills the scene/listener
}
Side question: Is it important? I searched for it, but did not find a definite yes or no. I know 100% of the destroyed object no methods are called after it is destroyed. I can keep the return value of Replace () until the end of the OnEvent () method if I need to.
The real problem is the InputListener
InputListener::InputListener() {
Game::Get()->InputMgr()->Subscribe(this);
}
InputListener::~InputListener() {
if (m_Manager) m_Manager->Unsubscribe(this);
}
as it is called during OnEvent () which is called by InputManager during HandleEvents ()
void InputManager::HandleEvents(EventQueue& a_Events) const {
while (!a_Events.empty()) {
sf::Event& e = a_Events.front();
for (auto& listener : m_Listeners) {
if (listener->OnEvent(e)) //swallow event
break;
}
a_Events.pop();
}
void InputManager::Subscribe(InputListener* a_Listener) {
m_Listeners.insert(a_Listener);
a_Listener->m_Manager = this;
}
void InputManager::Unsubscribe(InputListener* a_Listener) {
m_Listeners.erase(a_Listener);
a_Listener->m_Manager = nullptr;
}
So, when a new Scene + Listener is created and when the old one is destroyed, the m_Listeners list changes during the loop. So the case breaks down. I was thinking about setting a flag when starting and stopping a loop and saving (un) subscriptions that happen when it is set in a separate list, and handle that after. But it does feel a little hacked.
So how can I actually reverse engineer this correctly to prevent situations like this? Thanks in advance.
EDIT, Solution: I ended up with loop flags and deferred list (inetknight's answer below) for subscription only, as it can be safely done later.
Non-subscriptions should be processed immediately, so instead of storing the original pointers, I store a mutable pointer (bool) pair (mutable since the set returns a const_iterator). I set bool to false when this happens and check it in the event loop (see dave's comment below). Not sure if this is the cleanest possible solution, but it works like a charm. Thanks a lot guys
source to share
Side question: Is it important? I searched for it, but did not find a definite yes or no. I know that 100% no methods are called on the destroyed object after it is destroyed. I can keep the return value of Replace () until the end of the OnEvent () method if I need to.
If you know that 100% of the methods are not called, it destroys the object, and none of its member variables are available, then that is safe. Whether it is intended or not is up to you.
You may have a different list of objects that were requested to be un / signed. Then after you tell everyone in the event list, you then process the un / subscription request list before moving on to the next event.
/* this should be a member of InputManager however you did not provide a class definition */
typedef std::pair<InputListener *, bool> SubscriptionRequest;
bool handleEventsActive = false;
std::vector<SubscriptionRequest> deferredSubscriptionRequests;
void InputManager::HandleEvents(EventQueue& a_Events) const {
// process events
handleEventsActive = true;
while (!a_Events.empty()) {
sf::Event& e = a_Events.front();
for (auto& listener : m_Listeners)
{
//swallow event
if (listener->OnEvent(e)) {
break;
}
}
a_Events.pop();
// process deferred subscription requests occurred during event
while ( not deferredSubscriptionRequests.empty() ) {
SubscriptionRequest request = deferredSubscriptionRequests.back();
deferredSubscriptionRequests.pop_back();
DoSubscriptionRequest(request);
}
}
handleEventsActive = false;
}
void InputManager::DoSubscriptionRequest(SubscriptionRequest &request) {
if ( request.second ) {
m_Listeners.insert(request.first);
request.first->m_Manager = this;
} else {
m_Listeners.erase(request.first);
request.first->m_Manager = nullptr;
}
}
void InputManager::Subscribe(InputListener* a_Listener)
{
SubscriptionRequest request{a_Listener, true};
if ( handleEventsActive ) {
deferredSubscriptionRequests.push_back(request);
} else {
DoSubscriptionRequest(request);
}
}
void InputManager::Unsubscribe(InputListener* a_Listener)
{
SubscriptionRequest request{a_Listener, false};
if ( handleEventsActive ) {
deferredSubscriptionRequests.push_back(request);
} else {
DoSubscriptionRequest(request);
}
}
source to share