Main Menu

Table

Started by AlexxanderX, 11 December 2015, 15:23:23

AlexxanderX

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.

texus

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.

AlexxanderX

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.

texus

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.

AlexxanderX

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.

AlexxanderX

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.

texus

QuoteBoxLayout which contains only 1 widget
Quoteit 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.

AlexxanderX

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.
QuoteE:/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);

texus

QuotemWidget->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:
Code (cpp) Select
target->draw(mWidget, states);

AlexxanderX

#9
This was my first try:
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.

AlexxanderX



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: 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.

texus

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
Code (cpp) Select
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.

Quoteif 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.

AlexxanderX

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.

texus

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.

QuoteTried "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.

texus

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.

AlexxanderX

"std::static_pointer_cast" is working. My bad: I called the function before rows were added. Here is a nicer table:

AlexxanderX

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.

texus

Either you wait and I will try to add such a function later today, or you can just add it yourself.

texus

I have added the getRatio function (and a getFixedSize function) in the latest version.

AlexxanderX

I don't have much time to finish the table so I upload the work: https://bitbucket.org/AlexxanderX/tguitable. Some notes:

  • setTextColor() from Table and setNormalTextColor() from TableRow are not finished, need to think how to set it because it may not contain only labels;
  • With the getCustomHeight() from TableRow I check if the row contains widgets bigger then the label( a button). If is not bigger then in the Table's insert() I check it;
  • I separated the sizing of rows from Table's updateWidgetPositions() in order to make work the custom row sizes;
  • TableItem's mouseOnWidget() doesn't work, I don't really investigated to much how should be in order widgets like button to work
  • TableItem's Align::None is just for internal coding: in order to know if need to use the set align in adds function from TableRow

texus

I have a very busy week myself, so I will take a look at it next week.

AlexxanderX

In the time I will still work on it, but yea, pretty busy week here too :D

texus

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.

AlexxanderX


texus

#24
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).