Recommended way to make a GUIList that respects constructors/destructors?

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

Post Reply
btzy1996
Engineer
Engineer
Posts: 18
Joined: 31 Aug 2012 05:09

Recommended way to make a GUIList that respects constructors/destructors?

Post by btzy1996 » 08 Jan 2019 05:59

I'm attempting to add the ability to make vehicle lists have an option to group by shared orders (viewtopic.php?f=32&t=84587), but I've run into an issue there.

In the current master branch, the list of vehicles is backed by a GUIList<const Vehicle*>, and GUIList<T> inherits from SmallVector<T, 32>. In order to implement the ability to group by shared orders, each element in the GUIList can on longer be a single vehicle. Instead, each element should be a list of vehicles (and this list can be arbitrarily long, depending on the number of vehicles sharing the same orders).

Thus comes the problem: SmallVector (unlike std::vector) does not call constructors or destructors of its elements. This means that if I have a GUIList<std::vector<const Vehicle*>> (or any other type that does dynamic memory allocation), it would leak memory because the destruction of GUIList<std::vector<const Vehicle*>> does not call the destructor for each std::vector<const Vehicle*>.

Modifying SmallVector to call constructors and destructors seems to be a bad idea because it is used practically everywhere in the code (so other stuff might break). Also, it might become less performant.

There's also the possibility of storing the actual std::vector<const Vehicle*> somewhere else, and making the GUIList keep just a pointer, i.e. GUIList<std::vector<const Vehicle*>*>. But needing to store the actual data somewhere else seems needlessly complicated to me.

Any suggestions?

Alberth
OpenTTD Developer
OpenTTD Developer
Posts: 4730
Joined: 09 Sep 2007 05:03
Location: home

Re: Recommended way to make a GUIList that respects constructors/destructors?

Post by Alberth » 09 Jan 2019 20:40

I don't know what you're aiming for exactly, but if you want consistency between machines in a MP game, or process changes in the lists coming from your MP coop partner, GUI code is not the place to store this information. GUI code only paints nicely coloured rectangles and pretty images onto the screen in something we perceive as "windows", and nothing else.

If you do want to store data in the GUIcode, there is no rule that only smallvector is allowed. It's just that smallvector was available first (at the time the program was still C and not C++). Also, smallvector had emplace_back years before std::vector had it. For these two reasons you find smallvector all over the place. Since the code base is now C++, if GUIList doesn't rock your boat, pick something better, like std::vector. Performance is likely a non-issue unless you rebuild the lists every x milli-seconds (as I said, GUI code typically shouldn't do much, it should just display data stored and maintained elsewhere to the user).

As a side-note, obviously, currently there is already a way to get all vehicles with the same shared orders, your lists of vehicles thus already exist. I don't know why you want to duplicate such information in the GUI, but ymmv.
Being a OpenTTD developer does not mean I know what I am doing.
Also, other OpenTTD developers may have different opinions.

btzy1996
Engineer
Engineer
Posts: 18
Joined: 31 Aug 2012 05:09

Re: Recommended way to make a GUIList that respects constructors/destructors?

Post by btzy1996 » 12 Jan 2019 12:50

Sorry for the slow reply, I forgot to check the "notify me" option for this post. Thanks for the reply!

I don't just want to display all the vehicles with the same shared orders; I want all the vehicles with the same shared orders from a given subgroup of vehicles. I've found a better way, by making the GUIList store a pair of begin/end pointers to a list that is stored somewhere else, and I made a PR based on that.

Eddi
Tycoon
Tycoon
Posts: 7410
Joined: 17 Jan 2007 00:14

Re: Recommended way to make a GUIList that respects constructors/destructors?

Post by Eddi » 13 Jan 2019 01:44

maybe i'm understanding this wrong, but why don't you pick one vehicle out of each shared vehicle list as a representant of that list, and make a GUIList out of these representants?
You might not exactly be interested in Ferion, but if you are, have fun :)

btzy1996
Engineer
Engineer
Posts: 18
Joined: 31 Aug 2012 05:09

Re: Recommended way to make a GUIList that respects constructors/destructors?

Post by btzy1996 » 13 Jan 2019 06:54

maybe i'm understanding this wrong, but why don't you pick one vehicle out of each shared vehicle list as a representant of that list, and make a GUIList out of these representants?
For each item in the GUIList, I need to iterate through all vehicles within that subgroup and draw every vehicle onto the screen, so I still have to have all members of the subgroup readily accessible. So every vehicle (not just the representants) still has to be stored somewhere.

Unless you're saying to keep the vehicle list somewhere else, but have the GUIList just store one representative from that list. That works, it isn't much easier to write than my solution, and it has an inferior time complexity when iterating through the vehicles (for the purpose of drawing to the screen).

Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 3 guests