Patch: Improved Breakdowns v2.4 (r14563)

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

Tekky
Route Supervisor
Route Supervisor
Posts: 420
Joined: 19 Dec 2006 04:24

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by Tekky » 06 Nov 2009 14:01

I don't understand your last message, for the following reason:

As far as I know, there are currently three types of servicing:
  1. Automatic servicing, in which the train automatically tries to find a depot, as soon as the train's maintenance period has expired.
  2. "Service in depot" order. This order will be skipped if the train's maintenance period has already expired.
  3. "Go to depot" order. In contrast to the "service in depot" order, this order will ALWAYS be executed, even if the train's maintenance period has not yet expired.
If I understand the source code correctly, automatic train servicing will only occur for trains that do not have explicit "service in depot" or "go to depot" orders.

I never suggested that a train with permanent damage should be immediately issued a "go to nearest depot" order, as this would indeed severely mess up routes of trains which have a "service in/go to depot" order, as you correctly pointed out. Instead, I suggested that a train with permanent damage should act as if its maintenance period had expired, even if this is not the case. That way, trains would still use their regular depot for maintenance. The only difference would be that trains with an explicit "service in depot" order would ALWAYS service in that depot if they have suffered permanent damage, even if their maintenance period had not expired yet. I still think this solution would be ideal.

User avatar
bigos
Traffic Manager
Traffic Manager
Posts: 181
Joined: 13 Sep 2009 21:15
Location: Poland
Contact:

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by bigos » 06 Nov 2009 15:45

Are you sure about that? I just look in code... When maintenance period expire train get order - service in nearest depot. So doesn't meter if you give him order go to depot or change his maintenance time. But maybe I'm wrong. Would be nice if you write your code to change it and then we can test how it is. In my opinion making all automatic makes game a bit boring...

Tekky
Route Supervisor
Route Supervisor
Posts: 420
Joined: 19 Dec 2006 04:24

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by Tekky » 06 Nov 2009 19:05

bigos wrote:Are you sure about that? I just look in code... When maintenance period expire train get order - service in nearest depot. So doesn't meter if you give him order go to depot or change his maintenance time. But maybe I'm wrong.
Take a look at Vehicle::NeedsAutomaticServicing() in vehicle.cpp. This function will return false for vehicles that have a go to depot/service in depot order in their vehicle list.

Therefore, as far as I can tell, there is no automatic train servicing for trains which have explicit orders in which depot to do maintenance.
Last edited by Tekky on 06 Nov 2009 19:09, edited 1 time in total.

User avatar
bigos
Traffic Manager
Traffic Manager
Posts: 181
Joined: 13 Sep 2009 21:15
Location: Poland
Contact:

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by bigos » 06 Nov 2009 19:07

For me its more like to not repeat same order all the time if its in list of orders... that's why there is false.

Tekky
Route Supervisor
Route Supervisor
Posts: 420
Joined: 19 Dec 2006 04:24

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by Tekky » 06 Nov 2009 19:37

bigos wrote:For me its more like to not repeat same order all the time if its in list of orders... that's why there is false.
I just tested what happens when I deactivate the line which checks for depot orders by commenting it out:

Code: Select all

bool Vehicle::NeedsAutomaticServicing() const
{
	//if (_settings_game.order.gotodepot && VehicleHasDepotOrders(this)) return false;
	if (this->current_order.IsType(OT_LOADING))            return false;
	if (this->current_order.IsType(OT_GOTO_DEPOT) && this->current_order.GetDepotOrderType() != ODTFB_SERVICE) return false;
	return NeedsServicing();
}
Just as I suspected, when the line is commented out, trains will always perform automatic servicing in any depot they can find, even if they have a different depot in their list of orders. They will not do this when the line above is not commented out.

I assume that what you were referring to the third line of the function body in your previous post? I was referring to the first line.

User avatar
bigos
Traffic Manager
Traffic Manager
Posts: 181
Joined: 13 Sep 2009 21:15
Location: Poland
Contact:

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by bigos » 06 Nov 2009 20:24

I never used that option you are talking about. Because if I set depot for repairs in orders then train can go twice same rout to get repaired. I force trains to go to depot after each station. If you want to implement you idea about repair after mechanical failure then go ahead :)

Tekky
Route Supervisor
Route Supervisor
Posts: 420
Joined: 19 Dec 2006 04:24

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by Tekky » 06 Nov 2009 22:21

bigos wrote:I never used that option you are talking about. Because if I set depot for repairs in orders then train can go twice same rout to get repaired.
I don't understand the problem. The problem you describe, concerning trains servicing twice on the same route, should only happen when you use automatic train servicing, i.e. when you don't explicitly set "go to depot" or "service in depot" orders.

It never was my intention to recommend using automatic train servicing, where the train decides itself which depot to visit. As you pointed out, this can cause trains to get lost. Therefore, I think it is better not to use automatic train servicing. In my previous two messages I was merely trying to show that this statement you made was wrong:
bigos wrote: Are you sure about that? I just look in code... When maintenance period expire train get order - service in nearest depot. So doesn't meter if you give him order go to depot or change his maintenance time. But maybe I'm wrong.
In my last post, I showed that Vehicle::NeedsAutomaticServicing() always returns false if the current vehicle has a depot in its list of orders, which, as far as I can tell, shows that your quote above is wrong.

By the way, even with automatic train servicing, the train doesn't immediately receive the order to go to the nearest depot, when the maintenance period expires. Instead, the train starts searching for nearby depots whenever it makes a pathfinding decision. If it never is able to find a nearby depot, it will not service at all.

User avatar
bigos
Traffic Manager
Traffic Manager
Posts: 181
Joined: 13 Sep 2009 21:15
Location: Poland
Contact:

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by bigos » 11 Nov 2009 00:49

Merged with trunk (r18037) and set that additional speed reduction work only if you have improved breakdowns on.
Attachments
IB.patch
IB 2.4 with additional speed reduction for r18037
(58.85 KiB) Downloaded 119 times

User avatar
bigos
Traffic Manager
Traffic Manager
Posts: 181
Joined: 13 Sep 2009 21:15
Location: Poland
Contact:

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by bigos » 11 Nov 2009 14:40

Tekky I think i made patch that you were asking for. After mechanical failure train get permanent speed reduction until next visit in depot. Also it will act same as train with expired servicing internal (don't know how to call it :/ ). It means it will go to depot if he brake by mechanical failure.
Attachments
IB_with_service.patch
IB 2.4 + speed reduction + service after break
(59.49 KiB) Downloaded 124 times

Tekky
Route Supervisor
Route Supervisor
Posts: 420
Joined: 19 Dec 2006 04:24

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by Tekky » 12 Nov 2009 00:42

Thank you very much for your patch. However, I was unable to load newer trunk savegames with your patch. I was able to fix this by setting

Code: Select all

SLE_CONDVAR(Vehicle, breakdown_type,        SLE_UINT8,        103, SL_MAX_VERSION),
SLE_CONDVAR(Vehicle, breakdown_severity,    SLE_UINT8,        103, SL_MAX_VERSION),
to

Code: Select all

SLE_CONDVAR(Vehicle, breakdown_type,        SLE_UINT8,        129, SL_MAX_VERSION),
SLE_CONDVAR(Vehicle, breakdown_severity,    SLE_UINT8,        129, SL_MAX_VERSION),
in saveload/vehichle_sl.cpp, and by setting

Code: Select all

SDT_CONDBOOL(GameSettings, vehicle.improved_breakdowns,  103, SL_MAX_VERSION, 0, 0, false, (...)
to

Code: Select all

SDT_CONDBOOL(GameSettings, vehicle.improved_breakdowns,  129, SL_MAX_VERSION, 0, 0, false, (...)
in table/settings.h.

I assume that SAVEGAME_VERSION (defined in saveload/saveload.cpp) used to be 103 in the original patch. When SAVEGAME_VERSION was increased to 129, I guess these values should also have been updated, but weren't. However, I do not fully understand the OpenTTD saveload system, so I could also be wrong. All I can say is that it I can now load recent trunk savegames, after making these changes.

By the way, is there a reason why SAVEGAME_VERSION was bumped by 2, from 127 to 129? Isn't it normal to bump the savegame version only by one?

User avatar
bigos
Traffic Manager
Traffic Manager
Posts: 181
Joined: 13 Sep 2009 21:15
Location: Poland
Contact:

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by bigos » 12 Nov 2009 01:05

I did not change those lines from original patch :) The version in that lines is version when they were put there :) and all should work from that version to SL_MAX_VERSION. So you don't have to change them. Problem might be if things from trunk have higher version then your SL_MAX_VERSION.
I don't remember why its bumped by 2 - probably because i worked at few different patches and version took from other one. It doesn't meter how much is bumped until all lines in "saveload" or settings.h are below or equal SL_MAX_VERSION.

Tekky
Route Supervisor
Route Supervisor
Posts: 420
Joined: 19 Dec 2006 04:24

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by Tekky » 12 Nov 2009 01:30

bigos wrote:I did not change those lines from original patch :) The version in that lines is version when they were put there :) and all should work from that version to SL_MAX_VERSION. So you don't have to change them.
These lines were added by the "improved breakdowns" patch. Therefore, it is my understanding that the variables introduced by these lines should only be valid for savegames which have the same SAVEGAME_VERSION as the patch. This value used to be 103 in the original patch, but this has changed now to 129. Therefore, it seems obvious to me that the lines I quoted before should also be changed from 103 to 129. The fact that recent savegames from trunk are unable to load in your patch, but seem to work perfectly with my changes, confirms this.
bigos wrote:Problem might be if things from trunk have higher version then your SL_MAX_VERSION.
SL_MAX_VERSION is defined as 255. Therefore, this is not an issue, as SAVEGAME_VERSION does not exceed 255.

User avatar
bigos
Traffic Manager
Traffic Manager
Posts: 181
Joined: 13 Sep 2009 21:15
Location: Poland
Contact:

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by bigos » 12 Nov 2009 02:01

Its 3 am... i hardly can read what I'm writing... I had in mind SAVEGAME_VERSION not SL_MAX_VERSION.

Never tried to load save games from trunk. But yea you have right all additional stuff should have higher that number so you can load games from trunk but not from your game with trunk.

Tekky
Route Supervisor
Route Supervisor
Posts: 420
Joined: 19 Dec 2006 04:24

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by Tekky » 12 Nov 2009 11:05

bigos wrote:Problem might be if things from trunk have higher version then your [SAVEGAME_VERSION].
Yes, after there is a bump of the SAVEGAME_VERSION in trunk, it will no longer be possible to load the newest trunk savegames into your patch, until your patch is merged again with trunk.

By the way, I think it would be best if you would only bump SAVEGAME_VERSION by 1 instead of 2. Otherwise, it would be possible that no error message is produced, when attempting to load a trunk savegame whose version number is increased by 1, although the savegame format has changed. In my opinion, it is better to produce an error message than to risk causing bugs by trying to load an unknown future trunk savegame format.

Tekky
Route Supervisor
Route Supervisor
Posts: 420
Joined: 19 Dec 2006 04:24

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by Tekky » 13 Nov 2009 00:01

I just tested your patch and I like it. Just as I hoped for, trains now act as if they require maintenance after suffering permanent damage.

I have one question though:
bigos wrote:Because this patch, in my opinion, make game a bit easier I have modified it a bit. I have added effect of mechanical failure - maximum speed of train is permanently reduced to 50% (more locos in train - less speed reduction). Reduction can be removed by visiting depot.
When you wrote "this patch", were you referring to my "no reliability decrease for stopped trains" patch or were you referring to the improved breakdowns patch? If you were referring to my patch, then it seems that you forgot to include it in the last patch you posted.

I'm not asking you to include my patch if you do not want to, as I can easily apply my patch myself on top of yours. I just want to make sure that you didn't forget it.

User avatar
bigos
Traffic Manager
Traffic Manager
Posts: 181
Joined: 13 Sep 2009 21:15
Location: Poland
Contact:

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by bigos » 13 Nov 2009 00:05

It was about IB.

Tekky
Route Supervisor
Route Supervisor
Posts: 420
Joined: 19 Dec 2006 04:24

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by Tekky » 13 Nov 2009 00:14

Ah, ok.

By the way, it may be nice to have some kind of visual feedback so that you can see if a train has suffered permanent damage, such as black smoke, as is the case with planes.

User avatar
bigos
Traffic Manager
Traffic Manager
Posts: 181
Joined: 13 Sep 2009 21:15
Location: Poland
Contact:

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by bigos » 13 Nov 2009 00:29

Tekky wrote:Ah, ok.

By the way, it may be nice to have some kind of visual feedback so that you can see if a train has suffered permanent damage, such as black smoke, as is the case with planes.
There is. If train is permanently broken in info tab you can find warning - than train need service in depot because of speed reduction.

Black smoke is "static" animation... so it will not move with train and animate correctly. You need to figure out something else. But I'm not sure if its necessary.
Maybe some "news warning" if trains speed fell to like 20 km/h. But I don't know yet how to make it. Need to read code.

Tekky
Route Supervisor
Route Supervisor
Posts: 420
Joined: 19 Dec 2006 04:24

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by Tekky » 13 Nov 2009 00:38

bigos wrote: Black smoke is "static" animation... so it will not move with train and animate correctly.
The smoke that planes produce when they brake down looks quite nice to me, even if it does not move with the plane.

User avatar
bigos
Traffic Manager
Traffic Manager
Posts: 181
Joined: 13 Sep 2009 21:15
Location: Poland
Contact:

Re: Patch: Improved Breakdowns v2.4 (r14563)

Post by bigos » 13 Nov 2009 00:56

Ah that smoke. I was talking about that from train brekdown. I will check this maybe tomorrow.

Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 4 guests