Moderator: OpenTTD Developers
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.
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.
Also, other OpenTTD developers may have different opinions.
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.
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.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?
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).
Users browsing this forum: Majestic-12 [Bot] and 5 guests