Jump to content
  • Advertisement

Archived

This topic is now archived and is closed to further replies.

HellRiZZer

OpenGL GUI in C++ - C&C please..

This topic is 5181 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Advertisement
Hi there!

"All Comments and Critiques are welcome, no matter how harsh they are."
Well I'm always full of critique, but I'll try to keep it non-harsh

This article series was especially interesting for me because I'm also writing a GUI lib for our game.

Notice, I'm no professional in any way. I've been struggling with my GUI lib for 2 months now, so my thoughts here are thoughts that been in my head for some time

Something that occurs in all your code. You use NULL, and C++. Actually in C++ you should use 0, but I guess if you think it's more clear with NULL then I'm with ya. Just be carefull about teaching new coders about old habits.


[ GUI TUTORIAL, PART 1 ]

-1-

First of, I too had a GUI object base class. Which I called cGUIObject. Then it came to my attention that those have a more common name, namely widget . So now my base class is called cWidget.


-2-

I don't know about saying that you're always a quad, if so: how do you think about texts? Dynamic and static labels.
(Edit: Oki, you bring that up in part 4. But then I have to dissagre. What if you want the text floating without a background quad? As if you want the text to be a header on a window? Meaning a window object is the parent to a text, and other elements)


-3-

It's not that bad, but... on CGUIElement::OnDraw() you initialize x1,y1,etc with 0, and then give a value. Why not just initialize with your value? For readability you can always use newlines. No matter


-4-

CGUIElement::OnDraw(). Code missuse, glVertex3f() where you pass int's... you should be using glVertex3i() instead.


[ GUI TUTORIAL, PART 2 ]

-1-

I've made a TinyXML wrapper actually, it's quite nice. The loading rutine of CGUIElement isn't something I would recommend people to write, since there's gonna be alot of duplicate code in different parts of the game where XML is read. But this is a small tutorial so...
A) don't know if you should be teaching how to parse with TinyXML or not.
B) don't know if you should use (and supply in the download) a TinyXML wrapper or not.
C) anything that makes your tutorial go into non-topic issues (like XML parsing) is proabably gonna scare away newbies.
Yes I'm aware of your warning in the beginning, but then again. I would recommend you write a simple wrapper helper, smaller == better. Write a small tutorial, smaller == better. And refer to that instead. Either way, you're gonna force readers to learn something other then GUI structure design...

-2-

I have also thought about truncated XML elements, like when you want default values. It's
A) great when you're writing a XML in notepad
B) useless when you're saving the object to XML, because you have to check if the value is a default value and then not write it to XML. what exactly do you win? smaller .xml files?
C) makes your XML file inconsistent.


-3-

Yet again, calling glVertex3f() and passing int's.


-4-

More OpenGL code missuse. Calling glTexCoord2d() using floats, instead of calling glTexCoord2f(). (Note: Your first CGUIElement::Draw() uses glTexCoord2d() and your second uses the correct glTexCoor2f(). Meaning you've changed your code without telling your reader about it )


[ GUI TUTORIAL, PART 3 ]

-1-

Just didn't like all the singletons


[ GUI TUTORIAL, PART 4 ]

-1-

header: "Font XML data details"
text: "The font has the type by which we can idenity it...". You mean identify it?

Haven't read so close, so I suggest you just pass your text through a spell checker. Misspelled tutorials... well...


-2-

CGUIStatic::DrawText()
A) I'd use a enum or something for the m_iTextHAlign and m_iTextVAlign checks instead of 1,2,3.
B) Why are there no else-statements?
C) I would go with to switch statements though, like so:
switch(m_iTextVAlign)
{
case LEFT : /* code */
break;
case CENTER : /* code */
break;
case RIGHT : /* code */
break;
}
and the same for m_iTextHAlign.


-3-

CGUIStatic::DrawText()
{
if(!(m_strText == NULL || strlen(m_strText) == 0))
...
}

Hmm, never seen that kind of check before. Whate is it you're looking for? Because if you've already checked that the pointer isn't null, you're going to check the length of the string. Which is a O(n) (linear time) operation.



[ GUI TUTORIAL, PART 5 ]

-1-

CGUIButton::OnDraw(), in the innermost if(). m_fTexCoord[2].x and .y. You don't cast the left part of the division to float, like you've done for all the other m_fTexCoord's. Just found it inconsistent.



< /rant >

I didn't mention the good parts, because you don't have to reconsider those But I did like the way you setup the tutorials and there's not much blabbering, which is great.

I did miss another thing though, in the end where you normally would encourage the reader to expand your structure by adding ScrollBars, Radio buttons, Check boxes, and what not (brag about how flexible and great your design is ).

Well that's all for now... Good night!
*goes of to bed*

(Edit: formating)

}-- Programmer/Gamer/Dreamer --{
I didn't choose to code, coding choose me.


[edited by - seriema on June 9, 2004 12:51:41 AM]

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
quote:

Something that occurs in all your code. You use NULL, and C++. Actually in C++ you should use 0, but I guess if you think it''s more clear with NULL then I''m with ya. Just be carefull about teaching new coders about old habits.




Not to pick or anything, but I''m interested in where this came from... I''ve used both NULL and 0 with pointers and must admit I prefer NULL. I was just curious how using the define constant NULL was a bad habit since it is defined in c++ as 0. Just curious...

Share this post


Link to post
Share on other sites
delete'ing a pointer to 0 is guaranted to be safe by the standard, i.e.
int* i = 0;
delete i;

is perfectly safe. whilst NULL nowadays normally just is a macro for 0, it's not guaranted by something as grand as the ANSI C++ standard committe. in other words, NULL might be -1 (or whatever). so this is NOT guaranted to be safe:
int* i = NULL;
delete i;

(edit: "safe" as in, not undefined behaviour. i.e. it won't crash your program)

}-- Programmer/Gamer/Dreamer --{
I didn't choose to code, coding choose me.

[edited by - seriema on June 10, 2004 6:39:09 AM]

Share this post


Link to post
Share on other sites
I did not really read them but the code looks great and these are the only GUI tutorial I have found for OpenGL :O !!!
Is it possible to ask Nehe to host them too ? Just saying that because that way more people will read it

[edited by - vbtest on June 10, 2004 6:49:03 AM]

Share this post


Link to post
Share on other sites
vbtest, well _not_ reading them is _not_ gonna help HellRiZZer _not_ getting flamed after posting the tutorials here on gamedev

HellRiZZer, me and a classmate have been discussing your articles.

First , we really feel that the XML loading doesn''t belong in this tutorial series. Sure, it''s cool to use XML. All the hip-kids, including myself, use it. But forcing that XML parsing code on a new coder, while he''s trying to learn GUI (and not XML parsing using TinyXML), is not nice.

1) don''t. just don''t. don''t write your gigantic parsing code in a GUI tutorial. you can supply it in the code if you want to.
2) let the reader make his widgets from code, and not a file. let the reader decide what file type he thinks is best
3) tell the reader that if he writes a save()/load() rutine it should first save/load itself and then it''s children. I implemented this using the template method ([GOF95] p. 325). meaning:
cWidget::Save()
{
SaveThis(); // template method, a function that holds the actual implementation code but delegating the implementation to the subclass. I like to make them protected pure virtual.
for( all children )
child->Save();
}
4) don''t write the parsing code in the tutorial, but take a small paragraph talking about what the reader should think about when implementing a save()/load(). and then finish the paragraph with "I''ve made my use XML, check the source". Just make it easy for the reader to make his own parsing, if he wants to make one.


Second , please supply some screenshots. Try to motivate the reader a bit. Eye candy is always good.

}-- Programmer/Gamer/Dreamer --{
I didn''t choose to code, coding choose me.

Share this post


Link to post
Share on other sites
Just did a skim through. You might want to add links to the other parts at the end of each tutorial. Now you have to go back before you can go to the next tutorial.

And I agree with Siriema on the XML stuff.

Share this post


Link to post
Share on other sites
cut away the xml-parts. You have made a gui-tutorial, stick to it

[edited by - dathui on June 10, 2004 4:56:39 PM]

Share this post


Link to post
Share on other sites
I''m kindly thanking everyone for responding to my post. You have gave me a great deal of information. So here is my response:
1. Write a wrapper - Great idea, will do!
2. Do not use XML - but what the user would store the information in, plain text file? I came from that (in earlier versions in was plain old text file) which I''ve found a little... um, not good.
Secondly, XML is structured, so it would be easy even for beginners to write it using plain old Notepad and preview it in Internet Explorer.
3. Make the user to create code, not XML - that is what I have now. You can easily go:


CGUIStatic *static = new CGUIStatic;
static->SetRect(tRect(0,0,100,100));
static->SetBackgroundMaterial(pointer_to_material);
static->GetFont()->Parse(NULL, "font_file.xml");
static->SetText("Here is my go");


4. Write a small paragraph about saving/loading - will do, good idea!

5.
quote:

I don''t know about saying that you''re always a quad, if so: how do you think about texts? Dynamic and static labels.
(Edit: Oki, you bring that up in part 4. But then I have to dissagre. What if you want the text floating without a background quad? As if you want the text to be a header on a window? Meaning a window object is the parent to a text, and other elements)



Well, for that purpose I will introduce m_bVisible variable in CGUIElement (it will be covered in next tutorials) that will not draw the element, but will draw it''s children (including text).

6. Spellchecking - I tried, but Dreamweaver had lots of problems with source code words.

7. Add ScrollBars, ProgressBars, Radio Buttons, ListBoxes - will be covered in next tutorials (in fact, my next tutorial is radio buttons).

8. Little bits & pieces in code - I will correct them.

Thanks again to everyone that has responded. I''ve learned a great deal about what am I doing right or not.

" Do we need us? "


Ionware Productions - Games and Game Tools Development

Share this post


Link to post
Share on other sites

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!