News:

Simutrans Tools
Know our tools that can help you to create add-ons, install and customize Simutrans.

[Project] GUI Theme

Started by Max-Max, May 31, 2013, 11:12:48 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Max-Max

I am in Bulgaria right now on vacation. But I have everything in my laptop. I will send you an emergency fix as soon I get the time and internet connection...
- My code doesn't have bugs. It develops random features...

Fabio

enjoy your vacation! have fun!!!

prissi

Do not, enjoy your vacation. SImutrans can wait.

Max-Max

#213
Today it is my birthday so I will give you a fixed map dialogue as a present :)

You have to add the new scr_coord.h and gui_map_preview.h/cc to the project.

Is there another way to display the relief map? I started to break out the code in welt.c into a gui_map_preview class, but Isn't it possible to use the same code used in the mini map?

The default static text color is a bit light, please feel free to find a better colour.

Still in Bulgaria, home in 12 days :)

EDIT
Removed the patch after I found some bugs. New patch coming soon...
- My code doesn't have bugs. It develops random features...

Fabio


kierongreen

Indeed - Happy Birthday :) Also good on you for working away on this while away from home!

isidoro

Happy birthday, Max-Max!  But you should be the one to receive the presents, not the one to give them...  :D

prissi

Happy birthday, enjoy your holiday!

Yona-TYT


Max-Max

#219
Thank you every one.

I'm having trouble to apply my own patch... :(
The file gui/components/gui_map_preview.h and gui/components/gui_map_preview.cc is not created. I can see them in the patch file but when I apply the patch (TortoiseSVN apply patch -> all) they aren't created...

What have I done wrong?
Does any one else have the same problem?

EDIT:
I noticed that the depot dialogue got mixedup a bit due to my changes in gui_label_t. I thought it wan't used. I will make a new patch shortly...
- My code doesn't have bugs. It develops random features...

Max-Max

Well, here is a patch that will fix all the trouble I caused so far :o ...and some more.

* Map dialouge done
* Fixed help dialogue
* fixed depot image miss alignment
* fixed scrollbar auto hide
* fixed better string cap routine in Label and button
* Added THEME debug macro
* Fixed edit width in climates (themed)
* Fixed so an opened combobox follows the control when moved.
* gui_image_t got an alignment mode and an auto size mode.
* Added SYSCOL_BUTTON_TEXT

I fixed the annoying image miss alignment in the depot dialouge and implemented an auto hide scrollbar function (now default).
There is no previous way to handle a hidden scrollbar, so I draw a place holder instead. The best would be if the parent's client area where expanded instead, but we aren't there yet.

I had as usual trouble to get the new added files into the patch so I have attached theme separately. If the patch doesn't create:

in gui/components folder:
gui_image.cc
gui_map_preview.cc
gui_map_preview.h

in project folder:
scr_coord.h

please add them from the zip archive manually. Don't forget to add these files to your project as well.
It feels like I'm back on track again...

- My code doesn't have bugs. It develops random features...

prissi

Thank you, I have commited most of the code. As for the calculation og button sizes from their graphics, I have not yet submitted the define, as this will change anyway when doing it porperly after reloading from files defined in themes.

Soem further comments:

I have my doubts on the implicit conversion from  scr_coord to koord. If scr_coord is made 32 bit as discussed, it will silently produce errors.

Also I am not so keen of a default constructor for scr_coord. It shoudl have rather none if possible. Or when, then one which set them to some undefined value, imho.

And should rect be rather x,y and width and height? I though that was the consent form the other thread? Most UI elements have a size (which they care for) and a position (which often somebody else cares for). But I will submit it without this for the moment.

scr_coord is also defined unusually. Imho it is better anyway a class to define operators. (I have at least one compiler who does not like structs with operators.) Apart from the tag_... definition. Form me this file would be expected either in gui/ or into dataobj/? Also the set method and copy constructor is no needed, as it will be done right be the compiler anyway.

At the moment in buttons koord and scr_koord are happily mixed. But this is certainly a temporary state, or?

"size_mode_stretch" will most likely never come. It will be most ugly too.

A question of style is your par for your parameters. If you are afraid of changing them, you can make them cost; but the par is ugly in my eyes. But other may think different on that.

And finally, please no client rectangles. This is not needed, or please give an example where get_groesse is not sufficient. It will rather introduce new errors, and is another thing which needs to be initialized. A function for a contianer like GET_minimum_rect os so would be ok something much more useful and descriptive.

Autosizing buttons: How should you design a nice balanced dialogue with such buttons? I would rather add a function like for the flowtext, i.e. recalc_groesse() or so. Or resize_to_minimum() which resizes to smallest rect or so.

About gui_map_preview: What is this good for? It just draw the array, so why must it be done this way?

The autohide scrollbars is something I have to get used too; but it is certainly innovative. (Actually, is the autohide graphic smaller than the previous scrollbar?)

Max-Max

QuoteI have my doubts on the implicit conversion from  scr_coord to koord. If scr_coord is made 32 bit as discussed, it will silently produce errors.
Hmm, are you referring to the code that is commented out in scr_coord? I tried to make a conversion routine so we could start to migrate slowly froom koord to scr_coord. I got caught in some cross reference problems and commented out the code. I would be happy if you could fix that routinefor me. This is temporary so we can do the update of koord to scr_coord more slowly. It would be a heck of a patch if we where to change all of this at once. When koord is replaced by scr_coord we can remove this the conversion (and the one in koord).

We can't do a search replace because not all koord should be updated to scr_coord. Those used as map coordinates should still be koord as I understood from the previous discussion.

QuoteAlso I am not so keen of a default constructor for scr_coord. It shoudl have rather none if possible. Or when, then one which set them to some undefined value, imho.
Why not?!? To have uninitialised members is to beg for strange bugs...

QuoteAnd should rect be rather x,y and width and height? I though that was the consent form the other thread? Most UI elements have a size (which they care for) and a position (which often somebody else cares for). But I will submit it without this for the moment.
In the code we quite often use pos.x + groesse.x etc... The beauty of OOP is that we don't need to care for the internal representation. We do need to define a rectangle and its sides, both as absolute and relative values.

Quotescr_coord is also defined unusually. Imho it is better anyway a class to define operators
I have no objections here. In fact both scr_coord and scr_rect could be classes. Maybe next patch?

QuoteAt the moment in buttons koord and scr_koord are happily mixed. But this is certainly a temporary state, or?
My guess is that you refer to scr_coord_val and koord? These are very different. scr_coord_val replaces KOORD_VAL,while scr_coord will replace koord. Yes it is temporary...

Quote"size_mode_stretch" will most likely never come. It will be most ugly too.
Well, as the remark says, it's not implemented yet and I don't see where we should use it anyway. It's only an enum on the drawing board... you (or I) can remove it.

QuoteA question of style is your par for your parameters. If you are afraid of changing them, you can make them cost; but the par is ugly in my eyes. But other may think different on that.
Well, I'm not so keen of_par either, but I want to use a postfix (or prefix) on parameters so they don't collide with member variables.
If you have a shorter or better way of a prefix/postfix I open for suggestions. Just don't use _ or __ because these are usually internal or extended C/C++ names.

QuoteAnd finally, please no client rectangles. This is not needed, or please give an example where get_groesse is not sufficient. It will rather introduce new errors, and is another thing which needs to be initialized. A function for a contianer like GET_minimum_rect os so would be ok something much more useful and descriptive.
The concept of client area is a widely used term and it is passed to a control's all children call of zeichnen() so they can position, update or do whatever they do, without the concern of what the parent look like.
Think of the client rect as a boundary for the children to stay within. A container on the other hand, owns other controls, where the client rect only is a boundary.
For now, the controls outer boundry rect (pos and groesse) happen to be the same as it's client rect. When we apply a theme this might not be the case. A button with a fat border wants the text to stay inside that border, here the client rect is different from the controls size. In my document I published earlier, I touched the client concept briefly.

Already now, the tab panel is an excellent example where a client rect would begin below the tabs. Okay why do we not  only use a client rect for controls that really use it? Because of simplicity and polymorphism. If we can treat all components the same, we don't need to build in specail cases for those who use or don't  use a client rect. If all parents pass their client rect to their children, it is a quick and simple process.

When it comes to maintaining and initialisation, this is again OOP. The object is handling it. As you can see, set_groesse and set_pos does the handling and both pos and groesse could infact beprivate to force the use of set pos/groesse.

zeichnen(koord) will migrate to display(scr_rect client). The scr_rect makes it easy to move, add, substract, expand, contract the area.
If you want a margin, just contract the client rect and, voila, all children automatically moves in.

QuoteAutosizing buttons
What do you refer to here? Do you mean the auto size mode for gui_image_t? This is how the gui_image_t always has worked. The size is set to the size of the image. I made it possible to set the image area to a fixed size and then have the image to align inside it (as you suggested in an earlier post)

QuoteAbout gui_map_preview: What is this good for? It just draw the array, so why must it be done this way?
I started to make a component to display a relief map, but got a bit unsure of how the graphics was stored. Then realized that the minimap and city info dialogue also shows a relief map. I did ask if it was possible to use this for the welt dialogue but I got no answers...

If it is possible to reuse the minimmap or city information I think this will be better. If they all use their own code, it would be a good thing to break it out as a separate gui control.

To answer your question: I started on a new control, but got interrupted and then didn't know what to do. Pleas give me some advice if we can reuse something else...

QuoteThe autohide scrollbars is something I have to get used too; but it is certainly innovative. (Actually, is the autohide graphic smaller than the previous scrollbar?)
I mentioned it before and you(?) said it would be best to hide the whole control. I did that but it looked odd if the client area wasn't expanded to fill out the space, so I tried a place holder (proxy) instead. Yes it is slight smaller, it was too eye catching when it had the same size. However it is configurable as, always hide (still responds on mouse wheel), always show or autohide (drawing a proxy when full).

We will see how this evolves... The best would be to have the client area ( :police: ) to expand and completely hide the scrollbar.

So some questions...
Why can't I add new files to a patch? Or did the patch work for you?
- My code doesn't have bugs. It develops random features...

Ters

The problem with conversion operators is that you can get a much faster conversion to scr_coord than you expected. You could make them explicit though, unless there are compiler issues with that. That still allows for comparatively easy conversion, but only when you explicitly ask for it.

Max-Max

The scr_coord.h seems to be missing in the trunk. Is this intentionally?
- My code doesn't have bugs. It develops random features...

prissi

No the missing scr_coord.h is an accident. It seems I submitted in the wrong directory anyway, and also omitted a lot of other changes. Here comes the corrected version. It also features an eclipse string from translater as those are quite language dependent (there is even an Unicode character for japanese).

You can easily have a constructor for scr requiring explicit parameters. That way not scr_coord() is possible. Or just do not initialized the parameters at all and let the program/debugger complain when using them.

BTW: the patch works for me (i.e. adds the new files).

Max-Max

Thank you Prissi...

For the next patch I will transform scr_xxx_xxx to classes. As you suggested, I can hide the default constructor :)

Should I move scr_coord.h to the same folder as koord and koord3d?
- My code doesn't have bugs. It develops random features...

captain crunch

I had to make some changes to the Makefile and gui/components/gui_map_preview.cc
to successfully compile this.

Max-Max

I have applied your patch into my working copy. But maybe Prissi should do a quick fix in the main trunk too.

I'm on VC++ 2012 Express so I can't really test on other systems or modify the VC++ solution file (VC++ 2010). Unless we make the decision to upgrade the VC++ 2010 project file to VC++ 2012 Express.
- My code doesn't have bugs. It develops random features...

kierongreen

Actually came up with the same modifications as in the gui_theme_8aca7d54 patch to get Linux compile working before seeing this thread! Have committed them in r6608 anyway :)

Max-Max

- My code doesn't have bugs. It develops random features...

prissi

the scr_coord.h should be maybe go into gui folder, as it only affects display and not the map. Or maybe all drawing stuff should go to a display folder, including simview and the like.

Max-Max

It sounds like a good idea to make a display folder to collect all the drawing stuff.
- My code doesn't have bugs. It develops random features...

Max-Max

#233
Prissi,

Have you removed all my client stuff from the gui?

Think of the client rect as a margin for a controls children. the macros D_MARGIN_XXXX is a client rect, but instead of 4 variables you have a rect class that can be added, subtracted, compared etc... I don't see how this can be more confusing than the current system. By sending a client rect the children don't need to know anything about their parent's dimensions.


Sorry, disregard this... My fault... :)
- My code doesn't have bugs. It develops random features...

prissi

About the client: Perhaps a routine like "set_size_to_contain_all" or so coudl achieve what your wanted to to the client rectangle so far.

And still, I am not opposed to a rectangle class. I would rather prefer x, y, w, and h, but I would also live with top_left.x, top_left.y and right_bottom.x and right_bottom.y or simple top, left, bottom, right. I would like to avoid x, y, xx, yy which are confusing.

And yes, as display folder will help. The only thing is, what to put in there. Simwin.cc maybe belong ratehr into the gui folder? Or putting it there, next to the ticker?

Max-Max

This patch has some adjusted label widths and the some moved files to a new folder /display/ as discussed earlier.

Since we moved some files this patch is very big because all #includes "xxx" had to be updated in a looot of files.

I haven't included my VC++ 2012 project and solution file, but I tried to update the make file (without being able to test it).
- My code doesn't have bugs. It develops random features...

TurfIt

Committed r6624 (in a fit of insomnia...)

Note: files cannot be moved by patch, must be done directly by svn move or else all history will be lost.
Also: your patch contained two copies of several of the moved files - applying patch appends these two to the same file, pain trimming. Something wrong with whatever program you used to diff...
I unreverted your reversion to dwachs r6616 commit. Why revert this...
Not all references to moved files were updated - e.g. all the simsys_* variants.
Const cast warnings in scr_coord.h, I added extra consts to suppress the 38482284332 warnings that came out., check if this still works as you intended.
Patch had files with trailing whitespace on lines - cannot commit like this - svn rejects.
Patch had files with trailing LFs - cannot commit - rejects.
Patch had files with no LF on last line - cannot commit - rejects.
Patch had files with lines containing only whitespace - cannot commit - rejects.

Please watch those svn rejected items on future patches. It's a pain fixing afterwards...
Done griping.  8)



kierongreen

Brilliant - well done TurfIt (and Mad-Max of course). I'd just started looking at the patch when I luckily ran svn update and saw someone had beaten me to it!

Ters

What's up with tag_scr_coord_t? Some crazy compiler compatibility? This is C++, structs are first-class datatypes.

Max-Max

I'm not an expert on SVN and for some reason I can't merge your trunk with my branch directly.
First I have to manually copy the trunk to a local "master trunk" and check that into my local repository before I can merge it with my local Theme copy.

I do everything in SVN (adding, removing, renaming etc...) and then copy my working copy to my copy of your trunk manually. Now I have your version number so I can create a patch.

I did go through all files to manually clear all moved files, check that files where added and removed properly, compile and test.

This is of course a pain for me too, because if your trunk has been updated in between I have to repeat the whole procedure and merge it with my Theme test copy again etc...

For some reason not all my SVN operations makes it into the patch file...

Regarding scr_coord and scr_rect, I just forgot to change them to classes in this patch... They will be classes...

Thank you TurfIt for your work, I can imagine it must have been a lot.
- My code doesn't have bugs. It develops random features...

kierongreen

QuoteThis is of course a pain for me too, because if your trunk has been updated in between I have to repeat the whole procedure and merge it with my Theme test copy again etc...
I know from experience how it's a pain keeping a local branch with a large number of modifications up to date with trunk. Remember it'll be worth it in the end though :)

Ters

Using a distributed VCS like Git or Mercurial on your own machine at least gives you proper branches to work with locally. They can both be set up to pull from SVN. But I don't know how it is to take out patches from a branch, have someone commit them to SVN, then get them in via pull from SVN and then merge over to branch again, where the changes already exists. Git probably has better support, while Mercurial is more familiar to those who know SVN.

TurfIt

Thanks dwachs, kierongreen for fixing things back up. text_pixel.c was strange, it worked fine here showing it moved, and the file was definitely intact in its new spot, but now 'svn up -r6624' sets it to 0 bytes. Hiccup with my checkin maybe??

svn file additions, deletions, moves, properties, etc. don't survive the diff/patch process. Best to avoid when doing patches...

A simple 'svn up' should take care of it. However, if the update is expected to produce some conflicts, I save my work into a diff first. 'svn diff > blah.diff', then open blah.diff in an editor and make sure it makes sense/has all the files, then 'svn revert -R ./' to blow away all my changes. Then 'svn up' against the virgin older trunk. Then 'patch -p0 < blah.diff' to put my work back - and deal with any conflicts (.rejs) that result. This procedure gives me backups of my work by keeping the .diffs, and a way to keep projects separate while only using a single working copy, and makes merging the right way around IMHO. Merge your changes (which you should know) into the new trunk, rather than trying to merge somebody elses unknown code into yours (why the git process makes no sense to me).

---
The proxy for the autohiding scrollbar is rather ugly. IMHO if the skin doesn't provide appropriate images for the disabled/hidden state (not really hidden when something's drawn???) the old behaviour should be retained. The old behaviour should also be easily selectable by the user IMHO - autohiding like this is another of those strongly love it/hate it things.

kierongreen

QuoteA simple 'svn up' should take care of it. However, if the update is expected to produce some conflicts, I save my work into a diff first. 'svn diff > blah.diff', then open blah.diff in an editor and make sure it makes sense/has all the files, then 'svn revert -R ./' to blow away all my changes. Then 'svn up' against the virgin older trunk. Then 'patch -p0 < blah.diff' to put my work back - and deal with any conflicts (.rejs) that result. This procedure gives me backups of my work by keeping the .diffs, and a way to keep projects separate while only using a single working copy, and makes merging the right way around IMHO. Merge your changes (which you should know) into the new trunk, rather than trying to merge somebody elses unknown code into yours (why the git process makes no sense to me).
I use exactly this method to maintain branches on my computer (the new landscape patch was like this for a year). I also can't get the hang of git...

prissi

I did a lot of work on scr_coord class to remove the tags and make a readable class. Why has this been reverted?

I am also still against having client rects with containers as variables, as stated various times before. Also using containers for the sole purpose of grouping labels is very hackish. Well, I left it so for the moment (with a little change, since I did not quite understood the align procedure. Imho now there is too much in align, as it influences size too, if I got this correctly.

I am not convinced to make gui_frame a part of gui_component. Those are very different, since frame have help files, owner and so on.

Please comment what these ALIGN_INTERIOR_... are doing. I have no good idea, and your calls to the align_to procedure produces more crytic code than the explicit calcualtion before. Please, mroe comments to align_to.

ALIGN_INTERIOR_V = 0x00,
ALIGN_EXTERIOR_V = 0x10,