[Solve] Events raised on shutdown (CEGUI 0.8.5) (Bug?)

[cyberjunk]

Hey guys,

I've been using CEGUI in my game project for quite a while now. Very nice UI solution.
I recently upgraded from 0.8.3 to 0.8.5 and I was faced with some weird issue:

When shutting down CEGUI (destroy()/destroySystem()), it seems some of my subscribed event handlers get raised one more time.
This is particularly true for the "Deactivated" and the related "TextAccepted" events on the Editbox class (in my case stored in List-Items).

I had to add a call to "removeAllEvents()" into the destroy() of Window class to "fix" this for all cases.
The other solution was to manually "unregister" all subscribed events on my cegui-instances before shutting down CEGUI...

See this commit on my repo:
https://github.com/cyberjunk/meridian59 ... 3f4d84f5c8

Note: This commit also includes another fix from your repo. The important line is only the "removeAllEvents()".
(Other changes from: https://bitbucket.org/cegui/cegui/commi ... 1eca12aba5)

Is this a bug? Or are users supposed to manually unregister all events before shutting down?

[cyberjunk]

PS: I don't think this is a good fix above, because there are clearly possible events raised in that code after I remove them all now...

May be this is some kind of design-change?
Do I need to manually unregister event-handlers on ui-controls, which I don't want to run on shutdown anymore?

Also this seems to be happen only with some of my ui-elements...

[cyberjunk]

Since I only had trouble with the "deactivate" event on the shutdown, I found a less invasive change to disable deactivate and textaccepted events during shutdown: https://github.com/cyberjunk/meridian59 ... 331e20f7ea

[Ident]

This is a bug, introduced due to a fix I made for some other issue.

I am looking into it, your github and bitbucket commits are very helpful

[Ident]

This is quite tricky to resolve without breaking more

Can you try this?

Code: Select all

void Window::removeChild_impl(Element* element)
{
    Window* wnd = static_cast<Window*>(element);

    Window* const capture_wnd = getCaptureWindow();
    if ((capture_wnd && wnd) &&
        (capture_wnd == wnd || capture_wnd->isAncestor(wnd)))
            getCaptureWindow()->releaseInput();

    // remove from draw list
    removeWindowFromDrawList(*wnd);

    Element::removeChild_impl(wnd);
   
    // find this window in the child list
    const ChildList::iterator position =
        std::find(d_children.begin(), d_children.end(), wnd);

    // if the window was found in the child list
    if (position != d_children.end())
    {
        // unban properties window could write as a root window
        wnd->unbanPropertyFromXML(RestoreOldCapturePropertyName);
    }
   
    wnd->onZChange_impl();

    // Removed windows should not be active anymore (they are not attached
    // to anything so this would not make sense)
    if(wnd->isActive())
    {
        wnd->deactivate();
    }
}


and

Code: Select all

//----------------------------------------------------------------------------//
void Window::destroy(void)
{
    // because we know that people do not read the API ref properly,
    // here is some protection to ensure that WindowManager does the
    // destruction and not anyone else.
    WindowManager& wmgr = WindowManager::getSingleton();

    if (wmgr.isAlive(this))
    {
        wmgr.destroyWindow(this);

        // now return, the rest of what we need to do will happen
        // once WindowManager re-calls this method.
        return;
    }

    // signal our imminent destruction
    WindowEventArgs args(this);
    onDestructionStarted(args);

    // Check we are detached from parent
    if (d_parent)
        d_parent->removeChild(this);

    releaseInput();

    // let go of the tooltip if we have it
    Tooltip* const tip = getTooltip();
    if (tip && tip->getTargetWindow()==this)
        tip->setTargetWindow(0);

    // ensure custom tooltip is cleaned up
    setTooltip(static_cast<Tooltip*>(0));
   


    // clean up looknfeel related things
    if (!d_lookName.empty())
    {
        d_windowRenderer->onLookNFeelUnassigned();
        WidgetLookManager::getSingleton().getWidgetLook(d_lookName).
            cleanUpWidget(*this);
    }

    // free any assigned WindowRenderer
    if (d_windowRenderer != 0)
    {
        d_windowRenderer->onDetach();
        WindowRendererManager::getSingleton().
            destroyWindowRenderer(d_windowRenderer);
        d_windowRenderer = 0;
    }

    cleanupChildren();

    releaseRenderingWindow();
    invalidate();
}


The latter just seems in better order to me, I dont expect a direct effect on your issue but this one was just recently changed. The first part is the important one: it should then only fire deactivation events on the very few affected windows, and that is acceptable and proper behaviour imo. My logic here is: How can a window be active it is not attached to anything?

And yes, a removed window being active has lead to issues in certain use-cases.

Regarding the TextAccepted: any idea why this is fired?

[YaronCT]

In a test I've made a "TextAccepted" event isn't fired.

[Ident]

Too bad cyberjunk didn't reply to verify, but I guess this is solved so I will mark it as such.