GUI API Suggestion

aleroy · Post by **aleroy** » Fri Apr 15, 2011 6:02 pm

{I didn't see a thread for suggestions, so I apologize if this post is misplaced}

I was surprised when my CGUIFileOpenDialog instance called IGUISkin::draw3DWindowBackground() ... (which caused a few headaches while refactoring my concrete skin).

I would expect CGUIFileOpenDialog to either inherit IGUIWindow (is-a), contain a window instance (has-a), or that it not--apparently--break encapsulation by calling IGUISkin::draw3DWindowBackground(). The function name and documentation insinuates that the function exclusively skins IGUIWindow instances. However, this is not true. Perhaps it is merely a result of the development lifetime rather than design.

In all honesty, it is a minor issue that could be improved (if not resolved) by updating the documentation. But, in my opinion the issue ought to be formally resolved in a future release.

{Edit: added proposed solution}
My proposed solution is to refactor CGUIFileDialog, removing the window behaviors:

Drawing methods (titlebar, window buttons, background, and borders)
Dragging behavior
Modal state

Thus, the dialog would simply be an IGUIElement with a few sub-elements and the specialized behaviors indicative of its name.

Such an implementation would adhere more closely to the OOP concept of encapsulation. Implementers would contain IGUIFileOpenDialog instances in an IGUIWindow instance ... or, for that matter, any instance of a class derived from IGUIElement!

Cheers,
~Andrew~

{ BTW, great job overall on the GUI API ... best 3D-Graphics Engine GUI implementation I've seen (to date)! }

CuteAlien · Post by **CuteAlien** » Sat Apr 16, 2011 7:32 am

Yeah, it's a little strange. I suppose derving from IGUIWindow could make sense maybe (not sure if there had been reasons not to that). Just removing the window functionality would make using it harder and break backward compatibility, so not sure if that is a good idea.

hybrid · Post by **hybrid** » Sat Apr 16, 2011 11:54 am

Well, refactoring into a FileOpenComponent and making FileOpenDialog a window containing the component would be an option.

aleroy · Post by **aleroy** » Sun Apr 17, 2011 5:03 pm

Upon further thought, my suggestion and Hybrid's input would only resolve part of the issue.

The real issue at hand is that non-window instances are calling IGUISkin::draw3DWindowBackground. I was having to exclusively check for concrete window instances in a function that, IMO, should only handle instances of IGUIWindow. When I'm drawing a window background, I ought to know that I'm dealing with a window. Although I can think of few case where you would want to draw a "fake" window background, that should have its own function (again, my opinion).

As an example: Say you want a skin to center a window's titlebar text. To do this properly, you need know which titlebar icons are visible (lest the text overlap). But if non-window elements are calling draw3DWindowBackground, you have to exclusively check that the element is a IGUIWindow instance and cast it before you can know anything about titlebar icons.

Which, as I said before, really isn't all that big an issue ... until you start implementing new IGUIWindow classes. Then, to make the above example work, you would have to add an exclusive type check in the draw3DWindowBackgroud skin function every time you implement a new window.

And that is issue that inspired this thread.

{EDIT: scratch that bit about windows-only in the draw window background. The only thing missing in IGUISkin::draw3DWindowBackground() are boolean parameters for whether each of the three titlebar icons is visible. Easy fix ... I would think}

aleroy · Post by **aleroy** » Mon Apr 18, 2011 11:21 am

Another suggestion/idea--on a totally different topic--is to provide and interface for adding/overriding element behavior using the Strategy Pattern. This would make defining custom behavior much simpler. Implementers would only have to write code for the bits they want to add or change.

The interface would simple declare executeAddedOnEvent() executeAddedDraw(), and executeOverriveOnEvent(), executeOverrideDraw(). A proper implementation would mean quite a bit of refactoring, though. Each function for all the default implementations would need to be refactored to support it. Also, the override bits could potentially compromise default and/or added behaviors in a bad bad way. So ... there are drawbacks.

But this is actually one of things I'm currently developing. I'm also working on creating more complex elements and components:

drop boxes, dock panels, accordion menus, slide-out panels
panels that can be picked-up and dragged out of- or into- another container
panels that can become windows, tabs, or accordion elements
a slew of in/out animators (IN: flock, gather, resolve, slide, zoom) (OUT: flock, scatter, dissolve, slide, zoom)
command/memento for undo/redo behavior
mediators and a few other support classes.

My focus is on developing GUI elements for toolset/level editors. I created a cinematic sequence tool before starting this project, and my interface wound-up being so horrid as to inspire this new project ... but I digress.

aleroy · Post by **aleroy** » Mon Apr 18, 2011 9:05 pm

Alright, it looks like I'm on a roll here ... Downhill ... and off the cliff-side and into the chasm with this triple post!

I'm all finished with my newbie, "Let me tell you how to make it better," suggestions. You're work is great as it is. I truly appreciate this excellent contribution to the community. Thanks.

I'll get back to you when I have some code to show off.

Cheers

ChaiRuiPeng · Post by **ChaiRuiPeng** » Tue Apr 19, 2011 12:05 am

you could have used the edit button to avoid triple posting

CuteAlien · Post by **CuteAlien** » Tue Apr 19, 2011 9:26 pm

Triple post is ok, then I see when he has more ideas :-)

Also this does not just affect the file-open dialog but also the color-select dialog (although that one is not working anyway so far).

If the file-open and color-select dialogs would be derived from IGUIWindow then this would automatically allow for checking the state of the buttons. But I think it also would be cleaner to pass an IGUIWindow* instead of an IGUIElement* then. But not sure right now if that makes sense in the internal implementation.

Flags are not so nice as you have to know then about the size of those buttons. Whenever those are changed I can bet it would be forgotten in the skin. So if some space should be used or not used by text it's probably better to pass that space directly as parameter I think.