Building convoys in depot shows/hides vehicles randomly

[wlindley]

With the current latest (commit 7c09fc9b36ff883de3e29bad222c8154a20fcbfb):

• Click on a depot.  Have "Show All" turned off.
• Build a locomotive, or a Boat horse, etc.
• Only a few of the possible vehicles will be displayed, or perhaps none at all.  Sometimes, instead, a large range of vehicles including red-underlined ones that should not be displayed without "Show All" will actually be displayed anyway.
• Turn "Show All" on and all the vehicles (including impermissible ones with the red underline) are displayed.
• Turn "Show All" off again and a different set (perhaps the correct ones, perhaps not) of vehicles is displayed.

Moving between Insert / Append and back again also displays multiple seemingly-random sets of vehicles each time.

At least "Show All" seems to function properly.

[Ranran(retired)]

This behavior is odd, but the problem seems to be only the behavior after the first action is wrong. For example, if you click Show All twice, or if you connect two locomotives, the display will be correct.

EDIT:
I think this is because the screen does not refresh properly after the action. Certain operations refresh the screen to the state of the previous action. The depot dialog consists of two parts, depot_frame and gui_convoy_assembler, and I think there was a problem with mutual communication.
But I don't think there have been any recent changes to the depot dialog code that would cause this behavior. In other words, I think that changes related to interaction indirectly caused this symptom.

EDIT2:
I think that update_tabs () is now executed before the action when selecting a vehicle when clicking on a vehicle. 

[Ranran(retired)]

Anyway, the problem seems to be that update_tabs() and build_vehicle_lists() don't do the right thing on the first click.
As already mentioned, no recent changes have been made to the depot dialog itself that affect this.
This is definitely being called. And strangely, it's called three times with one click. But it doesn't do the right thing three times. We need the following clicks.

These three seemingly nonsensical duplicate attempts may be related to this bug I previously reported.
https://forum.simutrans.com/index.php/topic,21214.0.html
The component at the top of the depot dialog may have two ghosts.

EDIT:
I wonder if build_vehicle_lists() and update_tabs() are executed before the vehicles are updated.

However, I suppose that this Frankenstein's monster will be difficult to repair and it may be faster to create a new Frankenstein's monster. Anyway I have given up on further modifying the old Frankenstein's monster. (´・ω・｀)

[Matthew]

Steps to reproduce (Refunds)

1. Load the demo game from pak128.Britain-Ex.
2. Open the depot at (490,8).
3. Open the Locomotives tab and click on the SR Bulleid/Raworth CC.
4. Refund the locomotive.

Expected results (Refunds)

You have an empty or new convoy and can Append any vehicle to it.

Actual results (Refunds)

The Locomotive tab is empty. All vehicles in other tabs have a red status indicator because none of them can be Appended to the convoy.

------------------

Steps to reproduce (New convoy)

1. Load the demo game from pak128.Britain-Ex.
2. Open the depot at (490,8).
3. Open the Locomotives tab and click on the SR Bulleid/Raworth CC.
(Optional: Switch to the Passenger carriages tab. Note the lack of suitable passenger carriages.)
4. Open the Convoy List at the top and select New convoy.

Expected results (New convoy)

You have an empty new convoy and can Append any vehicle to it.

Actual results (New convoy)

The Locomotive tab is empty. All vehicles in other tabs have a red status indicator because none of them can be Appended to the convoy.

-----------------------------------------------

Steps to reproduce (Carriages)

1. Load the demo game from pak128.Britain-Ex.
2. Open the depot at (490,8).
3. Open the Locomotives tab and click on the SR Bulleid/Raworth CC.
4. Switch to the Passenger carriages tab. There are no suitable carriages for this convoy.

Expected results (Carriages)

Passenger carriages suitable for this convoy can be Appended.

Actual results (Carriages)

All vehicles in the Passenger Carriages tab have a red status indicator because none of them can be Appended to the convoy.


Workarounds
1. Close the depot window and open it again.
2. Switch to Insert mode and Insert a front-of-convoy vehicle or a first Passenger Carriage, then switch back to Append mode and Append the remaining vehicles.
3. Click the Show all option on and then off again.

All of these are confusing for new players.

Comments
It seems very likely that this is a single bug that can be reproduced in three ways, hence one post.

[Ranran(retired)]

Before reporting a bug in a new thread, it's a good idea to check the bug report board for reported the similar bug recently. It will be easy to find.
https://forum.simutrans.com/index.php/topic,21637.0.html

[Ranran(retired)]

I suppose pull request #554 will fix this. Please check it.

EDIT:
Unfortunately it couldn't be fixed properly. After all, we may need to rewrite most of the depot dialog code...
But that may be necessary for ver 15.

[jamespetts]

I suppose pull request #554 will fix this. Please check it.

EDIT:
Unfortunately it couldn't be fixed properly. After all, we may need to rewrite most of the depot dialog code...
But that may be necessary for ver 15.

Thank you for your work on this. I would suggest that a rewrite of the depot GUI is probably best saved for ex-15, since there are a great many depot related changes to be introduced there. I am hoping to set up the basic depot related changes, at least the data structures and some of the logic, soon to allow the UI to be developed while I work on the rest of the logic. I have already made a start, as you will have seen in the other thread.

[Matthew]

Before reporting a bug in a new thread, it's a good idea to check the bug report board for reported the similar bug recently. It will be easy to find.
https://forum.simutrans.com/index.php/topic,21637.0.html

You are right, apologies.

[Ranran(retired)]

The cause of this bug seems to be the update lag of the convoy (vehicle) data that depot has. But I'm having a hard time understanding why it's occurring and how it came to occur.
This appears to have been caused by a merge of standards, but the depot code has not been changed, nor does this occur with the current depot code prior to the merging standard code.


For example, press the refund button on the depot dialog to dismantle the convoy.
That convoy no longer exists. So depot->get_convoi(icnv) should be null. Then gui_convoy_assembler_t::vehicles should be null and also convoi_pics should be null.
But convoy still seems to exist and based on that the depot dialog will try to update with the latest information. get_convoy gets the ghost convoy from the depot that should have been emptied. This difference will not give correct results.
But if you take the action to update the dialog after a short time, the dialog will be updated correctly as it will be correctly recognized that the convoy does not exist.

The code to dismantle convoy from the depot was exactly the same as standard.

I don't know how this difference was made. Overhaul of depot seems to solve this, but for the above reasons it seems that updating dialog has to repeat a nonsensical loop. I would appreciate any advice from those who know more about this.

[jamespetts]

You are right, apologies.

I have merged this with the other topic now.