[v2.1.0] Template based train replacement [OpenTTD 1.9.3]

[FLHerne]

I've been playing with it and some other patches for a while, and haven't found any obvious problems.

Feature request: Would it be possible to show the length of the train in the template-creation window?

A really nice patch, I've been wanting something like this for a while. Thanks for making it!

[ffpp]

Tried building again, after trowing out my local source copy and checking out a new one. Clean OpenTTD worked. After applying the 0.1b patch I get this warning:

7>d:ttdopenttdtrunk.hgsrcvehicle.cpp(814): warning C4800: 'OrderDepotActionFlags' : forcing value to bool 'true' or 'false' (performance warning)

Seems like something outside of my patch from the first glance.

And more worryingly these errors:

7>Link:
7> Creating library D:TTDOpenTTDtrunk.hgprojects..objsWin32Debugopenttd.lib and object D:TTDOpenTTDtrunk.hgprojects..objsWin32Debugopenttd.exp
7>vehicle.obj : error LNK2019: unresolved external symbol "struct TemplateReplacement * __fastcall GetTemplateReplacementByGroupID(unsigned short)" (?GetTemplateReplacementByGroupID@@YIPAUTemplateReplacement@@G@Z) referenced in function "void __fastcall VehicleEnteredDepotThisTick(struct Vehicle *)" (?VehicleEnteredDepotThisTick@@YIXPAUVehicle@@@Z)
7>vehicle.obj : error LNK2019: unresolved external symbol "class CommandCost __fastcall CmdTemplateReplaceVehicle(struct Train *,bool,enum DoCommandFlag)" (?CmdTemplateReplaceVehicle@@YI?AVCommandCost@@PAUTrain@@_NW4DoCommandFlag@@@Z) referenced in function "void __fastcall CallVehicleTicks(void)" (?CallVehicleTicks@@YIXXZ)
7>vehicle.obj : error LNK2019: unresolved external symbol "class CommandCost __fastcall TestBuyAllTemplateVehiclesInChain(struct Train *)" (?TestBuyAllTemplateVehiclesInChain@@YI?AVCommandCost@@PAUTrain@@@Z) referenced in function "void __fastcall CallVehicleTicks(void)" (?CallVehicleTicks@@YIXXZ)
7>group_gui.obj : error LNK2019: unresolved external symbol "void __fastcall ShowTemplateReplaceWindow(unsigned short,unsigned int,struct VehicleListIdentifier,class GUIList<struct Vehicle const *,char const *> *,unsigned char,int)" (?ShowTemplateReplaceWindow@@YIXGIUVehicleListIdentifier@@PAV?$GUIList@PBUVehicle@@PBD@@EH@Z) referenced in function "public: virtual void __thiscall VehicleGroupWindow::OnDropdownSelect(int,int)" (?OnDropdownSelect@VehicleGroupWindow@@UAEXHH@Z)
7>group_cmd.obj : error LNK2019: unresolved external symbol "short __fastcall deleteIllegalTemplateReplacements(unsigned short)" (?deleteIllegalTemplateReplacements@@YIFG@Z) referenced in function "class CommandCost __fastcall CmdDeleteGroup(unsigned int,enum DoCommandFlag,unsigned int,unsigned int,char const *)" (?CmdDeleteGroup@@YI?AVCommandCost@@IW4DoCommandFlag@@IIPBD@Z)
7>afterload.obj : error LNK2019: unresolved external symbol "void __fastcall AfterLoadTemplateVehicles(void)" (?AfterLoadTemplateVehicles@@YIXXZ) referenced in function "bool __fastcall AfterLoadGame(void)" (?AfterLoadGame@@YI_NXZ)
7>saveload.obj : error LNK2001: unresolved external symbol "struct ChunkHandler const * const _template_vehicle_chunk_handlers" (?_template_vehicle_chunk_handlers@@3QBUChunkHandler@@B)
7>saveload.obj : error LNK2001: unresolved external symbol "struct ChunkHandler const * const _template_replacement_chunk_handlers" (?_template_replacement_chunk_handlers@@3QBUChunkHandler@@B)
7>saveload.obj : error LNK2001: unresolved external symbol "struct Pool<struct TemplateVehicle,unsigned short,512,65536,1,0,1> _template_pool" (?_template_pool@@3U?$Pool@UTemplateVehicle@@G$0CAA@$0BAAAA@$00$0A@$00@@A)
7>D:TTDOpenTTDtrunk.hgprojects..objsWin32Debugopenttd.exe : fatal error LNK1120: 9 unresolved externals
7>
7>Build FAILED.

This is bad. From the first look of it, it seems like the linker errors are concerned with all the entry points to functions provided by the patch.
Maybe the windows code uses some different type of include directive ? I know that the name mangling is different for c-style functions and c++-style functions, at least for windows compilers.

I've been playing with it and some other patches for a while, and haven't found any obvious problems.

Feature request: Would it be possible to show the length of the train in the template-creation window?

Yes, this was in my mind as well, I'll put it in the list.

[Zuu]

I've made a new attempt, now with the 0.1b version of your patch.

aaa_template_replacement_sl.cpp:

Code: Select all

extern const ChunkHandler _template_replacement_chunk_handlers[] = {
	{'TMPL_RPLS', Save_TMPL_RPLS, Load_TMPL_RPLS, NULL, NULL, CH_ARRAY | CH_LAST},
};

This gives an error because 'TMPL_RPLS' is too long to fit in a uint32. If you lookup the declaration of ChunkHandler, you'll see that it states that the id should be 4 characters.

What g++ does is either truncating your constant to 'TMPL' or writing outside of the buffer. If the assignment of the other struct members happens just after and are aligned after the id in memory, that overwrite will probably not be noticed.

Another instance of the same problem can be fonud in aaa_template_veh_sl.cpp:

Code: Select all

extern const ChunkHandler _template_vehicle_chunk_handlers[] = {
	{'TMPLS', Save_TMPLS, Load_TMPLS, Ptrs_TMPLS, NULL, CH_ARRAY | CH_LAST},
};

Last problem is in aaa_template_gui_main.cpp:

Code: Select all

GUITemplateList::SortFunction *template_sorter_funcs[];

Visual Studio 2008 complain that the array doesn't have a size. Since an array-pointer is just a pointer to the first element, you can just make a pointer:

Code: Select all

GUITemplateList::SortFunction **template_sorter_funcs;

Patch with all my changes: (I haven't verified that the new chunk names don't conflict with anything else in OpenTTD)

fixes.patch

Fixes to 0.1b to get it to compile on Visual Studio 2008.

(1.37 KiB) Downloaded 266 times

Binary:

train_templates_0.1b.zip

* Built using Visual Studio 2008
* OpenTTD r24018 + Train Templates 0.1b + fixes.patch
* No DirectMusic

(4.73 MiB) Downloaded 271 times

[Zuu]

Tried building again, after trowing out my local source copy and checking out a new one. Clean OpenTTD worked. After applying the 0.1b patch I get this warning:

7>d:ttdopenttdtrunk.hgsrcvehicle.cpp(814): warning C4800: 'OrderDepotActionFlags' : forcing value to bool 'true' or 'false' (performance warning)

And more worryingly these errors:

7>Link:
7> Creating library D:TTDOpenTTDtrunk.hgprojects..objsWin32Debugopenttd.lib and object D:TTDOpenTTDtrunk.hgprojects..objsWin32Debugopenttd.exp
...
7>D:TTDOpenTTDtrunk.hgprojects..objsWin32Debugopenttd.exe : fatal error LNK1120: 9 unresolved externals
7>
7>Build FAILED.

Did you forgot to re-generate the project files before compiling?

Run projects\generate.vbs to re-generate all the Visual Studio project files so that the new source files get included in the project.

[Wowanxm]

ffpp, thank you for your undertaking!

Another feature request: would it be possible to order template based autorenew for current train/group of trains at the specific depot?

[ffpp]

Ah yes, the saveload code I did on a train ride to or from home, it was a bit harder to concentrate there :>
I will merge your changes with my latest ones before I make the next upload. I'm currently finishing the cargo refit for and from templates.

Tried building again, after trowing out my local source copy and checking out a new one. Clean OpenTTD worked. After applying the 0.1b patch I get this warning:

7>d:ttdopenttdtrunk.hgsrcvehicle.cpp(814): warning C4800: 'OrderDepotActionFlags' : forcing value to bool 'true' or 'false' (performance warning)

And more worryingly these errors:

7>Link:
7> Creating library D:TTDOpenTTDtrunk.hgprojects..objsWin32Debugopenttd.lib and object D:TTDOpenTTDtrunk.hgprojects..objsWin32Debugopenttd.exp
...
7>D:TTDOpenTTDtrunk.hgprojects..objsWin32Debugopenttd.exe : fatal error LNK1120: 9 unresolved externals
7>
7>Build FAILED.

Did you forgot to re-generate the project files before compiling?

Run projectsgenerate.vbs to re-generate all the Visual Studio project files so that the new source files get included in the project.

Shouldn't there be compilation errors before it even gets to link anything when it was missing source files ?

[ffpp]

ffpp, thank you for your undertaking!

Another feature request: would it be possible to order template based autorenew for current train/group of trains at the specific depot?

What do you mean with autorenew ? The replacement as it is done by the patch right now or the autorenew feature where the game just replaces a vehicle by a newer one of the same type ?

My guess would be the former, and it should be very well doable to replace all trains only in a specific depot. But it sounds more like a bit of more advanced feature and would be further down the list for now.

[Zuu]

Shouldn't there be compilation errors before it even gets to link anything when it was missing source files ?

Not necessarily. When you include .h files that aren't in the project, the compiler will not complain as long as the file exist. The .h file will give all information needed to compile some source file that call methods or use types that are declared in the .h file. However as the corresponding .cpp file is not included in the project, the implementation of those methods are never compiled and the linker will thus not find them when trying to connect the different code blocks.

[ffpp]

Of course, you're right.

[FooBar]

I didn't know I had to run that. When it comes to compiling, I can click the buttons the instructions tell me to click (or run the commands it tells me to run). The instructions I followed didn't say anyting about regenerate.vbs. For what it's worth I did a 'rebuild solution' after the clean source build.
I'll give that a shot later, to see if that indeed fixes the unresolved thing.

[Eddi]

well, usually it's the responsibility of the patch author to run that, after he modified source.list. (there's also a script that runs on linux, so that's not an excuse for not doing it)

[Ottomobeale]

WHOA!
If you make this work for road vehicles too, you win patch of the year for sure!

[FooBar]

Why would you want this for road vehicles
You can already do group-based replacements for them. And since they're always one piece...

[Eddi]

but you can't define refit-subtype based replacement

[FooBar]

I didn't know this could. Well in that case that voids my previous remark completely. Although for road vehicles the subtype should probably be implemented in the regular autoreplace (as well as for any other vehicle type), which may be a bit outside of the scope of this patch. It certainly shouldn't be combined in one big patch, looking at possible trunk inclusion.

[Ottomobeale]

Sorry if I offended.

It would just be nice to have the ability to define this in road vehicles like the OP describes in this thread:

If cargo = type x, type y, type z then replacement vehicle = vehicle xx and refit accordingly.
If cargo = type a, type b, type c then replacement vehicle = vehicle yy and refit accordingly.

[FooBar]

Sorry if I offended.

No, you didn't. If somebody offended it's probably me.

In my head I didn't realize there was more to this than just "replace this train and wagon combination with that train and wagon combination". Your suggestion is more than valid.

[ffpp]

Sorry if I offended.

It would just be nice to have the ability to define this in road vehicles like the OP describes in this thread:

If cargo = type x, type y, type z then replacement vehicle = vehicle xx and refit accordingly.
If cargo = type a, type b, type c then replacement vehicle = vehicle yy and refit accordingly.

Did I say this ?
Currently, decision if replacement or not is solely based on group membership.
What you can do, is define what cargo refit the new vehicles will be using.

Or did I not get something ?

[ffpp]

Hi,

here is a new version of the patch, including

• Zuu's fixes
• needed vcproj files for compiling on windows
• features
  • trains composed under template replacement get the specified cargo refit setting
  • now configurable replacement behaviour (as described in the OP, about reusing idle depot vehicles and selling the remains of a trains after template replacement)
  • template length (in tile units) displayed in edit window and main gui

Here is a small screenshot of the changes as seen in the gui, maybe there could be some reorganization of the information presented as it can become cluttered with long trains:


the new patch is found in the first post again.

[Wowanxm]

Another feature request: would it be possible to order template based autorenew for current train/group of trains at the specific depot?

What do you mean with autorenew ? The replacement as it is done by the patch right now or the autorenew feature where the game just replaces a vehicle by a newer one of the same type ?

Yes, I mean autoreplacement exactly as it is done.

My guess would be the former, and it should be very well doable to replace all trains only in a specific depot. But it sounds more like a bit of more advanced feature and would be further down the list for now.

[/quote]It would be great to let the player choose in what depot ro to replace train.
Futhermore, it might be interesting feature if we could insert replacement procedure as a specific order in the train's timetable.