New refit_cost callback for vehicles

[Wahazar]

Is there any possibility to force non-zero refit costs within the same cargo type?
For example, to pay for refit from 2nd to 1st class carriage, or to pay for refit to longer trailers.

[PikkaBird]

Is there any possibility to force non-zero refit costs within the same cargo type?

Apparently there is. Did you read the thread to which you are replying? Did you try it?

[Wahazar]

Did you read the thread to which you are replying?

Of course did, albeit to fast to notice Snail post...

[Supercheese]

Is there any possibility to force non-zero refit costs within the same cargo type?
... pay for refit to longer trailers.

From Fake Subways source:

Code: Select all

#define ADTRANZ_TUBE_REFIT_COST_FACTOR		10

...

// Refit cost switch

switch(FEAT_ROADVEHS, SELF, adtranz_refit_cost_switch, var[0x10,8,0xFF] - cargo_subtype) {
    /* Refit cost based on difference between target cargo sybtype and current cargo subtype.
	   Refitting to fewer carriages refunds money, while adding carriages costs money.        */
	-6:				-(6*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	-5:				-(5*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	-4:				-(4*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	-3:				-(3*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	-2:				-(2*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	-1:				-(1*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	0:				0;
	1:				(1*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	2:				(2*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	3:				(3*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	4:				(4*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	5:				(5*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	6:				(6*ADTRANZ_TUBE_REFIT_COST_FACTOR);
	return CB_FAILED;
}

[Wahazar]

I want to use cargo classes to determine refit cost:

Code: Select all

switch (FEAT_ROADVEHS, SELF, switch_LCV_refit_from_box, extra_callback_info1 & 0xFFFF0000) {
   bitmask(CC_EXPRESS) : return 0 | CB_RESULT_AUTOREFIT;
   return CB_FAILED; }
switch (FEAT_ROADVEHS, SELF, switch_LCV_refit_from_mailvan, extra_callback_info1 & 0xFFFF0000) {
   bitmask(CC_MAIL) : return 0 | CB_RESULT_AUTOREFIT;
   return CB_FAILED; }
switch (FEAT_ROADVEHS, SELF, switch_LCV_refit_from_armoured, extra_callback_info1 & 0xFFFF0000) {
   bitmask(CC_ARMOURED) : return 0 | CB_RESULT_AUTOREFIT;
   return CB_FAILED; }

switch (FEAT_ROADVEHS, SELF, switch_LCV_refit_cost, cargo_classes ) {
   bitmask(CC_MAIL) :  switch_LCV_refit_from_mailvan;
   bitmask(CC_ARMOURED) :  switch_LCV_refit_from_armoured;
   switch_LCV_refit_from_box; }

but it doesn't work - if I have two different cargos within same class (for example diamonds and gold, or goods and ENSP),
there is still payment required.

[planetmaker]

I want to use cargo classes to determine refit cost:

Code: Select all

switch (FEAT_ROADVEHS, SELF, switch_LCV_refit_from_box, extra_callback_info1 & 0xFFFF0000) {
   bitmask(CC_EXPRESS) : return 0 | CB_RESULT_AUTOREFIT;
   return CB_FAILED; }
switch (FEAT_ROADVEHS, SELF, switch_LCV_refit_from_mailvan, extra_callback_info1 & 0xFFFF0000) {
   bitmask(CC_MAIL) : return 0 | CB_RESULT_AUTOREFIT;
   return CB_FAILED; }
switch (FEAT_ROADVEHS, SELF, switch_LCV_refit_from_armoured, extra_callback_info1 & 0xFFFF0000) {
   bitmask(CC_ARMOURED) : return 0 | CB_RESULT_AUTOREFIT;
   return CB_FAILED; }

switch (FEAT_ROADVEHS, SELF, switch_LCV_refit_cost, cargo_classes ) {
   bitmask(CC_MAIL) :  switch_LCV_refit_from_mailvan;
   bitmask(CC_ARMOURED) :  switch_LCV_refit_from_armoured;
   switch_LCV_refit_from_box; }

but it doesn't work - if I have two different cargos within same class (for example diamonds and gold, or goods and ENSP),
there is still payment required.

The cargo classes variable works as bitmask, thus comparing for a single value does not work that easily:

Assume we have a cargo which has classes CC_ARMOURED, CC_EXPRESS, CC_PIECE_GOODS, thus it has the the cargo_class value of bitmask(CC_ARMOURED, CC_EXPRESS, CC_PIECE_GOODS) = 1 << CC_ARMOURED | 1 << CC_EXPRESS | 1 << CC_PIECE_GOODS = 00101100b = 0x0000002C

You mask that now with 0xFFFF0000 which always will result in 0. Thus you miss some bit shifting there.

With that shift applied the code above will fail always except in corner cases, where a cargo has only the matching cargo class: In the form above with correct bit shifting it cannot detect the single cargo classes but will always yield all, thus the check for the single classes must fail, too. Taking the values from our example above it would evaluate it like

Code: Select all

switch 0x00101100b {
  000000001b: never true
  000000010b: never true
  000000100b: never true
  ...
}

So what you can do is check for the presence of individual cargo classes in a sequence of switches:

Code: Select all

switch (FEAT_ROADVEHS, SELF, switch_check_mail_refit, (((extra_callback_info1 & 0xFFFF0000) >> 16) & bitmask(CC_MAIL)) > 0) {
  1: return 0 | CB_RESULT_AUTOREFIT;
  0: switch_check_next_cargo_class;
}

EDIT: or simpler use the hasbit(bitpos) built-in function:

Code: Select all

switch (FEAT_ROADVEHS, SELF, switch_check_mail_refit, hasbit( (extra_callback_info1 & 0xFFFF0000) >> 16, CC_MAIL) {
  1: return 0 | CB_RESULT_AUTOREFIT;
  0: switch_check_next_cargo_class;
}

[Wahazar]

What of shameful mistake, I just forgot to shift those damn upper bits.
Thank you for your comprehensive explanations.

[Wahazar]

There is one flaw, which was already mentioned by Emperor Jake - if station refit is limited to a given cargos, or cargo classes,
it still allow you to refit to another cargos, but silently refuses to refit - it is dangerous, because if wait for full load order is given,vehicle would stuck.
Probably an error message should be displayed if station refit is not possible.
Another, more elegant but complicated method is to allow change of reffitable cargos/classes of the given vehicles
(as far as I understand openttd code, refitable cargos are defined for engine.h, but later copied to vehicle.cpp struct?)
Such approach would let to use cargo subtype switch to change allowed cargo classes).

What I want to achieve, for example, to have road set with a bare engine/chassis which can be equipped with any truck body: box, hopper, tank, flatbed etc, via workshop refit. It would help to keep limited number of trucks and decrease headache during autoreplace choice.

[Eddi]

if anything, that increases autoreplace headaches. the idea may sound nice on paper, but i think coding it is not worth it. there is no actual reason to keep number of vehicles down.

[Wahazar]

Currently station refit window give opportunities to refit to all allowed cargo types, even if some are suppressed via appropriate switch.
Is is misleading and wrong fixed station refit, together with full load command, can cause a massive gridlock.
Is it possible to make list of only those cargoes which are allowed to autorefit?
(where is piece of code responsible for this - order_gui.cpp ?)
Such list can be displayed with same method as refit list for vehicle inside depot (excluding those with blocked autorefit).

Of course, if there is scheduled depot refit before station refit, vehicle schedule is unaware about it, and such filtered results are irrelevant.
In such (rather sophisticated) situation, player should refit vehicle manually to proceed with scheduling appropriate station refit.
Still it is much better than displaying all (including these inaccessible) options.

Here is example:


If semitrailer lorry is refitted to coal, it can carry also scrap metal or other bulk, but cannot carry metal - flatbed trailer is needed.
If refitted to metal, can carry metal, wood etc, but cannot carry coal, ore, scrap metal etc.
The problem is, that station menu offer all possibilities and show false 0 cost - with station locked without warning as an result.

[planetmaker]

Is it possible to make list of only those cargoes which are allowed to autorefit?

No, that's principally not possible; it's the cost of the flexibility you have as NewGRF author: OpenTTD has no way to check what your NewGRF will allow at the time a vehicle is ordered to refit. Your NewGRF could decide to allow autorefit on Monday through Wednesday on odd years but Thursday to Saturday in even years, but vice versa when the year is > 2050. And never when it's a public holiday. Except when the leading engine is bought earlier than 1995 when only refit is allowed Sunday (as it's then a museum railway)

[Wahazar]

Of course, I know, that autorefit list can be dynamically changed within shedule, I don't expect that openttd will cope with it.
I was asking, if it is possible to run, for the station refit list, similar procedure as for the depot refit list, relying on actual vehicle state.
These green zeroes are very ambiguous.

[andythenorth]

Is it possible to make list of only those cargoes which are allowed to autorefit?

No. Presenting a filtered list to the player is misleading, because it may not be the same list used for any given refit cycle when the vehicle actually visits the station.

There are two problems.
(1) Possible to set invalid orders, which look valid.
(2) The refit may be non-deterministic.

To explain a little more.
- it is easy to imagine case where newgrf author does weird things like restrict refit by date, etc. We don't need to worry about the silly cases, the sensible cases are already broken.

Invalid orders
- it is easy for newgrf author to do something they think is sensible, such as limiting by class
- a chain of refits (in different orders) might cross class boundaries in one direction only, into a dead end. e.g. A can refit B, B can refit C, but C can't refit A.
- an order may be valid when initially set, but changes to other orders leave this order broken.

I discovered these broken orders with real code in FISH, which initially had class-based refit limitations. The discovery of it caused a total redesign of how FISH ships worked, resulting in Squid.

In theory, following any order change, the orders UI could scan all orders for consistency, and popup some warning. In theory. In practice it would be adding more complexity to an orders UI that is already stretched. It would need error highlighting (red) adding for the broken orders, and it would require a disruptive modal warning. It also would do nothing for the second case.

Non-deterministic refits
- future refits are totally non-deterministic from the point of view of the orders UI. (For pedantry, they follow strictly deterministic rules in the code, but within a simulation which is non-deterministic).
- result of 'refit any available' is unknowable in advance
- players can use advanced orders for non-deterministic routing
- player can skip orders, or change orders at any time

Summary
Can't be fixed in the UI. The current situation is not good, because if newgrf authors restrict refits, players can create invalid orders and receive no warning about it. But trying to solve that in OpenTTD is worse, because it's work, and the situation is fundamentally unfixable.

Conclusion
Newgrf authors should not restrict autorefit in any way at all. Autorefit should always be allowed to every cargo in the set of refittable cargos for a vehicle.

I'm going to get on a soapbox (which I might fall off at any point), and say that newgrf authors in this case are responsible for not breaking the game for players by introducing possibility of invalid orders.

I thought I had added a friendly note to newgrf wiki advising not to use any refit restrictions, but seems I misremembered.

Comment
So why was this cb option added at all?
- was asked for by people who sounded like they had a good case for it
- not all pitfalls are knowable in advance

[Wahazar]

Presenting a filtered list to the player is misleading, because it may not be the same list used for any given refit cycle when the vehicle actually visits the station.

I know - but only if player is changing refit in depot. It is rather unlikely situation if there are refit costs involved, and lesser of two evils than offering impossible station refits.

OK, lets skip idea with station refit filter.
But why there are "0 refit costs" displayed for all cargoes?
Again, I don't expect any wise prophecy determining refit costs for the given shedule,
only just same procedure as for depot refit.
If you are scheduling depot refit, refit costs are given for actual refit state (even if these are about to changed later),
but same schedule for station gives zero values:

It is somewhat ambiguous.

Newgrf authors should not restrict autorefit in any way at all. Autorefit should always be allowed to every cargo in the set of refittable cargos for a vehicle.

I would rather disable autorefit totally. Switching car body at the station doesn't look good for me.

[frosch]

Hmm, the value displayed is almost a random value. It only displays something while the vehicle is inside a depot while editing the orders, isn't it?

In other word: I believe the display in the order refit GUI is completely broken

[Wahazar]

Hmm, the value displayed is almost a random value. It only displays something while the vehicle is inside a depot while editing the orders, isn't it?

Exactly - thanks for pointing it, usually I'm changing orders before letting engine go.
Anyway, both refit windows (station and depot) should display same results (when engine is stopped inside depot).

[Eddi]

i don't think refit at station can ever show the correct values, except when every refit is allowed anyway.

in the very least, refit may depend on what cargo the vehicle arrived with, and whether that cargo was actually unloaded.

and if you're using full load with something as volatile as refit at station, you're just asking for trouble.


what may be useful is a news popup saying "refit failed"