GUI children isVisible not set with parent - HEAD suggestion

#1 Duh, thanks for the fix Entr0py! :)

#2 GUI controls already inherit visiblity. It just doesn't set the visible field for them. When a GUI control is a child to another, and that parent control turns off visiblity it is skipped in the rendering process and thus all children are skipped along with it.

#3 Still helps to have the flag set, so you don't have to check its parent(s) to check if the control's visible.

#4 The idea of easily checking the child's visiblity is good, but I have one thought on this.

This is only speculation, I haven't tried it out, so maybe this isn't the case. What happens if you have GUI B, that has controls C, D, E, F, and they have different combinations of visiblity depending on where your game is? Then, if GUI B's visibility is set to false, and then it sets C, D, E and F to off, when you turn GUI B back on, how will it know which one of its children are supposed to be visible?

I think that is the point of letting each child retain its visiblity flag. Just a thought... Maybe instead have another bit that stores whether it is being rendered or not, as opposed to changing the visibility bit which stores whether it *will* render?

- Drew

#5 Yeah I think its usefull for the children to retain the visibility flag even if they are not visible because a parent isn't visible, that way you can have non visible sub guis of non visible guis

#6 Matthew and Drew's assesment is correct - while having a parent flag set propagate down to children might be appropriate for some projects, in general it's nice not to have your visibility info get clobbered. So in HEAD it'll stay that way - but I'm glad Entr0py brought this up, and posted code on the topic. It's good to revisit these design decisions from time to time, and to explore alternate solutions.

#7 I figure its easier to just make a seperate group if you want something visible and something not. I prefer to not have to go through a million children setting their vis by hand ;)

If I run into a situation where I actually need to retain the original settings I will just throw a bool on there for recursive isVisible.

#8 I'm for retaining the visibility flag. I find it more sane and less tampering to check for parent visibility if I really need to know when a GUI is visible or not, by improving the GuiControl::isVisible() method or writing an alternative version of it, that recursively queries the parent GUI's visibility until it hits an invisible control and returns FALSE, or returns TRUE if no invisible controls were found up in the hierarchy.

Pseudocode:

bool GuiControl::isReallyVisible() 
{
   if (!mVisible)
      return false;

   if (mParent != NULL)
      if (!mParent->isReallyVisible())
         return false;

   return true;   
}

#9 There might already be a way to figure that out incidentally. Not sure, I don't do much GUI programming.

#10 Manoel : I believe your pseudocode is a great idea because knowing a controls visibility (without corrupting it's flag) based on it's parent is nice, I've gone ahead and worked up a small solution.

in GuiControl.h around line 206 change

inline bool isVisible() { return mVisible; } ///< Returns true if the object is visible

to

bool isVisible(); ///< Returns true if the object is visible

then in GuiControl.cc around line 1258 under setVisible add the following

bool GuiControl::isVisible()
{
   if( !mVisible )
      return false;


   // Check to see if a parent is not visible, in which case this will not be
   GuiControl *parent = getParent();
   

   if( parent && parent->isVisible() == false )
      return false;
   
   return mVisible;
}

It should be noted that it is not necessary to traverse all the parents of this control because the subsequent calls to parent->isVisible will do this for us.

This should be a suitable solution and will maintain the integrity of the visible flags on all controls while providing a more accurate representation of whether a control is actually visible on the canvas.

edit : It should also be noted that I did not test this code aside from making sure it compiles, but it should work fine, I will wait for someone to test it for confirmation before suggesting its inclusion in 1.4.

Regards,
Justin'

#11 Heh, the pseudocode was almost there.

I sugested creating a different method because I tought of a situation where changing the isVisible() behavior could break some things:

I got a GUI layed out like this:

- A is B's parent
- A is invisible
- B is visible

Then I do the following in script:

toggleB();
   A.setVisible(true);

(toggleB() toggles B's visibility on and off based on B.isVisible())

With the original behavior, B would be insivible (it was toggled off).
With the new behavior, B would be visible, because calling toggleB() while A is invisible would always change B's visibility to true (because isVisible() would always return false).

A real-world example would me information layers over a map that overlays the PlayGui, where both the map as a whole and individual layers could be turned on/off.

Of course isReallyVisible() sounds ridiculous, but I'm sure there might be other ways ;)