FS#6155 - Vehicles are sometimes incorrectly shown as refittable

Attached to Project: OpenTTD
Opened by Daniel Plaumann (dandan) - Monday, 27 October 2014, 21:00 GMT
Type Bug
Category NewGRF
Status New
Assigned To No-one
Operating System All
Severity Medium
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No


Under certain circumstances, the purchase menu and refit GUI show vehicles to be 'refittable' even when they are not (see screenshot).

This happens when the vehicle is refittable to a single cargo only but the cargo subtype display callback (CB 19) is enabled via veh. prop. 1E. It seems that this triggers the 'refittable' string and enables the refit GUI without any further test whether a cargo subtype string is actually returned.

This problem becomes apparent in the SBB set, which has a GRF parameter that enables more refit options. When the parameter is switched off, some vehicles behave as described.

A minimal test grf is attached.
This task depends upon

Comment by Peter Nelson (peter1138) - Tuesday, 21 March 2017, 21:58 GMT
While it is possible to execute the callback, this can result in the opposite problem: an engine not being flagged as refittable when actually it is.
Comment by frosch (frosch) - Wednesday, 22 March 2017, 21:16 GMT
Should "cb == CALLBACK_FAILED" fall through to the default_cargo condition?

I guess we can also add a Vehicle* parameter to IsEngineRefittable, which would be allowed to be NULL. I think there are other functions with similar behaviour.
Comment by Peter Nelson (peter1138) - Thursday, 23 March 2017, 07:02 GMT
Hmm, yeah, probably should fall through. Also as per the real callback, grf version should be checked.
Comment by Peter Nelson (peter1138) - Thursday, 23 March 2017, 08:05 GMT
This version reuses the existing function (although adding a Vehicle * parameter to something in engine.cpp feels wrong!), adds the fall-through, and also checks grf version.

Side affect is some vehicles (e.g. in DBSetXL) which were listed as refittable in the build engine list are no longer marked, but once built and connected in a depot should be fine.