[Project] GUI Theme

[prissi]

I do not think a pak should enforce its theme, as it may not be suitable for tablets or phone or whatever device the user has its hand on. Hence, it can be paked that additionally to its simutrans/pak.../ fodler it contains a simutrans/themes/pakxyz.tab/pak files, so its theme would be available via the selector. If no theme (or the default thems) has been selected, then an entry in the pak set simuconf.tab for a preferred theme is used, otherwise ignored.

[Markohs]

It's hard to say, I think I'd keep themes and paksets separated. So no, I think paksets should not specify anything about the theme, and if they do, the less relation possible.

[prissi]

If my post above failed to transmit this: I completely agree with you. I just outlined a way a pak could suggest a thme, and even isntall one optinal choice.

[An_dz]

It's hard to say, I think I'd keep themes and paksets separated. So no, I think paksets should not specify anything about the theme, and if they do, the less relation possible.

But don't we need to specify the pak file in the theme file? Does it ties a theme with only one image set?

[Max-Max]

We are still in an early stage here, it is hard to tell what is included in a theme and what is a part of the pak-set.
In my opinion, the skin-pak used in the pak-set today should be in the new theme "format" ( a separate pak-file ) but still be loaded with the pak-set if the user wants it. I think the themes that comes with a pak-set is a very important to give the right feel and look.

The user can always override this and use whatever theme he likes...

[Max-Max]

Okay so I think I finally have something that will enrich the user experience...
The patch gets quite big (I'm truly sorry Prissi), but adds more GUI elements to customize. We are back on the old skin format for the themes, but it can still do some neat new tricks without changing the format. It is even easier to make themes now.

It's quite late now and I will prepare and post the patch tomorrow. Meanwhile here are some eye candy from the comming patch.
There are still some dialogues that needs to be adjusted for buttons larger thean the old standard, and the theme manager have some minor clipping issues. But I will adress this as soon the patch has been implemented.

This is the default theme with the new skin elements. All buttons have an up, down and disabled image. Color buttons can also be themed now.


And this is an upgraded Aero theme


Theme images doesn't need to be chopped up anymore, it is enough that they span over the cell borders. Scrollbars and buttons doesn't need to be separated in left,middle and right. Just draw the full button in one cell and the Theme manager will chop it up for you.

Here are some default and Aero theme images...
I will post the source and photoshop files tomorrow as well...

Good night...

[Yona-TYT]

I like it, Nice work!

[IgorEliezer]

DON'T MOVE! I dropped my jaw! O.O

[Sarlock]

Very nice!  It gives it a very fresh appearance.

Might I suggest symbols in the menu control buttons at the top right to indicate what each colour does?  ie: an "X" in the red circle, etc.

[Markohs]

Good work! I must say I hate that aero theme, however. It's just personal taste.

Congratulations, Max-Max and prissi, for your hard work in the project so far.

I hope our artists get inspired and do a nice theme, I'm pretty tired of seeing the default one, it reminds me of very old UNIX desktops, like CDE, but worse.

[An_dz]

I hope our artists get inspired and do a nice theme, I'm pretty tired of seeing the default one, it reminds me of very old UNIX desktops, like CDE, but worse.

I always liked the 96comic theme, so bad it doesn't fit the other paksets style.

Good work! I must say I hate that aero theme, however. It's just personal taste.

Me too, now imagine a whole OS like it.

Might I suggest symbols in the menu control buttons at the top right to indicate what each colour does?  ie: an "X" in the red circle, etc.

From what I know this theme tries to follow the Mac look and there are no symbols on Mac. This theme should be updated to match the new MacOSX Lion theme.

[Max-Max]

Very nice!  It gives it a very fresh appearance.

Might I suggest symbols in the menu control buttons at the top right to indicate what each colour does?  ie: an "X" in the red circle, etc.

It is the Aero skin, modified to a theme pak. I have redrawn the old aero and hasn't really changed so much except it original original size (based on Aqua). The colour dots are a Mac thing and since aero came from the Mac... 

DON'T MOVE! I dropped my jaw! O.O

Thank you, this is only the beginning of the work ahead, but I think we are in a stage where we can see some of effect of my work. Meanwhile Prissi merges this patch into the trunk I will write an "Artist's theme guide" and continue on my mock-up design I showed earlier...


@Prissi (and every one else)
Here is the little too large patch file, but it isn't that bad. Half of it are brand new source files added.
Them manager patch

Here are the new Simutrans icons and the resource script that selects the right one depending on build version.
Resource script + icons

The theme pak format is back to the old format, you will need these new ones. I have built both default and aero in this archive ready to drop in and use.
Ready to use theme paks

Here are the photoshop source to the themes
New aero theme source
New default theme source

I have not done anything on the original work with dialogues and resize. But as soon this is in the trunk and I'm up in synch this will be my next task on the list. Meanwhile I will write an "Artist's theme guide", explaining what can be themed and not (at this point).

Cheers...

*** EDIT
Some of you had posted before I finished this one... So thank you for the great feedback so far, we will see how the code merge goes 
As you will find out there is a new skin for a filter button (the old colour button that couldn't be skinned). Any transparent area will be filled with the button colour.
You will also notice that almost all controls now have a down, up and disable image.
Most of the text, background, highlight and background colours can be set in the theme.tab file. It is a bit chaotic there, but I will clean up and try to find a structure in it...

[Max-Max]

I always liked the 96comic theme, so bad it doesn't fit the other paksets style.
Me too, now imagine a whole OS like it.
From what I know this theme tries to follow the Mac look and there are no symbols on Mac.

Now you can make a Comic theme to be used with any pak-set

[Sarlock]

From what I know this theme tries to follow the Mac look and there are no symbols on Mac. This theme should be updated to match the new MacOSX Lion theme.

It is the Aero skin, modified to a theme pak. I have redrawn the old aero and hasn't really changed so much except it original original size (based on Aqua). The colour dots are a Mac thing and since aero came from the Mac... 

Understood.  I never did like those buttons on MacOS... the Windows user in me was so confused that I went crying back to Windows again!   

Wonderful work so far!

[Markohs]

Me too, now imagine a whole OS like it.
From what I know this theme tries to follow the Mac look and there are no symbols on Mac. This theme should be updated to match the new MacOSX Lion theme.

I'm allergic to all Apple products and software, I think all they do it's overrated ****. The only good thing they did is iPods, rest is ****.

[Dwachs]

nice work! impressive patch file. Looks like a lot of clean and nicely documented code

[Markohs]

Just had it a brief look so far, but two comments:
* .cpp files should be renamed to .cc.
*  theme_element_t::encode and get_RLE_size can be useful in other parts of code(to build the world slopes for example, but not only there), I'd not associate them just with themes, I'd prefer them under a "imageutils::" or "imagetools::" namespace (this option will be rejected by some developers, I think), so move it to global C functions. They are not functions only related to theme management, so they should be moved.

[prissi]

It is a very large patch, and some is very nice and on some I think a little polishing is needed. For sure everything in one list will not do, this was a bad design decision by Hajo and I will not repeat it. But I will have to digest this longer.

A few initial comments in the order I found them in the patch:
- While it does not seems so, calling display_fit_proportional and then display_calc_proportional_string_len_width internall, does just replicate what the previous function did. Why doing this twice?
- The debug string will not be shown to the user. It will only yield stupid questions. If you want you can find out about a debug built anyway in the status line if you know where to look.
- I though you would put checkboxes into checkbox_t type and so on. They could still inherit from button. But ok, that is probably next. Similarely, if touching background as button public, then one should do set_background_color() to be consequent.
- you introduce a new element "component" while the GUI already has gui_komponente_t Why???

[Max-Max]

Just had it a brief look so far, but two comments:
* .cpp files should be renamed to .cc.

The .cpp files are an honest mistake. MSVC automatically add .cpp on new files, I was not paying enough attention, my bad.

*  theme_element_t::encode and get_RLE_size can be useful in other parts of code(to build the world slopes for example, but not only there), I'd not associate them just with themes, I'd prefer them under a "imageutils::" or "imagetools::" namespace (this option will be rejected by some developers, I think), so move it to global C functions. They are not functions only related to theme management, so they should be moved.

In the comments I wrote a note about making raw_bitmap_t to a class, working more or less as a canvas. There are several functions in this category. create_bitmap(), blit(), get_RLE_size(), decode(), encode(), make_horizontal() and make_vertical().

There are also a number of std::string functions that I wasn't sure where to place them; get_path() and get_name(). But I placed the std::string version of trim() in simstring.cc but I'm not sure that would be a good place for it.

This is still in development and more functions will be added, modified or removed... There will for sure be more theme_element_t derivatives as we go along...

It is a very large patch, and some is very nice and on some I think a little polishing is needed.

Keep in mind that this isn't the final, it is a ruff draft and there are a few clipping issues in the manager and no optimization done at all. But what is more important, it is good enough to be used and it will give a quite good user experience. I could go on and do more, but I felt that the patch was big enough.

For sure everything in one list will not do, this was a bad design decision by Hajo and I will not repeat it. But I will have to digest this longer.

What list are you referring to, the skin theme or images[]?

The use of the theme list is really more out of convenience, it is really an unnecessarily step in theme loading. But to get rid of it we need to write a new reader and I would prefer to have the theme manager to do it directly. The use of images[] is because all the display_xxx routines are using it. In my first early test version, the manager was doing this on its own, but I figured it would be better to use the display_xxx functions so I don't need to worry about any format changes in the future.

How ever, thanks to the theme manager it is a lot easier to prepare the theme images than before, but still using the old PAK format so we don't have to touch any reader or writers yet. I think all those magic index in the .dat file can be made simpler. I would like to write a scanner, tokenizer and a parser for the .dat/.tab files. This would allow us to create a more flexible system. By adding the parameter name together withthe value in the PAK file, we are not relying on indexed positions anymore and can actually use names instead. However, loading a PAK file isn't time critical, so we can afford the extra time to search for each parameter and put it where ever it belongs. because the gui elements are easy to visualize as objects, it would only be logical to also have a more object oriented format in the .dat file (...and I'm not thinking of the obj sections in the current format).

A few initial comments in the order I found them in the patch:
- While it does not seems so, calling display_fit_proportional and then display_calc_proportional_string_len_width internall, does just replicate what the previous function did. Why doing this twice?

As I said I have not optimized anything yet. However this isn't anything I have reflected over. Can you point out any place where you see this so I can have a look? When it comes to the drawing of truncated text with an eclipse, it should definitely go into its own function, or be a part of the text drawing routine.

- The debug string will not be shown to the user. It will only yield stupid questions. If you want you can find out about a debug built anyway in the status line if you know where to look.

What debug string are you referring to? There are a couple of them...
I forgot to mention that I have added a DEBUG switch to the makeobj.exe that will dump the progress of PAK creation. It is very useful to find out why a theme isn't working as it should. It dumps the makeobj's intepretation of the .dat file and all image positions, and size.

- I though you would put checkboxes into checkbox_t type and so on. They could still inherit from button. But ok, that is probably next. Similarely, if touching background as button public, then one should do set_background_color() to be consequent.

Yes that will be done in the new gui structure later down the road... For now I have been focusing on theming, next step to fix the remaining dialogues and after that I thinnk it is time for the new gui structure, or the parser...

- you introduce a new element "component" while the GUI already has gui_komponente_t Why???

Oups, my mistake. It is a draft on the new gui structure and it isn't used anywhere. It wasn't supposed to be in the patch at all, please delete it.

[Markohs]

In the comments I wrote a note about making raw_bitmap_t to a class, working more or less as a canvas. There are several functions in this category. create_bitmap(), blit(), get_RLE_size(), decode(), encode(), make_horizontal() and make_vertical().

Good.

There are also a number of std::string functions that I wasn't sure where to place them; get_path() and get_name(). But I placed the std::string version of trim() in simstring.cc but I'm not sure that would be a good place for it.

You also have savegame_frame_t::get_filename and get_basename() . Whatever you decide, both parts of the code should end calling the same function, no core duplicity is desired.

Cool, let's see what we get when this patch is finished.

[Max-Max]

You also have savegame_frame_t::get_filename and get_basename() . Whatever you decide, both parts of the code should end calling the same function

That is a very good point, but I didn't want to change the current string system. I think maybe we should make use of the std::string class instead of the pre STL string functions. If I'm right, std:string also handles unicode?

The VCL framework have a String type that is redirected to the ANSI or Unicode version depending on compiler target. Maybe we should have something similar, or just always use unicode?

[prissi]

One question about programming styles. Why do you not make you structures a class when you are using a constructor (or any other function). The ..._tag style is used nowhere in simutrans, and you are more likely to produce clashes by this. If you want to do any function (or whatever I would call a side effect possibility) by something like "bla_t a;" then you should put it into a class.

Or is there a hidden reason for this?

[Max-Max]

One question about programming styles. Why do you not make you structures a class when you are using a constructor (or any other function). The ..._tag style is used nowhere in simutrans, and you are more likely to produce clashes by this. If you want to do any function (or whatever I would call a side effect possibility) by something like "bla_t a;" then you should put it into a class.

Or is there a hidden reason for this?

I assume you are referring to the new scr_size. A struct is the simplest form of a class where all members are public. the _tag is there to be able to define a constructor to initialise the type when it is created. When I created it it just had one constructor and two member variables, then I needed to add some operators and it got bigger...

Structs are by tradition used by POD classes (Plain Old Data), meaning it is a type just like int, double etc... All other classes are defined as class. I'm sidestepping here from a "true" POD definition, by having constructors and operators, but the struct definition indicates that this is a type class that only store information, not a class like a karte_t or frame_t that manage a number of functions.

If you still want everything to be classes, even if they are types with some helper functions, you can turn it into a class and make ALL members public.

*** EDIT
Clarifying:
The use of tag_ is the only way to define members using the type they belong to, since scr_size hasn't been defined yet. This is a common solution to this problem.

A struct is a class with the only difference that all members are default public instead of private (well and the use of a struct vs class may differ in unions and templates).

[Ters]

If I'm right, std:string also handles unicode?

Yes and no. It handles Unicode like Linux handles Unicode: If it's UTF-8, it's also valid ASCII as far as the computer cares. Everything above codepoint 127 will appear as junk to a human of course, and a char won't necessarily be a complete character, so code wanting to operate on single characters and not just bulk text must do extra processing.

std::wstring, made up of wchar_t, is supposed to be Unicode, but on Windows, it's "just" UTF-16. UTF-16 should be enough, except for certain CJK characters. I'm not sure how common those CJK characters are though. On Linux, wchar_t, and hence std::wstring, is UTF-32 and encode the entire Unicode character set. So in this case, a wchar_t is a always a complete character. It wastes a lot of memory, though.

In my experience, it is difficult to use std::wstring, as lots of legacy APIs only accept char arrays. So in pratice, C/C++ doesn't handle Unicode. Windows has dedicated Unicode APIs (although only UTF-16), so you can almost get it right there if you stick to the Windows APIs. The rest just "fakes it" with UTF-8, which I've learned causes compatibility problems for both Git and Mercurial.

[Max-Max]

The question is really, where do we want to go? If we say that we will support languages such as Chinese, Japanese, Hebrew etc we must support full unicode.

I found recommendations to use the ICU string library for C/C++ that has full unicode support.
Maybe some one is interested to have a look?

[Tazze]

The question is really, where do we want to go? If we say that we will support languages such as Chinese, Japanese, Hebrew etc we must support full unicode.

My recommendation is Unicode, UCS-2.
It can apply various language in the world ,of course included in kanji ,in order to wards represented by 2 byte .
This contents is as same as IOS/IEC 10646

[Max-Max]

I have discovered something strange... If I start Simutrans without the -workpath switch AND have configured it to use the default_theme it crash. If I do the same procedure but instead select aero_theme it works well.

Now the strange thing, if I add the -workdir switch, the default_theme works!?!
Same installation and exe in both cases...

So there is something fishy in my patch. I will investigate ASAP...

[Ters]

My recommendation is Unicode, UCS-2.

UCS-2 has been superseeded by UTF-16 since 2 bytes can't encode all of Unicode. There is still the problem that virtually no API (except Windows') use UCS-2 or UTF-16 (or UTF-32), so you have to convert between them and UTF-8, or perhaps even ASCII.

Simutrans seems to be using UTF-8 when needed and ISO-8859-1 (or rather windows-1251 on Windows) otherwise.

[prissi]

Ok, I submitted scr_coord.h as suggested. I did not find some of the names very informative, so I changed them. I am open for discussion.

I still like to have a getter and setter instead accessing the elements directly. If everything is public, then we could have stayed with C ...

[Max-Max]

Ok, I submitted scr_coord.h as suggested. I did not find some of the names very informative, so I changed them. I am open for discussion.

I had commented out not yet used member functions, to be brought in when needed. You added them all back in again, I guess that's okay. I just commented them out to see what members was used and what wasn't...

A better name for the member reduce_to_overlap() (original clip()) imho is intersect() and have another member get_intersect() returning a new scr_rect as suggested.
The term intersect is a common term when performing bool operations on geometry/polygons.

I still like to have a getter and setter instead accessing the elements directly. If everything is public, then we could have stayed with C ...

I have mentioned this before, just because we use a C++ compiler doesn't mean that we have to put everything in classes. By tradition, simple types (POD) are put in structs because they don't need anything more advance that that.

C++ programming and Object oriented design isn't the same thing as putting everything in classes and hide all data... Getter and setters are used when you need to protect the data or perform some automatic operation when the data is accessed. In this case there is no need to do that. I tried the getter/setter version and it was a true pain to use it. These members are accessed so often that the code became really cluttered and hard to follow...

[prissi]

I left them public for this reason.

About the clip function. Maybe call it clip_to and then the parameter clip?rect instead rect. That may make the function more clear.

Anyway I am working on the rest now.

[Ters]

I get lots of warnings from scr_coord.h line 192. That function should perhaps return void. Alternatively, if someone is trying out chaining, it should return some form of reference to this, not an instance.

[Dwachs]

I get lots of warnings from scr_coord.h line 192. That function should perhaps return void. Alternatively, if someone is trying out chaining, it should return some form of reference to this, not an instance.

I changed the return type to void already.

[Max-Max]

I get lots of warnings from scr_coord.h line 192. That function should perhaps return void.

I'm complete innocent here, it is a void in my original code.

About the clip function. Maybe call it clip_to and then the parameter clip?rect instead rect. That may make the function more clear.

void clip_to( scr_rect clip_rect ) or void intersect( scr_rect rect ) will do fine.
Intercept is the standard term for this operation and will fit in well if we add the other operations as well.
However if this object would support such operations it is most likely something else than a rectangle and should maybe be another class instead.

So, I guess clip_to() will do.

Anyway I am working on the rest now.

I fell both happiness and anxiety at the same time... I do hope you don't change anything, the system isn't complete yet and it is to early to remove the single Theme list yet.

[prissi]

Sorry since Friday evening my MSVC was broken. (About some Unicode cahrachter (\uDB4B) in the project file while there was absolutely nothing. After reinstalling and checkout from empty it work again. So no progress since Friday. I will change scr_coord.h immediately.

EDIT: Microsoft really does not won't me working this weekend. While editing file Windows7 did a restart to install patches. Without a warning, not asking for saving, just restart while I was typing. (XP did this about once a year, but I never expected it from Win7). I lost the last two hours. Sorry, will not finish today then.