Topic: [fix] depot_frame

[gerw]

build_vehicle_list was called twice, while opening a depot window. First time it is called without timeline. Therefore the electric tab was shown (in an electrified depot), even if you couldn't build any electric engine.

I love fixing bugs while deleting lines 

[prissi]

This is done deliberately, since later opening this no electric engines will be shown or if the depot is open for longer time.

[gerw]

But the depot_frame isn't updated, if there is a new vehicle available, is it?

Edit:
If this is the desired behaviour, I've found an other bug: If you open a depot_frame and electrify the depot afterwards, there isn't an electric tab...

[prissi]

This is indeed a problem, because if you send a vehicle to a depot that does not has this category, it can crash the depot. Furthermore, if a certain kind of vehicle is obsolete, the tab is empty. But when you activate "show all" then it will be shown.

The electricity look therefore like a bug.

[gerw]

Maybe a solution is to make the tabs dynamic?

[gerw]

With this patch, the tabs will be created (and destroyed) dynamicly (also if a catenary is built or removed).

[prissi]

Still not 100% correct. Build a tram depot in pak 64 in 2030. Open Depot. Seletc show obsolete. ONly one car is there. Close window. Open again => All cars are there, not only the obsolete one (like show all). Not sure if this is related to your patch or was just hidden before.

[gerw]

Still not 100% correct. Build a tram depot in pak 64 in 2030. Open Depot. Seletc show obsolete. ONly one car is there. Close window. Open again => All cars are there, not only the obsolete one (like show all). Not sure if this is related to your patch or was just hidden before.

I can't reproduce this. If I open the depot first time, 'show all' is selected, 'show obsolete' isn't. If I reopen the depot, it shows always the same selection as before closing.

Edit: There is some other issue with the static show_retired_vehicles/show_all: If you have more then one depot_gui, then changing the state in one window, changes also the state in the others (it's static), but doesn't updates there vehicle lists. Edit3: Now, all depot_frames get refreshed. So this is solved.

Edit2: New patch version. Now, the depot frame is refreshed each new month (new vehicles?).

[prissi]

This with the static was requested for saving last state.

[gerw]

Any annotations or bugs about this patch?

[prissi]

No time to compile simutrans since sunday ...

[prissi]

I think I worked on some of this. But could you update you patch?

[gerw]

You can find the patch at
www.tu-chemnitz.de/~gerw/patches/depot_fix_v2.patch

[gerw]

New version www.tu-chemnitz.de/~gerw/patches/depot_fix_v3.patch

(Minor change + little red bolt is shown, if depot is electrified).

[prissi]

I get a memory corruption as soon as a convoi enters the depot. However, it seems this happend already earlier within the code, it writing in a readonly place aparently occurs. I think this patch needs to be handled with valgrind before it can become productive.

[gerw]

Thank you for reporting. I've found the bug. It's caused by an ugly data structure in depot_frame:
vehicle_map has pointers to the entries of the vectors electrics_vec, pas_vec,... But if they get resized, the pointers become invalid....

In the old code, the size of the vectors was set, before filling them. I deleted this code, cause the vectors will resize themselves.

Now I will look, how to change the vehicle_map thing, because the pointers to the vectors aren't very nice imho - and cause programming errors 

[gerw]

Ok, now I've reinserted the old code, albeit I don't like it, but I hadn't any good idea, how to handle this.

tu-chemnitz.de/~gerw/patches/depot_fix_v4.patch

[Dwachs]

The patch worked with Tortoise I updated the patch.

I have one question: What purpose does this block serve:

Code: [Select]

@@ -640,6 +589,11 @@
  loks_vec.clear();
  waggons_vec.clear();
 
+ pas_vec.resize(depot->get_vehicle_type()->get_count());
+ electrics_vec.resize(depot->get_vehicle_type()->get_count());
+ loks_vec.resize(depot->get_vehicle_type()->get_count());
+ waggons_vec.resize(depot->get_vehicle_type()->get_count());
+
  vehicle_map.clear();
 
  // we do not allow to built electric vehicle in a depot without electrification
The vectors are resized above this block anyway.

[gerw]

The patch worked with Tortoise I updated the patch.

Did you just update the patch or did you change something?

I have one question: What purpose does this block serve:
...
The vectors are resized above this block anyway.

Mmmh. Maybe this lines are superfluous and could be deleted.

[Dwachs]

I deleted only the lines I mentioned above.

[prissi]

IS this now the final version?

[gerw]

Since there was no feedback, I didn't alter the patch. The version posted by Dwachs should be the latest.

[prissi]

Incorporated when updating depot_frame for netowrk mode.