getProperty should be local-only

If you found a bug in our library or on our website, please report it in this section. In this forum you can also make concrete suggestions or feature requests.

Moderators: CEGUI MVP, CEGUI Team

Jamarr
CEGUI MVP
CEGUI MVP
Posts: 812
Joined: Tue Jun 03, 2008 23:59
Location: USA

getProperty should be local-only

Postby Jamarr » Wed Feb 16, 2011 00:09

I ran into an issue in v0.6.2 (this same issue exists in v0.7.5). I noticed that calling Window::getProperty("Visible") results in a call to Window::isVisible(). The problem is that isVisible has a default parameter bool localOnly = false which determines if the method limits the check to the local-window or to include the parent (and recursively the entire ancestry) for visibility; obviously this default-parameter results in checking the parent. In my opinion this is not the expected behavior when querying a property on a window - it should instead return the windows local/actual property value. The current behavior makes it impossible to query a sub-windows actual visible state via getProperty.

For context, I created a PropertyLinkDefinition for FrameWindow>Titlebar>Visible to represent a special state for the parent FrameWindow. When accessing this property while the FrameWindow is hidden, I noticed that the property was returning false, even though the actual property value was true.

Personally I think that property queries (and related methods) should be restricted to local-only. Any specialized queries (ex: isDisplayed) should reside in a separate method; having dual-purpose methods just leads to these obscure behavioral problems. In v0.6.2 I simply modified Visible::get in CEGUIWindowProperties.h to call isVisible(true) so the local-value is returned, however I think that refactoring this and similar methods/processes to avoid dual-purpose methods would be of greater benefit to CEGUI.

v0.7.5
CEGUIWindow.h: Line 567
CEGUIWindowProperties.h: Line 202
If somebody helps you by replying to your thread, upvote him/her as a thanks! Make sure to include your CEGUI.log and everything you tried when posting! And remember that we are not magicians!

User avatar
CrazyEddie
CEGUI Project Lead
Posts: 6760
Joined: Wed Jan 12, 2005 12:06
Location: England
Contact:

Re: getProperty should be local-only

Postby CrazyEddie » Wed Feb 16, 2011 07:53

Yeah, I agree that this is bad. In fact these dual-purpose functions were a cause of a bug I fixed this past weekend.

We can look into changing this for 0.8.0, I think.

CE.


Return to “Bug Reports, Suggestions, Feature Requests”

Who is online

Users browsing this forum: No registered users and 2 guests