Patch: Improved acceleration for road vehicles [In trunk]

[Terkhen]

Check again with a train that carries cargo in a powered part of the consist. If you plan to understand a patch, you should also read its thread completely; that will save time for both you and me.

[2007Alain2007]

I never ever saw it did that till today when i tested it

I realy am sorry

[Terkhen]

Here is version 9. I didn't feel like trying callbacks again, so I implemented some eyecandy instead.

v9-r17549: Weight is now shown with decimals. Corrected a bug while showing tractive effort in the build vehicle / engine windows.

Medhead Transport, 5º Feb 2045.png (44.6 KiB) Viewed 3860 times

The code is not very sweet, though. Here are the key pieces of it:

Code: Select all

STR_PURCHASE_INFO_WEIGHT_CWEIGHT                                :{BLACK}Weight: {GOLD}{WEIGHT_S} ({WEIGHT_S})
STR_PURCHASE_INFO_DECIMAL_WEIGHT_CWEIGHT                        :{BLACK}Weight: {GOLD}{COMMA},{WEIGHT_S} ({COMMA},{WEIGHT_S})

Code: Select all

int16 weight = e->GetDisplayWeight();
SetDParam(0, weight / 4);
int64 decimal = 25 * (weight % 4);
SetDParam(1, (decimal % 2) ? decimal : (decimal / 10));
uint cargo_weight = weight + (CargoSpec::Get(e->GetDefaultCargoType())->weight * GetTotalCapacityOfArticulatedParts(engine_number, VEH_ROAD)) / 4;
SetDParam(2, (cargo_weight) / 4);
decimal = 25 * ((cargo_weight) % 4);
SetDParam(3, (decimal % 2) ? decimal : (decimal / 10));
DrawString(left, right, y, STR_PURCHASE_INFO_DECIMAL_WEIGHT_CWEIGHT);

I know that this is a hack, but I don't know which would be the best way of implementing it. I also don't know which is the correct separator for decimals. Any suggestions on the best way to implement this code will be welcome.

[Comm Cody]

A binary Please?

[SirkoZ]

120 hp for a ' Leopard bus is [strike]unrealistic[/strike] unimproved. 240 hp should do nicely. for the newer Volvo busses 300 and 422 sounds nice.

[Terkhen]

Comm Cody: I finally had the time and the possiblity of installing Linux. As a result, I am not providing windows binaries anymore. If anyone is willing to upload one it will be welcome.

SirkoZ: The default acceleration values used for standard vehicles are already in OpenTTD trunk (they are just ignored), and they are the same in TTDPatch. Changing these values is out of the scope of this patch. I have played a few testing games mostly with standard coal trucks and they performed nicely in my opinion.

[Rubidium]

Some comments:

• Speeds are all in mph/3.2, not in 0.5 km/h. So, if you need it in mph multiply by 3.2 instead of 3.1.
• Please document all enums, enum values and functions you are adding. Old functions might not have comments, but they should've had them.
• I got the feeling code can be shared between the train and road vehicle acceleration; duplicating it seems the wrong way to go.
• Either introduce a new 'control character' for the decimal weight or, maybe even better, update the current one so it takes 1/4 tons. How needed is it actually to have such precision in the GUI?
• Related with the above point: if you change the measuring units to SI you'll get 64.000 kg instead of 64 t. Your magic with the comma will likely (haven't tested it) fail there. I except something like 9,25.000 kg instead of 9.250 kg.
• Again related with the above point: don't hardcode group/decimal separators in the translation as they are user configurable.

[Terkhen]

Speeds are all in mph/3.2, not in 0.5 km/h. So, if you need it in mph multiply by 3.2 instead of 3.1. Please document all enums, enum values and functions you are adding. Old functions might not have comments, but they should've had them.

These two issues will be corrected in the next version.

I got the feeling code can be shared between the train and road vehicle acceleration; duplicating it seems the wrong way to go.

I was thinking about getting this done at some point. Now that I have the acceleration code defined, I guess I can start working in a common implementation for both acceleration models. I don't know in which class would be better to put the common code, though. In the vehicle class, in a new class that would implement this behaviour or something else?

Either introduce a new 'control character' for the decimal weight or, maybe even better, update the current one so it takes 1/4 tons. How needed is it actually to have such precision in the GUI?

Since the weight of road vehicles is set in units of 1/4 tonnes, I think it is better to show the weight with this precision. I wasn't sure about changing the current control character, as such precision is probably not needed in trains except maybe for showing the weight of a wagon including cargo.

[andythenorth]

I've compiled against r17549. Acceleration looks good, but due to the changes in weight handling a few versions back, I need to change values for some HEQS vehicles and test again.

As pointed out, the commas aren't appropriate decimal indicators in all locales. I read 17,01 as 1701 (and my brain thinks there's a badly placed comma-thousand which is confusing). I'm not sure decimals are needed in the GUI.

Keep going!

cheers,

Andy

[Terkhen]

Indeed, it does look ugly.

v10_r17571: Weight is restored to the normal behaviour. Corrected the mph conversion error. Every enum, enum value and function is now documented.

For now I will halt the weight in decimals feature. I am going to start working in the unification of train and road vehicle acceleration codes: I have some ideas on how to do it, but I have two big questions.

• Should I implement the acceleration functions in a class that inherits vehicle and then make road vehicles and trains inherit from that class, or is there a better solution?
• Should realistic acceleration for trains be renamed to improved acceleration too?

[andythenorth]

Should realistic acceleration for trains be renamed to improved acceleration too?

I would say yes, both because it's not "realistic" and because "realism" isn't something the game promises (nor should it).

It will confuse players who are looking for the old setting, but not for long.

my 2p

[Hirundo]

Should I implement the acceleration functions in a class that inherits vehicle and then make road vehicles and trains inherit from that class, or is there a better solution?

I think that making such a class will conflict with the existing SpecializedVehicle that also requires a place between Vehicle and Train/RoadVehicle/Ship/Aircraft.

My suggestion: a function template that takes the magic constants in the acceleration function as parameters. You could call this function via v->GetImprovedAcceleration() (virtual function) or something similar.

[Terkhen]

Since I'm planning to get all acceleration related code unified (the acceleration cache, the functions that update it and the acceleration function itself), I think it's better to make a single templated class with all needed functions inside of it. Right now I have managed to create that base class (inherits from SpecializedVehicle instead of Vehicle), and both trains and road vehicles are already working with the same acceleration cache. Here is the key part of the code:

EDIT2: This code is obsolete. Check the patch at the first thread for an updated version of the code.

Code: Select all

+/**
+ * Cached acceleration values.
+ * All of these values except cached_veh_weight are set only for the first part of a vehicle.
+ */
+struct AccelerationCache {
+	/* Values recalculated on load or when the vehicle is created, serviced or visits a station. */
+	uint32 cached_power;      ///< Total power of the consist.
+	uint32 cached_air_drag;   ///< Air drag coefficient of the vehicle in arbitrary units.
+
+	/* Cached values, recalculated when the cargo on a road vehicle changes (in addition to the conditions above) */
+	uint32 cached_weight;     ///< Total weight of the road vehicle including cargo.
+	uint32 cached_veh_weight; ///< Total weight of this part of the road vehicle. For all parts besides the first, this means cargo weight.
+	uint32 cached_max_te;     ///< Max tractive effort of the road vehicle.
+};
+
+/**
+ * Class that defines functions used by vehicles with an improved acceleration model (trains and road vehicles).
+ */
+template <class T, VehicleType Type>
+struct AccelerationVehicle : public SpecializedVehicle<T, Type> {
+	AccelerationCache acc_cache;
+
+	AccelerationVehicle() : SpecializedVehicle<T, Type>() {}
+};

Trains and road vehicles inherit from this class, and both of them use acc_cache instead of rcache and tcache (of course, acceleration values have been removed of these caches). I plan to get all acceleration functions implemented in this class, implementing the parts that are different in both types of vehicles (powered wagons, curve handling, etc) in the child classes. I have ran into a big problem to unify all code smoothly, though: the presence of different magic numbers for the same vehicle property in the GetVehicleProperty function. Instead of using a lot of ugly conditional instructions, I'm planning a prerrequisite patch that will remove the need of using these magic numbers while checking callbacks in OpenTTD code, so probably the unified acceleration will get delayed.

EDIT: I have posted this prerrequisite patch at flyspray: http://bugs.openttd.org/task/3222

[Comm Cody]

May someone post a Win 32 Binary? And before you say "Go get the road pak" I couldn't load my old precious saves of the Rhine Mosel Bahn and so I would prefer the Improved RV acceleration.

[Terkhen]

After a lot of headaches, v11 is finished. Grab the diff file at the first post.

v11_r17736: Code unification complete: trains and road vehicles use the same cache, cache update functions and improved acceleration function. Lots of code changes caused by the unification and by performance improvements. Train air drag behaviour changed to implement the behaviour described at NewGRF documentation. NewGRF support for air drag in trains (untested). If tractive effort is not set or set to zero, the default value (0x4C) should be used: this was not true for trains in trunk. Yet another GUI bug caused by weight corrected.

I implemented the idea that I described in my last post. The AccelerationVehicle class contains the acceleration cache, and it implements two cache update functions and the GetAcceleration function. Road vehicles and trains use these common functions. The parts that vary between road vehicles and trains are defined as virtual functions at the AccelerationVehicle class, and they are implemented as inline functions at the Train and RoadVehicle classes. I have taken special care in making the unified code clean, readable and fast, but any reviews will be greatly appreciated as it surely can be improved.

Since every piece of different behaviour is now a different function, the improved acceleration code uses a lot of function calls: I don't know if making all of these functions FORCEINLINE solves all performance issues caused by this. Besides that, I tried to get as many calculation as possible done at the cache updating functions to make GetAcceleration lighter.

As a by-product of the unification, there are some changes to train acceleration behaviour. With the new air drag model, the first steam locomotives accelerate a bit faster than in trunk (I haven't tested any other engines). The unification also opens the door to implement road vehicles that behave as maglev vehicles in the future (probably as a miscellaneous flag).

After a much-needed break from this patch I will test NewGRF support for air drag in trains and upload the testing grf file.

[Terkhen]

I have updated the first post with a complete specification of all NewGRF features and changes.

[andythenorth]

After a lot of headaches, v11 is finished. Grab the diff file at the first post.

Have tested basic behaviour for RVs and trains. Trains don't seem broken. Gameplay effects on RVs look about right, except as discussed, most trucks go over large hills like they aren't there.

I don't have any code that tests the callbacks. I do have an idea for HEQS trucks with trailers, where TE coefficient could vary with consist load, but it's not really necessary.

Looking good

[Terkhen]

I'm thinking that maybe it is better to let the callbacks out of this patch. That way the patch as a whole would be simpler to test, and callbacks can always be added as separate patch(es) later anyways.

After more testing of the new air drag model in trains (still no NewGRF tests), I'm thinking about increasing the final air drag value a bit more, as trains now accelerate a bit faster than in trunk. A perfect adjustment is not posible since in trunk all trains use the same air drag and now they use different values, but this will make the difference smaller. I also have to check road vehicle behaviour in slopes, because (as andythenorth reported) right now is like the slope wasn't even there.

[Terkhen]

New version of the patch:

v12-r17802:

• AccelerationVehicle class renamed to AcceleratedVehicle.
• accelerated_vehicle.h renamed to accelerated_vehicle.hpp, since it contains a templated class.
• More documentation added and corrected
• Fixed compilation for Visual Studio
• Air drag tweaked to make it as similar to old air drag behaviour in trains as possible.
• Road vehicle behaviour in slopes corrected. (5/2 of the slope used for trains = 7.5% slope).

Now road vehicles take a significant speed hit when climbing slopes, but the solution adds another problem: trains use a 3% slope and road vehicles use a 7.5% slope. As long as it improves gameplay I don't have a problem with this difference, but any comments on this will be welcome. The 7.5% value could get some tweaking too (probably it should be lower, but it seemed okay while I was testing it), so please give me your opinion after testing this new version.

[Comm Cody]

I'll try to compile it.

Original Post was: Win32 Binary?