API breaking consistency fixes intended for CEGUI 0.8.0

Discussion regarding the development of CEGUI itself - as opposed to questions about CEGUI usage that should be in the help forums.

Moderators: CEGUI MVP, CEGUI Team

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

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby CrazyEddie » Fri Oct 08, 2010 09:23

ok, I will admit to being 'swayed' a little towards renaming the namespace ;)

I will update the OP in another couple of weeks with some of the results of my considerations of many of the points mentioned above :)

CE.

User avatar
Kulik
CEGUI Team
Posts: 1382
Joined: Mon Jul 26, 2010 18:47
Location: Czech Republic
Contact:

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Kulik » Tue Oct 12, 2010 18:34

Regarding the namespace, standard libraries use lowercase but cegui::Window just looks hideous, doesn't it? :lol: I seriously don't care though.

There is one more tiny inconsistency: CEGUI::colour should be CEGUI::Colour because all classes are capitalised (even ColourRect).

One idea for the future:

Code: Select all

template<typename T>
inline T* WindowManager::createWindow(const String& name = "")
{
    return static_cast<T*>(createWindow(T::WidgetTypeName, name));
}


This currently will only work for DefaultWindow and other skinless windows. I want this to work with everything :-) What if user was able to select a default skin and thus allowing this? It makes creating new windows easier with less repetition... The old way of creating windows would definitely stay...

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

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby CrazyEddie » Wed Oct 13, 2010 09:27

I'm not opposed to this type of change. I think the more things can be simplified, streamlined and what have you, the better. I might even go so far as to say the existing system of factories could get replaced in favour a system like this - since it alleviates the need to even register those factories. However, that would have massive impact elsewhere, so that will need to be carefully considered.

A default skin is one idea, another is to pass in the name of the desired skin.

I'm also not opposed to totally re-designing the way skins are mapped...

CE.

User avatar
Mikademus
CEGUI MVP
CEGUI MVP
Posts: 130
Joined: Wed Mar 18, 2009 19:14

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Mikademus » Wed Oct 13, 2010 11:45

I like this discussion. Very constructive and creative. I also think Kulik's template suggestion is attractive and might contribute elegance to client code.

Kulik wrote:Regarding the namespace, standard libraries use lowercase but cegui::Window just looks hideous, doesn't it? :lol: I seriously don't care though.


Nah, not ugly but consistent:

a) cegui::Window is standard notation for a struct or class under a namespace
b) Cegui::window is standard notation for a static method under the Cegui class
c) Cegui::Window is standard notation for the inner class Window in the class Cegui
d) CEGUI::Window would probably be understood as the dependent class Window under the typedef or macro "CEGUI"

These conventions are not written into the Standard, but they are principally conventions of the industry- and language.

User avatar
Kulik
CEGUI Team
Posts: 1382
Joined: Mon Jul 26, 2010 18:47
Location: Czech Republic
Contact:

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Kulik » Wed Oct 13, 2010 13:01

a) cegui::Window is standard notation for a struct or class under a namespace

Mikademus: except it isn't... std::vector? boost::lexical_cast?

IMO it's an unnecessary change... and since CEGUI::Class case in your little list isn't used in CEGUI at all and since CEGUI is an acronym, I don't see the reasoning behind this.

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

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Jamarr » Wed Oct 13, 2010 18:01

a) cegui::Window is standard notation for a struct or class under a namespace

Mikademus: except it isn't... std::vector? boost::lexical_cast?


I think he means outside the standard-library where most code bases start classes with an upper-case letter. And that convention even goes beyond c++.

IMO it's an unnecessary change... and since CEGUI::Class case in your little list isn't used in CEGUI at all and since CEGUI is an acronym, I don't see the reasoning behind this.


Let's review the general conventions that have arisen over time:
  • Use uppercase and underscores for macros.
  • Use upper camel for types (classes, structures) and enumerations.
  • Use functions in an appropriate namespace for constants.
  • Use lower camel for functions and variables.
  • Use verbs in function/method names to distinguish them from variables.
  • Use an underscore prefix for private member variables. Don't use an underscore for public member variables.
  • Use a leading underscore for template parameters to avoid name clashes.
  • Use unique, short, all lowercase namespaces. Use the company/library name as a root namespace.

All of these conventions have arisen to meet two needs 1) clarity and 2) ease-of-use. Now CEGUI already follows most of these conventions outside of namespaces, private member variables, and template parameters (afaik).

The idea behind namespaces is to keep them simple, unique, and unobtrusive - using all lowercase improves the ease-of-use and using unique names avoids collisions - similar to using verbs on functions to avoid collisions with variables. An all uppercase name is more intrusive because the developer is required to use shift/caps-lock - that is really what it boils down too. If you can simplify a process without affecting the result, why would you intentionally leave the process less efficient?

For private members, using d_member instead of _member has no benefit - it is more intrusive for no gain. The underscore and lower camel case is all that is required to prevent naming collisions.

For templates, preceding the parameter with an underscore helps avoid collisions, and thus also allows template names more clarity; for example: <T, A> becomes <_T, _Allocator>. All of a sudden the policy for this template becomes clear, solely from the parameters, because this convention avoids collisions with other conventions.

All of these conventions have risen out of the desire to clarify and simplify coding. Even if the reason is as simple as "lower case names are easier to type", so long as there are stipulations to prevent collisions, it is a logical change of convention. It also re-enforces the convention that all uppercase names are reserved for the pre-processor. So this simple change improves both clarity and ease of use.
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
Mikademus
CEGUI MVP
CEGUI MVP
Posts: 130
Joined: Wed Mar 18, 2009 19:14

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Mikademus » Sun Oct 17, 2010 08:58

This Being a Little Essay About the STL and Abbreviations in Symbols

Kulik wrote:except it isn't... std::vector? boost::lexical_cast?

IMO it's an unnecessary change... and since CEGUI::Class case in your little list isn't used in CEGUI at all and since CEGUI is an acronym, I don't see the reasoning behind this.


(I assume that "little" simply indicates that my set of examples was terse and was not meant as... well.. a belittlement of my post :pint:).

The STL is a strange beast when it comes to naming convention. First, as Jamarr indicated, it precedes the conventions evolved over time. Secondly, and this is important: since it is intended to be a part of the language (notice capital-S "Standard") it intentionally uses lower-case signatures to indicate this - "static_cast" and "vector" have an official status the same as keywords like "for" and "continue" - and Boost follows this as they are the experimental lab for the development of the STL. Thirdly, the STL has been the progenitor of quite a few language conventions, among them lower-cased namespaces: the STL (note abbreviation and upper-case prosaic representation) is "stl", the STL's Technical Report 1 ("TL1") is "tr1".

And also, about upper-case abbreviations: this is something that comes from written text where they do not detract from readability because of whitespace and such. But in code they are a mess. They are confused with macros and sometimes with template parameters; they make signatures less readable, introduces run-off errors when reading (that is, we read one letter to far and have to backtrack, which slows our code scanning speed and increases fatigue), and they are more arduous to type and more strenuous for our hands. Since macros are--or should be!--rare they being upper-case can be suffered but otherwise upper-cased words in code are simply bad ergonomics. In the industry today, we increasingly gravitates to the Uncle Bob recommendations (see his book "Clean Code") and let these too follow normal case conventions; that is "CeguiMagicFunction()" rather than "CEGUIMagicFunction()".

Good to have this discussion, because this is not an argument about personal preferences or "being right", this is about helping a tool we love become as good a user and usage experience as possible!

User avatar
Kulik
CEGUI Team
Posts: 1382
Joined: Mon Jul 26, 2010 18:47
Location: Czech Republic
Contact:

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Kulik » Wed Oct 27, 2010 10:10

I am still not convinced regarding the namespace thing - Ogre, Crystal Space (cs prefix and CS namespace) and OIS are breaking it. Irrlicht, OpenSceneGraph are following closely it seems.

I have some more extremelly breaking suggestions though :)

Keeping "falagard window renderer set" name but renaming the "core" falagard to something like "skinning system". It's confusing right now because falagard is a piece of CEGUIBase and a separate module at the same time.
I am still not decided whether I like the current widget mapping or not. It's hard to switch between skins and it's hard to create a new widget (lots and lots of boilerplate), on the other hand it's extremely flexible. I thought doing something like a default renderer and looknfeel per widget class would be a better idea (simpler, easier, less error-prone) and it would work fine for 90% of applications (only one skin) but it has it's drawbacks - less flexiblity, extra StaticText class would be needed, etc... It basically translates to less code in simple cases and more code in specific, rare cases. This depends on other changes in 0.8 (CEGUI::Widget will be bare bones so extra static text class will probably be needed anyways)

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

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Jamarr » Wed Oct 27, 2010 17:17

Kulik wrote:I am still not convinced regarding the namespace thing - Ogre, Crystal Space (cs prefix and CS namespace) and OIS are breaking it. Irrlicht, OpenSceneGraph are following closely it seems.


It sounds like you are holding onto this not out of reason, but because you are emotionally tied too it. Probably because of a fondness with one of the aforementioned libraries and/or a comfortableness with the style. In either case, it is not an objective reason. In fact, I do not think there has been a single objective argument to use an all upper-case namespace nor has any of the all lower-case points been objectively refuted ;)

Keeping "falagard window renderer set" name but renaming the "core" falagard to something like "skinning system". It's confusing right now because falagard is a piece of CEGUIBase and a separate module at the same time.


I found this quite confusing when I first started with CEGUI...falagard what??? I have never been fond of naming well-defined constructs after the author. A system should be named after what it does, not after the person who created it. Giving proper credit and using proper names are not mutually exclusive.

I am still not decided whether I like the current widget mapping or not. It's hard to switch between skins and it's hard to create a new widget (lots and lots of boilerplate), on the other hand it's extremely flexible. I thought doing something like a default renderer and looknfeel per widget class would be a better idea (simpler, easier, less error-prone) and it would work fine for 90% of applications (only one skin) but it has it's drawbacks - less flexiblity, extra StaticText class would be needed, etc... It basically translates to less code in simple cases and more code in specific, rare cases. This depends on other changes in 0.8 (CEGUI::Widget will be bare bones so extra static text class will probably be needed anyways)


It is certainly difficult to maintain consistency with 40 widgets across 4 different schemes. As there are inconsistencies, it makes swapping these in/out somewhat difficult. Why not streamline these a bit and make it easier to create skins based on a default .looknfeel, .imageset, and mapping eg:

  • unify all .looknfeel's into a single .looknfeel; each .scheme would reference the same .looknfeel
  • unify all .imagesets into a single .imageset; each .scheme would reference the same .imageset
  • update all skins (scheme-related image) to conform to the default .imageset
  • remove the skin name from the .imageset; each .scheme will now reference a relevant skin
  • remove the FalagardMapping from the .scheme; each .scheme will now reference the same .falagard mapping file

So with the above approach we only need 1 looknfeel, 1 imageset, and 1 falagard map for an unlimited number of schemes. The only difference between the schemes would be the skin (image) and potentially the font. This should significantly reduce maintenance and make it easier to create new skins since all you need is a conformant image. And as far as creating new widgets, the user would only need to extend the default .looknfeel and .falagard map and the widget would be immediately available to every scheme.
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
Kulik
CEGUI Team
Posts: 1382
Joined: Mon Jul 26, 2010 18:47
Location: Czech Republic
Contact:

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Kulik » Wed Oct 27, 2010 18:37

Jamarr wrote:
Kulik wrote:I am still not convinced regarding the namespace thing - Ogre, Crystal Space (cs prefix and CS namespace) and OIS are breaking it. Irrlicht, OpenSceneGraph are following closely it seems.


It sounds like you are holding onto this not out of reason, but because you are emotionally tied too it. Probably because of a fondness with one of the aforementioned libraries and/or a comfortableness with the style. In either case, it is not an objective reason. In fact, I do not think there has been a single objective argument to use an all upper-case namespace nor has any of the all lower-case points been objectively refuted ;)

Yeah most probably :lol: As I said cegui::Window seems extremely ugly to me. People like what they are accustomed to though so I think I could survive the switch.

I think your mapping idea would kill 90% of CEGUI flexibility even though it would fit most applications I think it's going too far.

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

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Jamarr » Wed Oct 27, 2010 19:27

Kulik wrote:I think your mapping idea would kill 90% of CEGUI flexibility


How so? The necessary changes would not be new requirements, they would be options. I am actually proposing adding flexibility to the existing system, so that a simpler system like the one I described would be possible.
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
Kulik
CEGUI Team
Posts: 1382
Joined: Mon Jul 26, 2010 18:47
Location: Czech Republic
Contact:

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Kulik » Wed Oct 27, 2010 22:18

Jamarr wrote:
Kulik wrote:I think your mapping idea would kill 90% of CEGUI flexibility


How so? The necessary changes would not be new requirements, they would be options. I am actually proposing adding flexibility to the existing system, so that a simpler system like the one I described would be possible.


I read it again and I probably interpretted it differently first time. You are proposing to unify stock skins, not the system if I understand correctly. I thought you wanted to scrap the whole look n feel :lol:

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

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Jamarr » Thu Oct 28, 2010 00:31

Kulik wrote:You are proposing to unify stock skins, not the system


Precisely.
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
Mikademus
CEGUI MVP
CEGUI MVP
Posts: 130
Joined: Wed Mar 18, 2009 19:14

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby Mikademus » Sun Oct 31, 2010 17:29

When I started with CEGUI I too was confused by the "falagard" thing, and I am still not comfortable with it. It seems to indicate that there are two different CEGUI "things" without a clear-cut distinction between them, but at the same time today CEGUI is falagard, or falagard is CEGUI, or?... Names of objects and systems should reflect what they are or do, and I have no idea what "falagard" does or is.

So all thumbs up if in support if you decide to address the symbol names to be more descriptive and self-explanatory!

Also, the unified stock skin suggestions seems pretty sweet to me!

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

Re: API breaking consistency fixes intended for CEGUI 0.8.0

Postby CrazyEddie » Tue Nov 02, 2010 10:31

IMO the standardised stock skin idea is not a good one :P

All skins are not created equal, and there are many differences between Taharez, WindowsLook and Vanilla that go a long way beyond simple imagery changes, and the Ogre skin is something else entirely :)

For example, Vanilla is very lightweight as far as imagery goes - to create some 'grand unified version' would require a new texture that basically repeats the same imagery again, again and again - not a very good example of using the system at all. Another example is differences in Titlebars - Taharez has a titlebar in three sections, so do we force this unnecessary 'extra' part on all other stock skins or do we say that Taharez can no longer have it's little section on the right for the close button? What about mouse cursor 'hot spots'? Are all mouse cursors going to have to be the same general size and shape? What of the offsets required for the corners of Taharez frame window? What of differences in frame thickness and how that relates to inner rect area definitions.

It's a noble idea and all that, but just not practical without bastardising the skins.

CE.


Return to “CEGUI Library Development Discussion”

Who is online

Users browsing this forum: No registered users and 10 guests