Bug with forgotten SEvent::GUIEvent::Element = 0

If you are a new Irrlicht Engine user, and have a newbie-question, this is the forum for you. You may also post general programming questions here.

Bug with forgotten SEvent::GUIEvent::Element = 0

Postby greenya » Mon May 31, 2010 12:42 am

Hello!

/* Please confirm that this is a bug */

When we working with SEvent, we check EventType first, and then we work with proper part of the event. For example:
Code: Select all
if (event.EventType == EET_GUI_EVENT)
{
       // here we can use/check event.GUIEvent freely
}


SEvent::SGUIEvent contains:
Code: Select all
gui::IGUIElement * Caller; // IGUIElement who called the event.
gui::IGUIElement * Element; // If the event has something to do with another element, it will be held here.
gui::EGUI_EVENT_TYPE EventType; // Type of GUI Event.


Problem with Element!
In most cases it filled correctly: with a pointer if used and 0 if not used, BUT there is few places where this behavior omitted and this field left without attention.

Please note, currently we cannot write next:
Code: Select all
if (event.EventType == EET_GUI_EVENT)
{
       s32 callerID = event.GUIEvent.Caller->getID();
       s32 elementID = event.GUIEvent.Element != 0 ? event.GUIEvent.Element->getID() : -1;
}

because assignment of "elementID" gives AccessViolationException only for next gui events:
EGET_ELEMENT_LEFT
EGET_TABLE_HEADER_CHANGED
EGET_TABLE_CHANGED
EGET_TABLE_SELECTED_AGAIN
EGET_TREEVIEW_NODE_EXPAND
EGET_TREEVIEW_NODE_COLLAPS
EGET_TREEVIEW_NODE_DESELECT
EGET_TREEVIEW_NODE_SELECT


Next list of places shows where bug is:

CGUIEnvironment.cpp:
Code: Select all
   ...
   if (Hovered != lastHovered)
   {
      SEvent event;
      event.EventType = EET_GUI_EVENT;

      if (lastHovered)
      {
         event.GUIEvent.Caller = lastHovered;
         event.GUIEvent.EventType = EGET_ELEMENT_LEFT;
         lastHovered->OnEvent(event); // <<<<<<<<<<<<<<<<<---- Element IS NOT FILLED HERE!!!
      }

      if ( Hovered )
      {
         event.GUIEvent.Caller  = Hovered;
         event.GUIEvent.Element = Hovered;
         event.GUIEvent.EventType = EGET_ELEMENT_HOVERED;
         Hovered->OnEvent(event);
      }
   }
   ...


CGUITable.cpp
Code: Select all
   ...
   case EGCO_CUSTOM:
      CurrentOrdering = EGOM_NONE;
      if (Parent)
      {
         SEvent event;
         event.EventType = EET_GUI_EVENT;
         event.GUIEvent.Caller = this;
         event.GUIEvent.EventType = EGET_TABLE_HEADER_CHANGED;
         Parent->OnEvent(event); // <<<<<<<<<<<<<<<<<---- Element IS NOT FILLED HERE!!!
      }

      break;

   ...

   if (changed)
   {
      SEvent event;
      event.EventType = EET_GUI_EVENT;
      event.GUIEvent.Caller = this;
      event.GUIEvent.EventType = EGET_TABLE_HEADER_CHANGED;
      Parent->OnEvent(event); // <<<<<<<<<<<<<<<<<---- Element IS NOT FILLED HERE!!!
   }

   ...

   // post the news
   if (Parent && !onlyHover)
   {
      SEvent event;
      event.EventType = EET_GUI_EVENT;
      event.GUIEvent.Caller = this;
      event.GUIEvent.EventType = (Selected != oldSelected) ? EGET_TABLE_CHANGED : EGET_TABLE_SELECTED_AGAIN;
      Parent->OnEvent(event); // <<<<<<<<<<<<<<<<<---- Element IS NOT FILLED HERE!!!
   }

   ...


CGUITreeView.cpp
Code: Select all
   ...
   SEvent               event;

   event.EventType         = EET_GUI_EVENT;
   event.GUIEvent.Caller   = this;
                                       // <<<<<<<<<<<<<<<<< event.GUIEvent.Element = 0; <<<<<< MUST BE HERE!!!
   xpos -= AbsoluteRect.UpperLeftCorner.X;
   ypos -= AbsoluteRect.UpperLeftCorner.Y;
   ...


All this places were found by searching pattern "GUIEvent.Caller" across all *.cpp and *.h files in "source" directory.
At all this places should be done next patch: after each "event.GUIEvent.Caller = " line, should be added line "event.GUIEvent.Element = 0;".


Thank You!
greenya
 
Posts: 1007
Joined: Sun Jan 21, 2007 1:46 pm
Location: Ukraine

Postby CuteAlien » Mon May 31, 2010 1:25 am

Mhm, I think the original idea was just to set that when it's used. But yeah - it's better to initialize it.

Thanks for finding all the places, I'll try to find time for it in the next days (a little too late already today for me, stupid body need a few hours sleep).
IRC: #irrlicht on irc.freenode.net
Code snippets, patches&stuff: http://www.michaelzeilfelder.de/irrlicht.htm
Free racer created with Irrlicht: http://www.irrgheist.com/hcraftsource.htm
User avatar
CuteAlien
Admin
 
Posts: 8295
Joined: Mon Mar 06, 2006 2:25 pm
Location: Tübingen, Germany

Postby greenya » Mon May 31, 2010 6:04 am

CuteAlien wrote:Mhm, I think the original idea was just to set that when it's used.


Here is an example of correct filling of Element:
Code: Select all
      SEvent e;
      e.EventType = EET_GUI_EVENT;
      e.GUIEvent.Caller = Focus;
      e.GUIEvent.Element = 0;
      e.GUIEvent.EventType = EGET_ELEMENT_FOCUS_LOST;

Element not used for EGET_ELEMENT_FOCUS_LOST, but its set 0, so it will not contain garbage.

I think there are 2 correct ways:
- simply set Element to 0;
OR
- write a note in documentation on Element with list of GUI event types where it actually used, since user cannot check is it filled correctly or not.
greenya
 
Posts: 1007
Joined: Sun Jan 21, 2007 1:46 pm
Location: Ukraine

Postby CuteAlien » Mon May 31, 2010 8:48 am

Yeah, don't worry, I will change it. Although improving event documentation has also been on my todo already for a long time. Some day I'll get to it (basically each gui-element should document which events it does use, nothing tricky - I just didn't get to it yet).
IRC: #irrlicht on irc.freenode.net
Code snippets, patches&stuff: http://www.michaelzeilfelder.de/irrlicht.htm
Free racer created with Irrlicht: http://www.irrgheist.com/hcraftsource.htm
User avatar
CuteAlien
Admin
 
Posts: 8295
Joined: Mon Mar 06, 2006 2:25 pm
Location: Tübingen, Germany

Postby CuteAlien » Sat Jun 05, 2010 8:39 pm

I've fixed it in the 1.7 branch, svn trunk will follow on next merge.
IRC: #irrlicht on irc.freenode.net
Code snippets, patches&stuff: http://www.michaelzeilfelder.de/irrlicht.htm
Free racer created with Irrlicht: http://www.irrgheist.com/hcraftsource.htm
User avatar
CuteAlien
Admin
 
Posts: 8295
Joined: Mon Mar 06, 2006 2:25 pm
Location: Tübingen, Germany

Postby greenya » Sun Jun 06, 2010 9:16 am

Thank you :!:
greenya
 
Posts: 1007
Joined: Sun Jan 21, 2007 1:46 pm
Location: Ukraine


Return to Beginners Help

Who is online

Users browsing this forum: No registered users and 1 guest