API breaking consistency fixes intended for CEGUI 0.8.0

[Kulik]

How about kicking the whole PropertyHelper and implement something like boost::lexical_cast instead?

That's what this is basically... Only faster because lexical_cast uses std::stringstream whilst this will remain C sscanf based I can't think of a better name for it...

[MarkR]

You're right, lexical_cast is probably the wrong name for this, but it still matches my basic idea.

What I meant is to have one single function that converses anything to string or vice versa, instead of two. You wouldn't be limited to convert to or from a string, but also could convert to or from utf8*, std::string or probably even std::wstring (no, I don't want to start another UTF32 / UNICODE discussion again, but this may be useful at least for the windows implementation) - and this with nearly no effort, no matter on how many to_string of from_string conversions you already implemented. Providing a conversion like to_std_string would mean to re-implement all other to_string conversions again...

It also could provide functions to convert from Vector2 to UDim. Not sure about the sense of such a conversion at the moment, but, well, I just said you COULD

I personally wouldn't care, as it's no problem to convert an utf8* to String before calling the from_string function (the String(utf8*) constructor isn't explicit, is it?), and also no problem in calling the utf8 conversion operator after a call of to_string, but the PropertyHelpers are convenience functions, aren't they?

Just my 2 ct.

[Kulik]

PropertyHelper will only convert from and to CEGUI::String, converting from UVector2 to Size or something like that doesn't make much sense. Converting from and to std::string, wstring or plain utf8* is not desired as these all can be represented by CEGUI::String and in CEGUI, you should only use CEGUI::String.

You're proposing something like TargetType PropertyHelper::convert<TargetType>(const SourceType& source)? It would work but IMO introduces ambiguity. I think String toString<SourceType>(const SourceType&) and TargetType fromString<TargetType>(const String&) is less ambiguous and introduces more readable code. With toString method you could omit the template parameter when using it, since compiler will use the right one for you.

[MarkR]

You're proposing something like TargetType PropertyHelper::convert<TargetType>(const SourceType& source)?

Yes, this is exactly what I meant. Concerning the ambiguity, there is not more or less ambiguity, than the boost::lexical_cast functions provide

[Kulik]

There are some inconsistencies between the words draw and render. For example CEGUI::Window has a method "render", but this method uses "drawSelf". I think this should be unified to avoid having to check what of those words should be used. I beliebe in this context, they are synonyms. I think "draw" is used more in CEGUI (GeometryBuffer, etc), so I think just migrating to draw everywhere where applicable would be the best solution.

[Mikademus]

About namespaces:
I agree with that a lower-case namespace is to be preferred. I very rarely see any namespaces using upper case characters, and when I do I frown and mutter. Nested namespaces can be a menace, though. Sometimes they are useful, but very rarely. I generally only use them when (1) I have classes or functions that are used by sub-functions of a module, classes and functions that (2) must still remain visible and accessible for client code, and (3) these classes or functions may cause name conflicts. One example is my Sprite class (under my project namespace), which under namespace "sprite" (lower-case!) collects its Frame, Sequence and Animation component classes.

About "window" and "widget":
I think it is mainly MS Windows nomenclature that all GUI components are "windows" because under the Win32 API all are created through the CreateWindow() family of functions which returns a HWND resource pointer. In normal parlance, though, a "window" is a GUI component that is always a modal or modeless top-level container that may contain child widgets, where "widgets" are always necessarily attached to a container component.

Also,
might it be time again to raise the topic of revamping the input event and input injection systems?

[CrazyEddie]

Just to update on some stuff (no changes in the OP).

The "CEGUI" vs "cegui" namespace thing IMO is not a 'consistency' issue; rather it's something that some people don't like. Genuine consistency fixes are one thing, making breaking changes for no real reason is another, given this, I seriously doubt that this will be changing

Still not decided on the nested namespaces, I see positives and negatives here

I think a consistency fix where the window/widget confusion is addressed will go ahead, and it will favour 'Widget" as the general term.

Pretty much any change that replaces code repetition with template usage, is highly welcomed.

For the input event / injection thing, it is "on the list" as it were, though many things are "on the list" Basically, the best way to potentially see changes in this area is to come up with some concrete implementation ideas that can be discussed. To see definite change, the way forward is to then implement any previously discussed and agreed ideas and submit them

Also note that 0.8.0 will be some time next year. While this thread is here to discuss and gather the breaking changes that will go into that release, it's somewhat doubtful that the majority of changes will go in until much later on (because it makes merging fixes from the stable branch a lot more difficult). Add this to the fact that my priority at the moment is the tools, and you will gather that not much other work will be done towards 0.8.0 for a while yet, either. But rest assured, eventually 0.8.0 will become a priority

CE.

[Jamarr]

The "CEGUI" vs "cegui" namespace thing IMO is not a 'consistency' issue; rather it's something that some people don't like. Genuine consistency fixes are one thing, making breaking changes for no real reason is another, given this, I seriously doubt that this will be changing

I guess it depends on your scope. In the CEGUI project itself, it is not a consistency issue. However, outside of CEGUI in the c++ library community, it could be considered inconsistent by some. There are certainly a number of c++ libraries that have mixed-case or all upper-case namespaces; but those libraries are typically much older, and because naming conventions for namespaces was considered irrelevant, every library has adopted their own convention. However, I think this is slowly starting to change and with improvements for nested-namespaces in the c++ standard on the horizon I think that more libraries are adopting the lower-case convention for reasons already mentioned.

Still not decided on the nested namespaces, I see positives and negatives here

CEGUI already has nested namespaces! Regardless of the outcome, CE, I would like to hear your opinion on the issue underlying the current nested-namespace debate: moving event-names into their own header and scope (namespace) to de-couple them from their associated elements headers. I doubt this has significant ramifications, but if there is no reason for them to be coupled, and it is easy to de-couple them, then why not?


This next issue came about from this thread: Extending ItemListBox. My question is, is there even a need for ItemListBox in it's existing form? Both ItemListBox and the new layout-containers manage the layout of child-windows, differently, but achieve similar effects.

I think the differences are that ItemListBox manages a list of ItemEntry (Window) objects (and apparently can still take child-windows? which is just awkward), uses the term "selection" for active items (getFirstSelectedItem, etc.), and allows multi-selection. Whereas layout-container just manages child Window objects, inherits the term "active" for active windows (getActiveChild), but does not support multiple-active (selected) windows. So I wonder if ItemListBox should simply be re-factored as an extension of a layout-container supporting the concept of (multi)selection?

[Kulik]

Jamarr: +1 about layout containers being used for MCL, listbox, etc... I will most likely redo the grid LC implementation so it can withstand being used with MCL (currently it would be slow). I don't think there will be many changes to the vertical and horizontal layout containers. I though about merging them into one and having something like "Direction" as a property but I rejected this idea (mainly because other libs don't do that and it would be hard to extend it).

[Jamarr]

I hope the layout/listbox thing is not too far off topic; while perhaps not entirely a consistency issue, it could be seen as inconsistent if there are multiple ways to achieve the same thing. And while I think options are good, too many are evil - if there is a "better" way to do /the same thing/, why even offer lesser approaches? I can understand why the MCL is still around, but why is Listbox still around? It is confusing. ItemListbox is superior in every way.

Back to the layout-based listbox topic, I am pretty sure that wxWidgets, and likely others, have listbox widgets that are not built upon their sizers/layout containers. I think this is simply because sizers where not added until much later (similar to CEGUI) and they simply never got around to refactoring those widgets, or adding new sizer-based lists, because they already had something that worked and wanted to tout backwards-compatibility because it sounds consistent. If there are any actual valid reasons for it, I would like to hear them. In my thoughts, a listbox /is/ a type of layout-container and hence why it makes sense to be derived from one.

Kulik, as far as merging vertical/horizontal containers, I do not think the decision should be based on what other libraries do or do not. It can be a good indicator of what to do, but it should hold no weight in the final decision. There are plenty of libraries who take the wrong approach. You simply need to answer the question: why? I cannot think of a single answer to that question. Until it can be answered, I agree, you should leave the vertical and horizontal layout-containers as separate implementations. I also agree with using the grid-layout to implement an MCL since it can easily impersonate a single-column or single-row list. Then we can get rid of ItemListbox too

[Kulik]

Jamarr, the main caveat with merging the vertical and horizontal layout container is that it wouldn't be extendable. Imagine somebody wants to do a novelty sequential container. Right now the new class can inherit SequentialLayoutContainer and the only code that has to be overridden is the actual layouting code (all the adding, removing, reorgranising, etc is already there so the new sequential container gets support in the layout editor with no further effort). If all sequential containers were merged, extending them would need patches to the core so the end developer wouldn't be able to make any new seq containers easily (imagine rotating inventory or such, there are more than just vert and horiz).

And the only drawback of separate implementations for vert and horiz container is the inability to switch between them, right? But when my clone patch is ready, morphing between sequential containers will be very easy, I think I might even add a new method to SequentialLayoutContainer to support this conveniently.

This is probably the point where I am getting offtopic: I want to do a new killer implementation of the grid layout container. Something that can support row span, col span, has no need to set dimensions before adding (apart from when you need auto ordering) while still being fast. It will need some alternative approach because just mapping to d_children would require dummies, setting of dimensions and would be really slow. When this is done a new MCL can be born. These changes will only affect the actual implementation of the grid, there will be no breaking changes to the API, so I think this could go in earlier than 0.8. The MCL will have to be torn apart and redone so that will definitely have to wait till 0.8.

[Jamarr]

Right. Again, I agree that the vertical and horizontal layouts should be kept separate. The only reason to do so would be for ease of switching of layouts, but even then merging them is probably the worst implementation choice. As you mention, it should simply be a matter of specifying a new layout container.

I am a bit confused by your association of the term "switch" with the term "clone". If you are switching a layout, say from vertical to horizontal, why does anything need to be "cloned"? On a side note, I just assumed that hot-swapping layouts was already possible, but I guess that assumption was wrong. So this "clone" patch is to add some formal process for doing so?

Btw I think you are doing very productive and worthwhile work; and it seems very clean too. Looking forward to any future contributions from you

[Kulik]

I would really really like if everything was CEGUI::Widget based in 0.8 CEGUI, thus removing the need for ItemListbox, etc... And perhaps some discussion about thinning up CEGUI::Widget a bit would be great. Most of elements only need 10% of what CEGUI::Window can do. If CEGUI::Window is renamed to CEGUI::Widget I think renaming folder cegui/include/elements to cegui/include/widgets would be logical and the same for src.

Jamarr: Inplace hot swapping is not possible as is the case with any widget. But you can create a new layout container, go over all children and move them to the new layout and then destroy the old layout, that's what I meant I probably didn't say it clearly. And yes I want to do a more formal approach to this. There will be two methods of "morphing", copy and move, for copy morphing I need the new patch, for move it's not needed.

Code: Select all

VerticalLayoutContainer* old = ...

HorizontallLayoutContainer* new = old->morph("HorizontalLayoutContainer");


This will rename the old container, create the new one with the old one's old name, move all child widgets to the new one and lastly (if requested), destroy the old layout container. This will only be possible with Sequential Layouts! I have to finish up some patches because my CEGUI work environment is getting a bit crowded

btw: I am really glad you like the code I just wanted to do a few patches to make my life easier but I ended up really enjoying "CEGUI hacking"

[CrazyEddie]

Heh. A bit of "CEGUI Hacking" is good for the soul

CE.

[Mikademus]

The "CEGUI" vs "cegui" namespace thing IMO is not a 'consistency' issue; rather it's something that some people don't like. Genuine consistency fixes are one thing, making breaking changes for no real reason is another, given this, I seriously doubt that this will be changing

I guess it depends on your scope. In the CEGUI project itself, it is not a consistency issue. However, outside of CEGUI in the c++ library community, it could be considered inconsistent ... I think that more libraries are adopting the lower-case convention for reasons already mentioned.

I think Jamarr nailed it here. Lower-case namespaces have after some time of gravitating toward becoming the convention finally been established as standard. Now upper- or mixed-case namespaces when found in code are provoking the same reactions as all-caps variable names would: the code may work, but it is formatted in an unfortunate, suspicious and non-standard way. A little smattering of example projects using lower-cased namespaces contains Boost, the STL, Irrlicht, OpenSceneGraph and SFML ("std", "irr", "osg" and "sf", respectively).

So perhaps since CEGUI0.8 is already scheduled to introduce a number of breaking changes and new interfaces this is in fact the perfect time to change to lower-cased namespaces. Client code will need to be revisited anyway, the code is being made more uniform and streamlined, some fundamental constructs and idioms are changing names; thus a convention-conforming namespace would signal, reflect and reinforce this new state. As breaking changes go, and compared to other ones slated for inclusion, this is a very minor one and well compensated and worth it in this supportive user's humble opinion.