Patch: Cargo backup/restore in depot during renewal

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

Post Reply
laurijh
Engineer
Engineer
Posts: 19
Joined: 21 Jul 2009 11:52
Location: Finland

Patch: Cargo backup/restore in depot during renewal

Post by laurijh »

Background: I hate losing cargo when renewing a vehicle. That's why whenever the game notifies me that a vehicle is getting old, I always wait for it to go to its destination station and unload its cargo to 0% and then send it to the depot for renewal. With passenger vehicles you have to be precise to press the button exactly at the right moment between loading and unloading. But this tends to get tiresome when there are a lot of vehicles around going old, so I wanted a way to (manually) renew a vehicle without having to care about its cargo getting lost.

Features:
  • It basically does the same thing as BackupVehicleOrders()/RestoreVehicleOrders() but with cargo.
  • When a vehicle is sold at a depot, its cargo is saved. If the user then creates a new compatible vehicle, the cargo is restored to it.
  • Works for all "inseparable" vehicles and vehicle sets, for example dual-headed trains, aircrafts and articulated vehicles.
  • Works between different vehicles, as long as they can use the same depot. For example, sell a dual-headed train engine which carries cargo, and restore that cargo to a new wagon.
  • Automatic refitting of the new vehicle for the backed up cargo if possible.
  • 2 new settings in the Vehicles tab, one for enabling the cargo backup/restore and the other for automatic refitting.
Limitations:
  • Only one backup in each depot per player (not per company) at a time.
  • Doesn't work for entire trains, only the first vehicle that is built will receive cargo from the backup (see 2nd question below for whether this should be changed).
  • The "Sell all" button doesn't back up any vehicle's cargo.
  • Backup state is not stored in save games.
Questions:
  • Should the automatic refitting cause the normal refitting cost to be charged? In the case of trains the refitting cost is sometimes even over half the value of the engine.
    • In my opinion no, because it's like the vehicle is built with the new cargo type from the start. Value of the vehicle when sold is the same regardless of having been refitted or not, so it makes sense that it would also cost the same to originally build regardless of cargo type.
  • If all the backed up cargo doesn't fit into the new vehicle, should the rest be left for restore to subsequent vehicles? You could sell some big old vehicle and restore its cargo to multiple smaller ones, or sell multiple vehicles and restore to multiple new ones. This would make the patch slightly more general-purpose and go a bit beyond the scope of just restoring cargo to renewed vehicles.
I think I've now fixed the issues pointed out by Rubidium. The biggest issue was solved by sending the backup state to new clients in a new type of network packet. Backups aren't restricted to one per company at a time either. Instead, there can be one in each depot for each network client. I've tested this with GRVTS and in a LAN game for a while.
rv3-old.png
rv3-old.png (6.59 KiB) Viewed 2820 times
rv3-new.png
rv3-new.png (6.63 KiB) Viewed 2804 times
As before, I would appreciate any further comments.
Attachments
cargobackup-0.4-r17128.patch
(29.13 KiB) Downloaded 60 times
Last edited by laurijh on 09 Aug 2009 12:14, edited 3 times in total.
Rubidium
OpenTTD Developer
OpenTTD Developer
Posts: 3815
Joined: 09 Feb 2006 19:15

Re: Patch: Cargo backup/restore in depot during renewal

Post by Rubidium »

Besides the normal 'it doesn't comply to the coding style', 'breaks savegame compatability' problems there is one really major problem. The major problem is that this system simply does not work in multiplayer games.

- new clients don't know about the cargo backup state, thus they get desynced.
- multiple players in multiple clients in the same company can be changing vehicles at the same time, making the behaviour 'random' for the player as to what cargo you get when you restore.

Besides that the code looks like it fails for articulated vehicles and on a conceptual level why backup cargo for only the front engine? After all, you might want to replace a wagon and keep the cargo. And if you want to replace/renew vehicles, what's wrong with autoreplace/autorenew?
laurijh
Engineer
Engineer
Posts: 19
Joined: 21 Jul 2009 11:52
Location: Finland

Re: Patch: Cargo backup/restore in depot during renewal

Post by laurijh »

Hmm, I did try to follow the coding style, looks like I missed the underscore in the global variable and checking for NULL explicitly...

I forgot to mention in the limitations that the backup state isn't stored in save games, just like with backed up orders, so there shouldn't be savegame compatibility problems.

You're right about the big multiplayer desync problem, I'll have to look into that. But the "only one backup per player" limitation should have read "only one backup per company" so I don't think the backups can get mixed up.

It shouldn't be difficult to make it work for wagons and secondary engines. I guess in some NewGRF's wagons have to be renewed too?

In my opinion, autoreplace/renew is too limited. It would be good if you could define "replace vehicles of type x with the newest available compatible vehicle type but only when it gets old", though I don't know how "compatible" would be determined. So, I prefer to do my renewals manually.

Anyway, thanks for pointing it all out. Looks like I have a lot more to learn :D
Rubidium
OpenTTD Developer
OpenTTD Developer
Posts: 3815
Joined: 09 Feb 2006 19:15

Re: Patch: Cargo backup/restore in depot during renewal

Post by Rubidium »

laurijh wrote:I'll have to look into that. But the "only one backup per player" limitation should have read "only one backup per company" so I don't think the backups can get mixed up.
Imagine you and me are in multiplayer in the same company. I'm in depot A, you in depot B.
- I delete train W (backs up cargo)
- You delete train X (trashes 'my' backup, makes a new backup)
- I build a new train to replace W (not in 'your' depot, thus no cargo restore)
- I delete train Y (trashes your backup, makes a new backup)
- You build a new train to replace X (not in my depot, thus no cargo restore)

With orders:
- I delete train W (backs up orders locally!)
- You delete train X (backs up order locally)
- I build a new train to replace W (backed up orders get reinserted)
- I delete train Y (backups up orders locally)
- You build a new train to replace Y (backed up orders get reinserted)

See the difference? With cargo the cargo for both is gone, with orders both orders survived. However, you can't do the order trick for cargoes as that means one can freely insert cargo into vehicles and thus create massive profit.
laurijh
Engineer
Engineer
Posts: 19
Joined: 21 Jul 2009 11:52
Location: Finland

Re: Patch: Cargo backup/restore in depot during renewal

Post by laurijh »

Yes, backups within a company currently do overwrite previous ones. I guess the best way would be to make the backups both tile- and player-specific so that two players can't mix up each other's backups even inside the same depot. There seems to be a global _network_own_client_id that could be used for this..? Can I trust that 2 players in the same company won't try to mess up each other's game by passing the wrong client ID to the Cmd? :)
User avatar
belugas
OpenTTD Developer
OpenTTD Developer
Posts: 1507
Joined: 05 Apr 2005 01:48
Location: Deep down the deepest blue
Contact:

Re: Patch: Cargo backup/restore in depot during renewal

Post by belugas »

Never trust players
Never assume (you'll make an a s s of you and me)
Always go for security
If you are not ready to work a bit for your ideas, it means they don't count much for you.
OpenTTD and Realism? Well... Here are a few thoughs on the matter.
He he he he
------------------------------------------------------------
Music from the Bloody Time Zones
laurijh
Engineer
Engineer
Posts: 19
Joined: 21 Jul 2009 11:52
Location: Finland

Re: Patch: Cargo backup/restore in depot during renewal

Post by laurijh »

belugas; ok, I agree on a general level but 2 players in the same company have much easier ways to harass each other. Besides, having player A fake player B's ID only allows A to create cargo backups with B's ID, and restore B's backups to vehicles that A himself creates. There doesn't seem to be any score kept between co-operating players so there's very little point or advantage for A to be doing this.

Of course, I would add a check for the ClientID but I couldn't find any way to check the ID of a command's initiator (player, not company) from inside the Cmd.

I updated the first post with a new version.
laurijh
Engineer
Engineer
Posts: 19
Joined: 21 Jul 2009 11:52
Location: Finland

Re: Patch: Cargo backup/restore in depot during renewal

Post by laurijh »

Updated with a small bugfix; restoring to an aircraft shadow vehicle (usually carrying mail) wasn't working when e.g. 2 helicopters were sold and then a larger plane was built.
petert
Tycoon
Tycoon
Posts: 3008
Joined: 02 Apr 2009 22:43
Location: Massachusetts, USA

Re: Patch: Cargo backup/restore in depot during renewal

Post by petert »

I agree with Rubidium, this whole patch really can't be used to its full potential when playing with 2 players in one company.
laurijh
Engineer
Engineer
Posts: 19
Joined: 21 Jul 2009 11:52
Location: Finland

Re: Patch: Cargo backup/restore in depot during renewal

Post by laurijh »

Actually I mostly fixed the 2-players-in-1-company issue right after the first patch version. The cargo backups are now marked with each player's own ClientID so they shouldn't get mixed up anymore.

There was still a bug when 2 players were selling/building vehicles in the same depot at the same time but that's now also fixed in v0.4 of the patch (updated first post).
petert
Tycoon
Tycoon
Posts: 3008
Joined: 02 Apr 2009 22:43
Location: Massachusetts, USA

Re: Patch: Cargo backup/restore in depot during renewal

Post by petert »

Sounds good.
Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 3 guests