TGUI Forum

General Category => Feature requests => Topic started by: AlexxanderX on 11 December 2015, 15:23:23

Title: Table
Post by: AlexxanderX on 11 December 2015, 15:23:23
For my game I need a table and Grid doesn't satisfy me. I started working on creating a table, but I want to know if the idea is good: a Row class that's a HorizontalLayout with functions like addItem(string column) who to add Label to the layout. Then, in the class Tabel a function to add the rows to the container. Tabel class to be a BoxLayout and to arrange the items vertically, but one after one.
Title: Re: Table
Post by: texus on 11 December 2015, 15:33:09
The idea sounds good enough, if it works then just do it like that. But can't Table be a VerticalLayout if it only has to put the lines below each other?

Just make sure to use the latest version and not 0.7-alpha2 as that release has a serious bug in the BoxLayout class.

Feel free to send me the code when you have a working table class, I think several other people will be interested in having such a class available in tgui.
Title: Re: Table
Post by: AlexxanderX on 11 December 2015, 15:44:11
Yes, it should be a VerticalBox, but if is a VerticalBox then I think I need to resize table everytime a new row is added or to make every row fixed, because if I using VerticalBox and set the table size then every row is aligned in that space and not one below one. And I thinked if I only need to put one below one, then a for is enough so when updated not need to do all the calculations that are done in VerticalBox.

Now I need to see how to set the size of the Label because now It give me {100,100} and it need to be minimum for height.
Title: Re: Table
Post by: texus on 11 December 2015, 15:52:34
The (100,100) is the default size of a panel on which is what BoxLayout is based on. Normally you know the height of a VerticalLayout because it is a fixed value. One thing you could do is create an empty label which you give a font and a text size and then use label->getSize().y as the height of your VerticalLayout. Or you could just not use any of these BoxLayouts at all and have full control over the position and sizes of everything, although that would make your code more difficult.
Title: Re: Table
Post by: AlexxanderX on 11 December 2015, 16:04:55
I think I need to do the modifications in updateWidgetPositions() or somewhere after the user add it to the gui, so we have the font.
Title: Re: Table
Post by: AlexxanderX on 11 December 2015, 22:46:10
I made some progress: I created a TableItem which is a BoxLayout which contains only 1 widget, with the possibility to set the align of widget(left, center, right)( not done, only center)( it would be nice if you would add padding properties to BoxLayout). Then in the TableRow(HorizontalLayout) you can add directly TableItem through a function which take a std::string or to add another widget( like a button). The class Table(BoxLayout) take the TableRow and put it one another other. Now I need to rewrite the Table's draw() to can add table bars between columns and rows.
Title: Re: Table
Post by: texus on 11 December 2015, 23:02:38
Quote
BoxLayout which contains only 1 widget
Quote
it would be nice if you would add padding properties to BoxLayout
Why does it has to be in a BoxLayout when there is only one widget? The BoxLayout is created to place widgets next to each other or below to each other, not to store single widgets.
Title: Re: Table
Post by: AlexxanderX on 11 December 2015, 23:08:26
It is just a quick hack to make it work because I entered in some errors when inherited from a Widget and because of time I inherited from a BoxLayout.
Quote
E:/Work/Programare/Libs/SFML/include/SFML/Graphics/Drawable.hpp: In member function 'virtual void tgui::TableItem::draw(sf::RenderTarget&, sf::RenderStates) const':
E:/Work/Programare/Libs/SFML/include/SFML/Graphics/Drawable.hpp:69:18: error: 'virtual void sf::Drawable::draw(sf::RenderTarget&, sf::RenderStates) const' is protected
     virtual void draw(RenderTarget& target, RenderStates states) const = 0;
                  ^
E:\Work\Programare\Apps\CPP\Katan\Client\src\xalTGUI\TableItem.cpp:19:37: error: within this context
         mWidget->draw(target, states);
Title: Re: Table
Post by: texus on 11 December 2015, 23:09:58
Quote
mWidget->draw(*target, states);
The draw function (which comes from sf::Drawable) is not meant to be called directly. You have to write the line like this:
target->draw(mWidget, states);
Title: Re: Table
Post by: AlexxanderX on 12 December 2015, 08:25:48
This was my first try:
Code: [Select]
virtual void draw(sf::RenderTarget& target, sf::RenderStates states) const override {
            if (mWidget != nullptr) {
                target.draw(*mWidget, states);
            }
        }
But doesn't draw anything.

EDIT: Studying the Container's code I saw the function setFont() and implemented it and now the draw works.
Title: Re: Table
Post by: AlexxanderX on 12 December 2015, 10:16:16
(https://s11.postimg.org/uk30zlvrn/aaaaa.png)

Now working on adding any widget to TableRow and next I will do some "hover effect" when mouse is over a row. After I will clean the code I will upload it. The only thing that is not really good( or maybe I'm wrong) is that the user need to call:
Code: [Select]
table->setFontSize(18, mGui.getFont()); in order to everything to work properly.

The header is a separated TableRow( is not added in the container) because in the future, when you will wants to add a scroll option you will want to have the header visible and only the content rows to move.

Still need to think how to do with the size of columns: if user add a button with size bigger then the table. I think of adding a function in table in which to say if to set the table row height to 'fixed' or 'auto' and if is fixed to set all the widgets size to that size, and if auto, the table to check what is the biggest height and apply to all rows.
Title: Re: Table
Post by: texus on 12 December 2015, 11:50:16
It's already looking good. But I would personally add a line below the Name and IP because now it looks like a multi-column listbox where the first line is selected. I'm not sure if this is the case or not, but it is best that the first line (the one with Name and IP) is handled separately from all the other lines. That would allow adding customization options in the future (making the column names bold or giving them a bigger text size).

Quote
table->setFontSize(18, mGui.getFont());
in order to everything to work properly.
It would be best that this are 2 different functions, setTextSize and setFont. When setTextSize is called and there is still no font nothing happens but the size is stored. When setFont is called (or when setTextSize gets called while there already is a font) then it should do what you currently do in setFontSize. But I can easily make changes like this myself when it is finished, so it doesn't matter that it requires such a function now.

Quote
if to set the table row height to 'fixed' or 'auto' and if is fixed to set all the widgets size to that size, and if auto, the table to check what is the biggest height and apply to all rows.
Another option that the user might want is having all rows fixed but just have that one row a bit bigger. But I guess the 'fixed' and 'auto' options would be more common than this so it is enough to have them.
Title: Re: Table
Post by: AlexxanderX on 12 December 2015, 15:40:23
I wants to do something like "setRowsColor()" but how to change Widget::Ptr to TableRow::Ptr? Tried "std::static_pointer_cast" but doesn't work( it compile, but have no effect). I think a solution is to make a template from class Container with default template argument a Widget::Ptr. With a template Container I can set the Table to accept only TableRows from functions add, insert.
Title: Re: Table
Post by: texus on 12 December 2015, 15:46:09
You don't need templates to accept only TableRow::Ptr, just make the parameter a TableRow::Ptr instead of a Widget::Ptr.
Templates and inheritance are basically opposites, if you need templates while writing the Table class then you are doing something wrong and should reconsider what you are trying to do.

Quote
Tried "std::static_pointer_cast" but doesn't work( it compile, but have no effect)
It is the correct way and if it compiles with TableRow but not with Widget then there must me a mistake somewhere with the function you are calling. There is one thing that could cause stange results though: did you forget to add "typedef std::shared_ptr<TableRow> Ptr;" to the TableRow class? If you forgot that then TableRow::Ptr is actually the same as Widget::Ptr which is why the cast wouldn't make any difference.
Title: Re: Table
Post by: texus on 12 December 2015, 15:48:50
I realize now what you were talking about with making Container templated. Maybe inheriting from Container isn't such a good idea, it should remain a generic class that works with just Widget. Also it isn't very user friendly if the user has to manually create those TableRow instances.
Title: Re: Table
Post by: AlexxanderX on 12 December 2015, 16:13:23
"std::static_pointer_cast" is working. My bad: I called the function before rows were added. Here is a nicer table:
(https://s13.postimg.org/6tsywmkw7/aasdwdq.png)
Title: Re: Table
Post by: AlexxanderX on 13 December 2015, 21:07:32
Don't worked too much today, but on what I worked I encountered a problem: I need to get the ratio for a widget but I can't find a function like getRatio() in BoxLayout.
Title: Re: Table
Post by: texus on 13 December 2015, 21:11:16
Either you wait and I will try to add such a function later today, or you can just add it yourself.
Title: Re: Table
Post by: texus on 13 December 2015, 22:59:59
I have added the getRatio function (and a getFixedSize function) in the latest version.
Title: Re: Table
Post by: AlexxanderX on 14 December 2015, 14:51:48
I don't have much time to finish the table so I upload the work: https://bitbucket.org/AlexxanderX/tguitable (https://bitbucket.org/AlexxanderX/tguitable). Some notes:
Title: Re: Table
Post by: texus on 14 December 2015, 14:54:08
I have a very busy week myself, so I will take a look at it next week.
Title: Re: Table
Post by: AlexxanderX on 14 December 2015, 14:55:35
In the time I will still work on it, but yea, pretty busy week here too :D
Title: Re: Table
Post by: texus on 23 December 2015, 14:22:10
I have some time in the next few days to help with the Table class.

Could you perhaps make the code independent of your xal code? If the headers would not be in a xalTGUI subfolder, you would remove the Global.hpp include and if you would make example.cpp a standalone example code then I can just clone your repository and compile and run here. That would make it a lot easier for me to help with writing the code.
Title: Re: Table
Post by: AlexxanderX on 23 December 2015, 16:56:54
Done ;)
Title: Re: Table
Post by: texus on 23 December 2015, 18:44:34
Here are some remarks on the code. I can fix some of these things, but maybe you want to give your opinion on some of these points first.

There should be some function that recalculates the entire widget. The reason why you have to call setFont manually is because otherwise it only gets the font when gui.add is called. Even if I call updateColumnsDelimitatorsSize and updateWidgetPositions in the setFont function the text still doesn't show on the correct location because the only place where you set the height of a row is in the insert function which is called before the font is set. The setFont function should trigger some recalculation because changing the font might result in different text sizes.

Was there any reason to use BoxLayout instead of VerticalLayout in Table?

Perhaps HorizontalAlign::None can be renamed to HorizontalAlign::Default and then in the documentation of the setItemsHorizontalAlign function we can state that it changes the default alignment. This way it doesn't has to be marked as internal because using that would thus result in a clear documented behavior. The only problem would be when the user tries to pass Default to setItemsHorizontalAlign, but we can interpret that as the default gui behavior and thus set it to Left (or Center).

For setTextColor and setNormalTextColor you can loop over all widgets and check the result of dynamic_pointer_cast<tgui::Label>. If it is nullptr then it wasn't a label and you just ignore it, if it isn't nullptr then it is a label and you can set its color. Alternatively you can hold a boolean in the items but since the color is not changed much the little overhead of dynamic cast definately shouldn't be a problem.

For the mouseOnWidget function instead of passing (x-getPosition().x,y-getPosition().y) you just had to pass (x,y) but it seems like that is not enough to show the hover picure. The problem is also that your TableItem is receiving all events but is not relaying it to the widget (the other functions have to implemented like mouseOnWidget is now). It would be better if I fix this because I know more about this, but I do start to wonder if having a TableItem widget is really needed. Maybe the TableRow should just take widgets directly and be responsible for positioning them (the alignments would be stored in TableRow as a vector).
Title: Re: Table
Post by: AlexxanderX on 23 December 2015, 20:13:15
Yes, there should be some calculations when the font or characters size is changed.

I don't really remember why I have chosen BoxLayout instead of VerticalLayout, but I think because I entirely rewrite the updateWidgetPositions(), so it's no more a legit VerticalLayout, but something else.

Well, need to think on this...

Yes, I never thought of the function returning nullptr.

I created the class TableItem for alignment of the widget. If you think would be easier to manage the alignment of every item of row in TableRow then go ahead. I already implemented some basic code for the events but don't work very well so I don't added in the repository.
Title: Re: Table
Post by: texus on 23 December 2015, 20:32:10
I think it would be best if you wrote the code to recalculate the positions and sizes of everything because I don't know yet what all variables do. Actually there probably shouldn't be a new function, updateWidgetPositions should just be extended to also set the sizes of the rows instead of only the positions. And then setFont should just call updateWidgetPositions at the end and the problem with manually having to call setFont will be gone as well.

It is a good point about it no longer being a VerticalLayout. I didn't know that VerticalLayout only contained an updateWidgetPositions function, I though it contained more.

I will take care of the setTextColor and setNormalTextColor and I will have a look at the TableItem and the events (tomorrow).
I will also change the insert function so that it will create a TableRow and insert the widget in it when you call insert with a widget other than TableRow.
Title: Re: Table
Post by: texus on 24 December 2015, 13:43:20
Should setTextColor and setNormalTextColor actually do something to the existing labels? Because if the user has created some labels with a custom color then they would get changed as well. It might be better to just say that these functions set the color of all future labels and thus doesn't affect the ones that already exist.
Title: Re: Table
Post by: texus on 24 December 2015, 18:15:40
You can find my changes here: https://bitbucket.org/texus/tguitable
I have added a few things, but I also seem to have broken a few things.
Title: Re: Table
Post by: texus on 25 December 2015, 00:03:38
I fixed the remaining problems, you should pull my version and see if there is still something missing or not.
Title: Re: Table
Post by: AlexxanderX on 26 December 2015, 11:34:17
Sorry for late response. I will try it, but Monday, after all the feast past.
Title: Re: Table
Post by: texus on 26 December 2015, 13:10:25
The next tgui release is planned in February, so there's no rush :)
Title: Re: Table
Post by: texus on 14 January 2016, 19:51:22
Did you already found some time to look at my version?
Title: Re: Table
Post by: AlexxanderX on 15 January 2016, 11:44:54
So I made time to finish the table. Why would you want to combine the size function with the position function? I want to say that updateWidgetPosition() is pretty frequently called and would be useless, only if somehow check if the size changed. What I need to resolve? I will also add comments in order to explain what I do.
Title: Re: Table
Post by: texus on 15 January 2016, 13:49:38
The updateWidgetPosition will only be called when widgets are changed, which isn't that frequent. I only care about performance when something is called almost every frame, but this function is only called a lot while creating the table (while adding the rows). The setPosition and setSize functions of widgets will become very light operations in the near future so the function won't be that heavy.

The Table class on my bitbucket (https://bitbucket.org/texus/tguitable/) looks finished at first sight. So you don't really have to resolve anything unless you find things that are still missing. It juststill has to be extensively tested.