Author Topic: create an array of comboboxes  (Read 1537 times)

0 Members and 1 Guest are viewing this topic.

Online Ves

create an array of comboboxes
« on: July 12, 2017, 09:48:08 PM »
Hi all, and hope you enjoy the summer! 8)
I want to create an array of comboboxes in the GUI for the new passenger and mail class system which is used in Extended. I want to create as many comboboxes in a row below each other as there are classes in the vehicle, and the comboxes are used to change the class to another one. The window I am working in is identical to the vehicle_detail_t window.
I have checked how arrays of buttons are created, and translated that to comboboxes:

Code: [Select]
buf.clear();
gui_combobox_t *cs = new gui_combobox_t();
cs->set_pos(scr_coord(pos.x + w + offset.x, pos.y + offset.y + total_height + extra_y));
cs->set_size(scr_size(185, D_BUTTON_HEIGHT));
cs->set_max_size(scr_size(D_BUTTON_WIDTH - 8, LINESPACE * 3 + 2 + 16));
cs->set_highlight_color(1);
cs->clear_elements();
class_indices.clear();
for (uint8 j = 0; j < v->get_desc()->get_number_of_classes(); j++)
{
char class_selector_name_untranslated[32];
sprintf(class_selector_name_untranslated, "p_class[%u]", j);
const char* class_selector_name = translator::translate(class_selector_name_untranslated);
cs->append_element(new gui_scrolled_list_t::const_text_scrollitem_t(translator::translate(class_selector_name), SYSCOL_TEXT));
class_indices.append(j);
}
cont.add_component(cs);
cs->add_listener(this);
cs->set_focusable(false);
extra_y += LINESPACE;

I get a red underline under "this" in cs->add_listener(this); stating that argument of type "gui_class_vehicleinfo_t *" is incompatible with parameter of type "action_listener_t *".
Adding:

Code: [Select]
bool action_triggered(gui_action_creator_t*, value_t) OVERRIDE;
to gui_class_vehicleinfo_t doesnt help either.

I cannot find any existing examples of that in the game, only arrays of buttons and other stuff. I am especially interrested in how to code the ::action_triggered part, as I still dont understand how that corelates with arrays of buttons, or in my case, comboboxes.
Would someone be able to help demonstrate how to make arrays of comboboxes and make them work with ::action_triggered?
« Last Edit: July 18, 2017, 05:02:51 PM by Ves »

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8830
  • Total likes: 324
  • Helpful: 229
  • Languages: De,EN,JP
Re: create an array of scrolled lists
« Reply #1 on: July 13, 2017, 08:51:11 AM »
And array of comboboxes might not be a good idea, as the focus (i.e. combobox) is remembered in order to close it, when clicked outside.

I would rather circle the content of the comboboxes on click.

Online Ves

Re: create an array of scrolled lists
« Reply #2 on: July 14, 2017, 07:48:35 AM »
Thanks for your reply, what do you mean with that you would rather circle the content of the comboboxes on click? Do you mean that you have one combo box that determines the value that should be changed, and another box that change the actual value?

I might have described badly what I would like: A list of selectors. Similar to the one where you select wether to buy or sell in the depot. I only just don't know how many of them is needed, hence the array.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8830
  • Total likes: 324
  • Helpful: 229
  • Languages: De,EN,JP
Re: create an array of scrolled lists
« Reply #3 on: July 16, 2017, 03:11:21 PM »
As long as it is a static array, you can put anything into it. But it must be an array to pointers to comboboxes, and you need to check the focus in the deletion routine and close any open first. The dielogue itself must no know of the array, you just add them one by one. Hence if "this" in your extract is the init routine of the dialogue (or a scrolled list, or anything else derived from gui_container_t) then it should work. However, this cannot by the array.

I think the full routine is needed to know what the problem is.

Online Ves

Re: create an array of comboboxes
« Reply #4 on: July 18, 2017, 08:04:46 PM »
Thanks! James helped me out a bit, so its only the "this" part that doesnt work it seems.

So what you are saying is that I cannot define the comboboxes directly in the array? Should I define ONE combobox in the initial routine and then point to that using pointers?

If it not is too much to ask for, could you take a look at the file in question and maybe give an example of what you describe?
Currently the button is defined around line 352 to 377 in an array in this file: https://github.com/VictorErik/Simutrans-Experimental-Ves/blob/b7b36e1e6865d70a419ad4881f2f40fafd75921b/gui/vehicle_class_manager.cc
The "this" line is commented out currently and compiles as such, but nothing is visible in the game.

Offline TurfIt

Re: create an array of comboboxes
« Reply #5 on: July 19, 2017, 01:07:53 AM »
You're trying to have gui_class_vehicleinfo_t respond to events (actions) created by the combobox gui element, but it's lacking the interface. i.e. The combobox has the interface class action_creator_t but gui_class_vehicleinfo_t is missing the corresponding ifc action_listener_t.

I think what prissi is trying to say is your actual gui_frame_t that's responding to the infowin events needs knowledge of the comboboxes contained within to possibly play some hackey focus games to get the comboboxes to close properly. The depot dialogue has some of that going on...

I can't really figure out what the structure of your dialog is supposed to be from your code... certainly you don't want those combobox manipulations in the ::draw(); Should be in an init(), or update_data(), or action_triggered, or something that's not called every frame.

Online Ves

Re: create an array of comboboxes
« Reply #6 on: July 19, 2017, 08:31:17 AM »
Thank you very much!
I think I have understood somethings much better now, as I can now get the comboboxes visible in game and show up and disappear when they are supposed to!

Online Ves

Re: create an array of comboboxes
« Reply #7 on: July 22, 2017, 02:58:17 PM »
I am getting a bit stuck with some things:

First, I struggle to make the content of the scrolled lists show anything sensible. I have defined the array of combo boxes like this:

Code: [Select]
int pass_class_capacity[255] = { 0 };
for (uint8 i = 0; i < goods_manager_t::passengers->get_number_of_classes(); i++)
{
for (unsigned veh = 0; veh < cnv->get_vehicle_count(); veh++)
{
vehicle_t* v = cnv->get_vehicle(veh);
if (v->get_cargo_type()->get_catg_index() == 0)
{
pass_class_capacity[i] += v->get_capacity(i);
}
}
if (pass_class_capacity[i] > 0)
{
gui_combobox_t *class_selector = new gui_combobox_t();
class_selector->set_pos(scr_coord(100, i*LINESPACE));
class_selector->set_size(scr_size(185, D_BUTTON_HEIGHT));
class_selector->set_max_size(scr_size(D_BUTTON_WIDTH - 8, LINESPACE * 3 + 2 + 16));
class_selector->set_highlight_color(1);
class_selector->clear_elements();
class_indices.clear();
for (uint8 j = 0; j < goods_manager_t::passengers->get_number_of_classes(); j++)
{
char class_selector_name_untranslated[32];
sprintf(class_selector_name_untranslated, "p_class[%u]", j);
class_selector->append_element(new gui_scrolled_list_t::const_text_scrollitem_t(translator::translate(class_selector_name_untranslated), SYSCOL_TEXT));
class_indices.append(j);
class_selector->set_selection(j);
}
add_component(class_selector);
class_selector->add_listener(this);
class_selector->set_focusable(false);
class_selectors.append(class_selector);
}
}

... which shows up nicely as expected in the game. The logic tells the window to add a combobox for every class that the convoy holds in total.
However, only the last entry shows the expected value and the rest of the content of the comboboxes shows []. There appears to be the correct number of [] -entries, and the last entry is the anticipated one. If you try to change the value in the combobox, the expected value disappears and only [] shows in its place.
I have imitated the livery selector from experimental which looks like this:

Code: [Select]
livery_selector.add_listener(this);
add_component(&livery_selector);
livery_selector.clear_elements();
vector_tpl<livery_scheme_t*>* schemes = welt->get_settings().get_livery_schemes();
livery_scheme_indices.clear();
ITERATE_PTR(schemes, i)
{
livery_scheme_t* scheme = schemes->get_element(i);
if(scheme->is_available(welt->get_timeline_year_month()))
{
livery_selector.append_element(new gui_scrolled_list_t::const_text_scrollitem_t(translator::translate(scheme->get_name()), SYSCOL_TEXT));
livery_scheme_indices.append(i);
livery_selector.set_selection(i);
livery_scheme_index = i;
}
}
... the main difference being that James has used "ITERATE_PTR" and I have used just a "for(...)".
It seems to me like the entries in my comboboxes overwrite them selfs, why only the last entry is preserved for display. What am I doing wrong?


The other issue I have is that I dont understand how to use the action creator when I have a list of comboboxes (or even buttons for that matter..). When I look at a normal combobox entry, like the livery selector or buy/sell options, it kinds of makes sence to me. However, when looking at array entries, like the destination buttons for the halt_detail window, I dont really understand how the action creator gets which button is pressed. Even less, I would understand how to make the combobox-array entries in there. Could somebody explain this to me or give me an example of how to do it? I have updated the github with the working array of boxes.

Thank you both for your help!

Offline TurfIt

Re: create an array of comboboxes
« Reply #8 on: July 22, 2017, 07:11:57 PM »
I have updated the github with the working array of boxes.
Can't find anything....


However, only the last entry shows the expected value and the rest of the content of the comboboxes shows [].
gui_scrolled_list_t::const_text_scrollitem_t    <- const should be the giveaway on what this expects. A reused string on the stack ain't it.


I dont really understand how the action creator gets which button is pressed.
The action creator *is* the button. It knows where it is, swallows the window event, and calls its listeners when clicked on. Unless you're implementing some new gui component, you don't really need to be concerned with this.
If the listener is listening to multiple creators, then it needs to distinguish who is calling... that's what the first parameter passed in ::action_triggered() is used for. Compare it against your selectors.

Online Ves

create an array of comboboxes
« Reply #9 on: July 22, 2017, 09:29:27 PM »
Sorry my bad, here is the link (changed gitub branch) https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/passenger-and-mail-classes-gui-combolist

Ok, so I have to create the list of names before I give them to the gui_scrolled_list... ?

The comboboxes are supposed to change some values to the convoy specified, I do need to use the action triggered then?
« Last Edit: July 22, 2017, 09:58:03 PM by Ves »

Online Ves

Re: create an array of comboboxes
« Reply #10 on: July 23, 2017, 07:45:51 PM »
I found a way to get the entries on the list show up. I dont know if this is failproof in all circumstances but it appears to work:

Code: [Select]
int pass_class_capacity[255] = { 0 };
for (uint8 i = 0; i < goods_manager_t::passengers->get_number_of_classes(); i++)
{
for (unsigned veh = 0; veh < cnv->get_vehicle_count(); veh++)
{
vehicle_t* v = cnv->get_vehicle(veh);
if (v->get_cargo_type()->get_catg_index() == 0)
{
pass_class_capacity[i] += v->get_capacity(i);
}
}
if (pass_class_capacity[i] > 0)
{
gui_combobox_t *class_selector = new gui_combobox_t();
class_selector->set_pos(scr_coord(100, i*LINESPACE));
class_selector->set_size(scr_size(185, D_BUTTON_HEIGHT));
class_selector->set_max_size(scr_size(D_BUTTON_WIDTH - 8, LINESPACE * 3 + 2 + 16));
class_selector->set_highlight_color(1);
class_selector->clear_elements();
class_indices.clear();

for (int j = 1; j < goods_manager_t::passengers->get_number_of_classes()+1; j++)
{
char pass_class_name_untranslated[32][510];
sprintf(pass_class_name_untranslated[j], "p_class[%u]", j-1);
class_selector->append_element(new gui_scrolled_list_t::const_text_scrollitem_t(translator::translate(pass_class_name_untranslated[j]), SYSCOL_TEXT));
class_indices.append(0);
}
add_component(class_selector);
class_selector->add_listener(this);
class_selector->set_focusable(false);
class_selectors.append(class_selector);
}
}
... the difference being this:
Code: [Select]
char pass_class_name_untranslated[32][510];
sprintf(pass_class_name_untranslated[j], "p_class[%u]", j-1);
class_selector->append_element(new gui_scrolled_list_t::const_text_scrollitem_t(translator::translate(pass_class_name_untranslated[j]), SYSCOL_TEXT));
Did I do this right?

Now the action creator still puzles me.
You write that the first parameter is used so I can compare it to my selectors, but how?
From other GUI-windows, like the halt detail with its arrays of line entries etc, it looks like something like this is specified in the beginning:
Code: [Select]
if (v.i&~1)... followed by
Code: [Select]
if(  k.x>=0  ) {}and
Code: [Select]
uint16 j=k.y;which somehow seems to specify precisely which button is is used.

How do I translate that to comboboxes?

Offline TurfIt

Re: create an array of comboboxes
« Reply #11 on: July 23, 2017, 11:37:22 PM »
Did I do this right?
Getting there... think about the scope of your variables - you need something that's valid for the duration of the open window...
(note also if you actually have translation entries for these, then it works. But bad program form to have something that crashes if an external user editable file is not perfect...)


Now the action creator still puzles me.
You write that the first parameter is used so I can compare it to my selectors, but how?
From other GUI-windows, like the halt detail with its arrays of line entries etc, it looks like something like this is specified in the beginning:
I translate that to comboboxes?
That's a bad example to be looking at - it's combining knowledge of how it layed out the buttons with the extra return parameter from button actions to fake it. i.e. In the action_triggered routine it doesn't actually directly know which button object was pressed - if you notice, it doesn't actually do anything with/to the any of the buttons.

Maybe try:
Code: [Select]
bool ai_option_t::action_triggered( gui_action_creator_t *comp, value_t v )
{
if(  comp==&construction_speed  ) {
...
}
else if(  comp==buttons+0  ) {
ai->set_road_transport( !buttons[0].pressed );
buttons[0].pressed = ai->has_road_transport();
}
else if(  comp==buttons+1  ) {
...
}
...
as an example. You have class_selectors as your list rather than buttons...

Online Ves

Re: create an array of comboboxes
« Reply #12 on: July 30, 2017, 10:00:05 PM »
YESS, it works!!   8)
Thank you TurfIt for your help with this. I had never figured this out all by my self....
As far as I can tell, it now works completely as it should, changing the values it is supposed to. Now I only need to polish it, but that should be no problem.

Quote
Getting there... think about the scope of your variables - you need something that's valid for the duration of the open window...
(note also if you actually have translation entries for these, then it works. But bad program form to have something that crashes if an external user editable file is not perfect...)
How would you construct it so it is failsafe?

James, you will have an updated version with clickable buttons within short time  ;D

Online Ves

Re: create an array of comboboxes
« Reply #13 on: August 01, 2017, 10:02:37 PM »
So, I am now having another issue with another set of comboboxes:

The window I am working with is copied from the vehicles detail window. This window contains two classes, one that holds the window and is called vehicle_class_manager_t (convoi_detail_t in the original window), and the other class is called gui_class_vehicleinfo_t (gui_vehicleinfo_t in the original window) and holds the individual vehicle entries. The comboboxes I succeded with your help to create are located in the first mentioned class.
Now, I want to add a combobox for every passenger- and mail class on every vehicle in the below section.

Following the example of the previous succes with creating comboboxes, I initially just copied the initialization of the comboboxes to
Code: [Select]
gui_class_vehicleinfo_t::gui_class_vehicleinfo_t(convoihandle_t cnv)I also added
Code: [Select]
bool gui_class_vehicleinfo_t::action_triggered(gui_action_creator_t *comp, value_t p)but when I try to put the definition in the headerfile, I just copied the corresponding entry from the succeeded attempt, but it created an error with the entry OVERRIDE.
Code: [Select]
bool action_triggered(gui_action_creator_t*, value_t) OVERRIDE;The action_triggered is redlined and tooltip says that "member function declared with 'override' does not override a base class member".
Removing the OVERRIDE part solves the trick and the redline disappears. for the good or bad....

Now returning to the cc-file, the "this" is redlined stating that "argument of type "gui_class_vehicleinfo_t *" is incompatible with parameter of type "action_listener_t *" "

Would you be able to help me out with this too?

The file is online on https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/passenger-and-mail-classes-gui-combolist and it is these files and lines that are interresting:
vehicle_class_manager.cc line 439-480
vehicle_class_manager.h line 51

Offline TurfIt

Re: create an array of comboboxes
« Reply #14 on: August 02, 2017, 12:21:53 AM »
How would you construct it so it is failsafe?
A const_text_scrollitem_t takes a pointer to a string - does not have any string storage itself.
translator::translate() returns a string pointer that works for the scrollitem if a translation match occurs. But if not, then ::translate() returns the original pointer.
If that original pointer is to a temp that goes out of scope, then that memory may be reused before the combobox displays it.
So, you need strings that have the same lifetime as the combobox. i.e. declare them together.

Allocating 2*32 sets of 1020 character strings as you've currently done is perhaps a bit overkill... especially when the contents repeat so much - no need for them to be unique with the way you're using them.
The 512 combo boxes is also  :o   I'd go back to allocating upon need as you had before, just remember to delete your news.


The action_triggered is redlined and tooltip says that "member function declared with 'override' does not override a base class member".
Removing the OVERRIDE part solves the trick and the redline disappears. for the good or bad....
That's the override keyword doing it's job. You're trying to create a virtual function, but the signature doesn't match with anything in the base class. In this case you're right back to your original issue - not inheriting from the interface class.

___
Also, if v->set_class_reassignment(i, new_class); is trying to do what I think it does, remember you can't make world simulation affecting changes directly from gui elements; Must route through a tool. Hopefully James can set that up for you, I think you'd be hopelessly lost in that.

Online Ves

Re: create an array of comboboxes
« Reply #15 on: August 02, 2017, 12:58:00 PM »
Thank you for your patience with me and my slow learning!

As far as I can see, the v->set_class_reassignment(i, new_class); works as its supposed to: It changes the correct values. Are you saying that that is not enough?

Quote
A const_text_scrollitem_t takes a pointer to a string - does not have any string storage itself.
translator::translate() returns a string pointer that works for the scrollitem if a translation match occurs. But if not, then ::translate() returns the original pointer.
If that original pointer is to a temp that goes out of scope, then that memory may be reused before the combobox displays it.
So, you need strings that have the same lifetime as the combobox. i.e. declare them together.

Allocating 2*32 sets of 1020 character strings as you've currently done is perhaps a bit overkill... especially when the contents repeat so much - no need for them to be unique with the way you're using them.
The 512 combo boxes is also  :o   I'd go back to allocating upon need as you had before, just remember to delete your news.

The amount of comboboxes (two times 255) comes from what James told me, that there might be a maximum of 255 classes for both passengers and mail. That might be overkill, but I thought I should make a combobox for each possible class.

I assume that, when you say I should declare the char together with the comboboxes, you mean like this:
Code: [Select]
for (int i = 0; i < pass_classes; i++)
{
pass_class_sel[i].set_highlight_color(1);
pass_class_sel[i].clear_elements();
char pass_class_name_untranslated[32][512];
for (int j = 0; j < pass_classes; j++)
{
sprintf(pass_class_name_untranslated[j], "p_class[%u]", j /*- 1*/);
pass_class_sel[i].append_element(new gui_scrolled_list_t::const_text_scrollitem_t(translator::translate(pass_class_name_untranslated[j]), SYSCOL_TEXT));
//class_indices.append(j);
}
add_component(pass_class_sel + i);
pass_class_sel[i].add_listener(this);
pass_class_sel[i].set_focusable(false);
offset_y += LINESPACE;
}
... and a similar one for the mail classes, but with "char mail_class_name_untranslated[32][512];", then the two first passenger selectors will have the mail class entries (m_class[ x ] instead of p_class[ x ]).
Using 256 instead of 512 gives the result of the first entry being m_class[0], then some strange signs for the next entries and finally p_class[4] as the last entry (there is only 5 entries).
Im sorry if Im not seing the obvious, but how do you delete the entries again?

Quote
That's the override keyword doing it's job. You're trying to create a virtual function, but the signature doesn't match with anything in the base class. In this case you're right back to your original issue - not inheriting from the interface class.
So, do I understand correctly that I cannot create a list of comboboxes in gui_class_vehicleinfo_t::gui_class_vehicleinfo_t(convoihandle_t cnv)?
When you and the compiler says "base class", do you mean that the vehicle_class_manager_t is the base class then?

Offline TurfIt

Re: create an array of comboboxes
« Reply #16 on: August 02, 2017, 07:06:19 PM »
As far as I can see, the v->set_class_reassignment(i, new_class); works as its supposed to: It changes the correct values. Are you saying that that is not enough?
Values that affect the simulation can't be changed directly; They must be set through a tool to ensure multiplayer synchronism.


The amount of comboboxes (two times 255) comes from what James told me, that there might be a maximum of 255 classes for both passengers and mail. That might be overkill, but I thought I should make a combobox for each possible class.
When the maximum is so far above the typical (at least I sure hope there's not normally 200+ classes), that's a good case to dynamically allocate what you need.
You were previously on the right track with:
Code: [Select]
gui_combobox_t *class_selector = new gui_combobox_t();
but you were doing this in ::draw() which is called every single frame, and at the end weren't cleaning up - memory leak.  "delete class_selectors[ x]"  in the dialogs destructor.

Your current implementation also has things being done in ::draw that shouldn't be - setting positions, etc. Do that stuff once in an init routine.


I assume that, when you say I should declare the char together with the comboboxes, you mean like this:
...
 and a similar one for the mail classes, but with "char mail_class_name_untranslated[32][512];", then the two first passenger selectors will have the mail class entries (m_class[ x ] instead of p_class[ x ]).
Using 256 instead of 512 gives the result of the first entry being m_class[0], then some strange signs for the next entries and finally p_class[4] as the last entry (there is only 5 entries).
pass_class_name_untranslated is only valid within the for loop. Something is rightly reusing that memory, hence you see gibberish when the combobox tries to display it's entries.
Since the text p_class[0], p_class[1], p_class[2], ... is common for all the boxes, you don't need a string array for each, share one. And again might as well dynamic allocate rather than 512 of them hanging around...


So, do I understand correctly that I cannot create a list of comboboxes in gui_class_vehicleinfo_t::gui_class_vehicleinfo_t(convoihandle_t cnv)?

When you and the compiler says "base class", do you mean that the vehicle_class_manager_t is the base class then?
Why not?  But if you want gui_class_vehicleinfo_t to receive actions, it needs to inherit from the action_listener_t ifc. Same as you had to add for the vehicle_class_manager_t... but, decide if it makes more sense to just have vehicle_class_manager_t respond to actions - most dialogs only have one for simplicity, unless you're planning on multiple instances of gui_class_vehicleinfo_t?

No, gui_class_vehicleinfo_t is your derived class inheriting only from gui_container_t base class. The compiler is complaining it can't find an action_triggered method in the base class to override. That's because you forgot to also inherit the action_listener_t class. The problem you started this whole thread with!

Online Ves

Re: create an array of comboboxes
« Reply #17 on: August 02, 2017, 11:31:31 PM »
Values that affect the simulation can't be changed directly; They must be set through a tool to ensure multiplayer synchronism.
I see, I will see if I can sort this out by comparing with other code.

Quote
When the maximum is so far above the typical (at least I sure hope there's not normally 200+ classes), that's a good case to dynamically allocate what you need.
You were previously on the right track with:
Code: [Select]
gui_combobox_t *class_selector = new gui_combobox_t();
but you were doing this in ::draw() which is called every single frame, and at the end weren't cleaning up - memory leak.  "delete class_selectors[ x]"  in the dialogs destructor.
Ok, I will revert back to that and get it working.

Quote
Your current implementation also has things being done in ::draw that shouldn't be - setting positions, etc. Do that stuff once in an init routine.
Other windows, like depot window and many others, have their buttons initiated first, and then in draw, the position etc is specified. Is this a bad way? Currently it seems to work fine in my window.

Quote
pass_class_name_untranslated is only valid within the for loop. Something is rightly reusing that memory, hence you see gibberish when the combobox tries to display it's entries.
Since the text p_class[0], p_class[1], p_class[2], ... is common for all the boxes, you don't need a string array for each, share one. And again might as well dynamic allocate rather than 512 of them hanging around...
I think I understand what you mean: I should create the list of names on beforehand and then let all comboboxes share the same list?

Quote
Why not?  But if you want gui_class_vehicleinfo_t to receive actions, it needs to inherit from the action_listener_t ifc. Same as you had to add for the vehicle_class_manager_t... but, decide if it makes more sense to just have vehicle_class_manager_t respond to actions - most dialogs only have one for simplicity, unless you're planning on multiple instances of gui_class_vehicleinfo_t?
The window is very similar to the convoy details window, and here the original author has decided to have these two classes. I'm not sure I feel confident enough to merge them without loosing anything on the way. Neither can I really figure out how to define the comboboxes in vehicle_class_manager_t and then put them on the scrolly with the gui_class_vehicleinfo_t. So the easiest option seems to be to have two action_listener_t classes, I guess?


Offline TurfIt

Re: create an array of comboboxes
« Reply #18 on: August 03, 2017, 12:15:03 AM »
I see, I will see if I can sort this out by comparing with other code.
In the convoi detail window you mention, cnv->call_convoi_tool( 'x', NULL ); can be seen. I expect you'll need a v->call_vehicle_tool(...), an of course the implementation of the tool itself in simvehicle, simmenu, and simtool files.


Other windows, like depot window and many others, have their buttons initiated first, and then in draw, the position etc is specified. Is this a bad way? Currently it seems to work fine in my window.
It's a performance thing - many open windows can drag things down. Minimize the work in ::draw - it should only do the actual drawing, not settings up components that aren't changing every frame. The depot window doesn't do this in draw that I see, all in layout.
Not sure if you noticed, but opening one of the comboboxes has a rather interesting bouncing effect when first opened.... due to this changing positions every frame.


I think I understand what you mean: I should create the list of names on beforehand and then let all comboboxes share the same list?
Since the strings are the same, yes.


The window is very similar to the convoy details window, and here the original author has decided to have these two classes. I'm not sure I feel confident enough to merge them without loosing anything on the way. Neither can I really figure out how to define the comboboxes in vehicle_class_manager_t and then put them on the scrolly with the gui_class_vehicleinfo_t. So the easiest option seems to be to have two action_listener_t classes, I guess?
A scrolly needs a container put into it. gui_class_vehicleinfo_t is that container instanced as veh_info in vehicle_class_manager_t.  Methods in vehicle_class_manager_t can put components into itself (it's also a container) with add_component(foo), or into gui_class_vehicleinfo_t with veh_info.add_componenet(foo).
The same applies for action_listeners. Instead of .add_listener(this)  you can add_listener(&veh_info).  1 or 2 action_listeners depends on what those listeners actually do. If all the information required to perform the actions of components in gui_class_vehicleinfo_t is also in gui_class_vehicleinfo_t, then it might as well do it. But if performing the action requires info in vehicle_class_manager_t, it might be easier to just let the higher class do the work instead of passing the required info down.

Online jamespetts

  • Simitrans-Extended project coordinator
  • Devotee
  • *
  • Posts: 16179
  • Total likes: 454
  • Helpful: 178
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: create an array of comboboxes
« Reply #19 on: August 03, 2017, 10:11:34 PM »
TurfIt - thank you very much for helping Ves with this: it is very much appreciated. It makes such a difference to have a second person working on the code for Extended, and it is of great value that he has been able to receive so much assistance.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Online Ves

Re: create an array of comboboxes
« Reply #20 on: August 03, 2017, 10:33:12 PM »
In the convoi detail window you mention, cnv->call_convoi_tool( 'x', NULL ); can be seen. I expect you'll need a v->call_vehicle_tool(...), an of course the implementation of the tool itself in simvehicle, simmenu, and simtool files.
Ok thanks

Quote
It's a performance thing - many open windows can drag things down. Minimize the work in ::draw - it should only do the actual drawing, not settings up components that aren't changing every frame. The depot window doesn't do this in draw that I see, all in layout.
Not sure if you noticed, but opening one of the comboboxes has a rather interesting bouncing effect when first opened.... due to this changing positions every frame.
Aha, I didnt realize that difference! In my window I want the number of comboboxes to match the correct amount of classes currently existing in the convoy, so adding a car might add another class (I know there is a bug now that makes all classes show up, but thats not supposed to). So, I need the comboboxes to show up and disappear as well as shift positions automatically.
Should I then maybe create a ::layout() like in the gui_convoy_assembler?

Quote
Since the strings are the same, yes.
Ok, I think I made something that is good:
Code: [Select]
char *class_name;
char *pass_class_name_untranslated[32];
for (int i = 0; i < pass_classes; i++)
{
class_name = new char[32];
sprintf(class_name, "p_class[%u]", i);
pass_class_name_untranslated[i]= class_name;
}
... and then I use the pass_class_name_untranslated to show the name in the combobox. At least it seems to work fine.

Quote
A scrolly needs a container put into it. gui_class_vehicleinfo_t is that container instanced as veh_info in vehicle_class_manager_t.  Methods in vehicle_class_manager_t can put components into itself (it's also a container) with add_component(foo), or into gui_class_vehicleinfo_t with veh_info.add_componenet(foo).
The same applies for action_listeners. Instead of .add_listener(this)  you can add_listener(&veh_info).  1 or 2 action_listeners depends on what those listeners actually do. If all the information required to perform the actions of components in gui_class_vehicleinfo_t is also in gui_class_vehicleinfo_t, then it might as well do it. But if performing the action requires info in vehicle_class_manager_t, it might be easier to just let the higher class do the work instead of passing the required info down.

I have now remade the comboboxes like this (and moved the positions etc to this location too):
Code: [Select]
for (int i = 0; i < pass_classes; i++)
{
gui_combobox_t *class_selector = new gui_combobox_t();
class_selector->set_highlight_color(1);
class_selector->set_pos(scr_coord(get_windowsize().w - 200, offset_y));
class_selector->set_size(scr_size(190, D_BUTTON_HEIGHT));
class_selector->set_max_size(scr_size(D_BUTTON_WIDTH - 8, LINESPACE * 3 + 2 + 16));
class_selector->set_visible(true);

for (int j = 0; j < pass_classes; j++)
{
class_selector->append_element(new gui_scrolled_list_t::const_text_scrollitem_t(translator::translate(pass_class_name_untranslated[j]), SYSCOL_TEXT));
}
add_component(class_selector);
class_selector->add_listener(this);
class_selector->set_focusable(false);
offset_y += LINESPACE;
}
... however this messes up with the action creator part. Can I make an array, just like with the pass_class_name_untranslated, with comboboxes and access in similar way?
Thanks for the explanations on how to make the comboboxes for the lower part of the window!

TurfIt - thank you very much for helping Ves with this: it is very much appreciated. It makes such a difference to have a second person working on the code for Extended, and it is of great value that he has been able to receive so much assistance.
YES INDEED!  ;D