Coding Tutorial: Monthly Costs Per Track Tile

[bigos]

Hmm I can try. There is small problem with ownership of roads but tomorrow will do it.

[smallfly]

Hmm I can try. There is small problem with ownership of roads but tomorrow will do it.

Dont put too much work in that point (roads). The track maintenance costs are the important point that make ottd worth to play for strategic players (but only if you balance them correctly; the maintenance costs may not to be to less => no effect or too high => "unwinable")

[bigos]

the maintenance costs may not to be to less => no effect or too high => "unwinable")

That's why I ask you to test it and share your results Its realy easy to cahnge costs and if it will be needed I can make GUI in advanced settings to change it during game. Probably I will make it sooner or later...

[smallfly]

the maintenance costs may not to be to less => no effect or too high => "unwinable")

That's why I ask you to test it and share your results Its realy easy to cahnge costs and if it will be needed I can make GUI in advanced settings to change it during game. Probably I will make it sooner or later...

You dont need a single tester for that. What I did during my short development phase on this patch: I switched on the patch and joined several multiplayer games. Then you check the financial data of all players, mainly players that built many tracks. You'll fastly see how high to set the maintenance costs. After a while you will get a desync of course.

You will need to find a maintenance cost multiplier that really hurts. Only that way people will start thinking like "do I really need 8 track lanes next to each other crossing the map ... or would I get more profit if I only had 4 track lanes but more trains on them"

[bigos]

I will try do that. For now costs are like renew all stuff ~ once per 3 years. Maybe should be like once per 2 years... or even 1 year. Still would be nice to hear some players opinions.

[smallfly]

I will try do that. For now costs are like renew all stuff ~ once per 3 years. Maybe should be like once per 2 years... or even 1 year. Still would be nice to hear some players opinions.

Every 3 years? Nobody will even notice that. Do 1 year per default. And wait for further player reactions to adjust the value.

[bigos]

Yea maybe you have right. Its easy to change. Just all ">> 5" replace in patch with ">> 3" then it will be like renew everything every 8 months. I'm not sure if its to much for first months of play.

[bigos]

There was bug in last version connected with rail waypoints. Fixed and made 5-6 first years of play free from paying for tracks - to make game bit easier at start.

[Hirundo]

Since you are learning how to code for openttd, I'll give some constructive criticism on the code.

- CountBits (core/bitmath.hpp) may be of interest to you.
- There is too much code duplication for my taste, especially with the new 'year > 5' check added everywhere. I would store a per-company total in an array. Then subtract the whole amount at once, if the company is old enough.
- Long blocks of indented code are not very easy to read. Instead of if (IsValidTile(tile)){ ... long code block..}, you could use if (!IsValidTile(tile)) continue;
- Please put spaces between '}' and 'else', and between 'if' and the round bracket following it. See http://wiki.openttd.org/Coding_style

As a little treat you will find that your code won't update cleanly to the latest trunk.

[bigos]

But what array should I use here? I dont know how big it should be because doont know how many companies can be... Should i use vector or better list? Maybe if you have time then help a bit and show some code
BTW I had to use 'trick' because game crush when you try to check with function IsDepotTile(tile) the tile with waypoint. Its crushing on checking if its hangar because it pass assertion check with Station in IsHangar(tile) function.
The checking how old company is is also tricky... Is there any way to check how many months have company?

[Hirundo]

But what array should I use here? I dont know how big it should be because doont know how many companies can be...

There can never be more companies than MAX_COMPANIES. Using an array would be my choice, the overhead of using a SmallVector is IMO not worth it.

BTW I had to use 'trick' because game crush when you try to check with function IsDepotTile(tile) the tile with waypoint. Its crushing on checking if its hangar because it pass assertion check with Station in IsHangar(tile) function.

crush != crash. I don't quite understand what 'trick' you mean. In general, using Is***Tile functions should not cause any crashes.

[bigos]

crush != crash. I don't quite understand what 'trick' you mean. In general, using Is***Tile functions should not cause any crashes.

But it dose. function IsDeoptTile() using IsHangarTile() witch use IsHangar function witch is not resistible for rail waypoints. Waypoints are diferent stations thats why they pass assertion in IsHangar() and crush game with error on next line.
The "trick" is to check if its waypoint before function IsDepotTile...

As I see MAX_COMPANIES is set to 15 for now... I was afraid of big memory usage but with such number probably you have right with this array. But still need if else construction.

[Hirundo]

But it dose. function IsDeoptTile() using IsHangarTile() witch use IsHangar function witch is not resistible for rail waypoints. Waypoints are diferent stations thats why they pass assertion in IsHangar()

I consider this to be a bug, I would report it on FlySpray.

Using this approach, you will only need one if(company age > xxx) { some code } instead of four. Code duplication is bad, in general.

[bigos]

Modified patch a bit base on your instructions. To make code simpler all depots have now same costs but you also pay for hangars. Payment for signals is higher - if you use 2 way signals.
Waiting for your opinion about code.

[Hirundo]

I would recommend testing your patch. At the moment it contains at least two serious bugs. Proper testing and/or code examination should reveal these relatively fast.

With regards to the code, I would do something like this:

Code: Select all

/* all non-initialized members are automatically set to 0 */
Money total_costs[MAX_COMPANIES] = {0};

/* when adding a cost */
total_costs[GetTileOwner(tile)] += _prices[PR_XXX] >> 4;

This saves a few variables, and makes it much easier to add or remove costs.

PS the bug mentioned above is fixed in r18035.

[bigos]

Heh I've added "continue" modification after tests on normal if block that's why it was not working But cant find second bug. Can you show it?

Here is code and patch file after your suggestions:

Code: Select all

	Company *c;
	Money total_costs[MAX_COMPANIES] = {0};
	
	for (TileIndex tile = 0; tile < MapSize(); tile++) {

		if (!IsValidTile(tile)) continue;

		if(IsLevelCrossingTile(tile)) {
			total_costs[GetTileOwner(tile)] += _price[PR_BUILD_RAIL] >> 4;

		} else if (IsRailWaypointTile(tile)) {
			total_costs[GetTileOwner(tile)] += _price[PR_BUILD_STATION_RAIL] >> 4;

		} else if (IsDepotTile(tile)) {
			total_costs[GetTileOwner(tile)] += _price[PR_BUILD_DEPOT_ROAD] >> 4;

		} else if (IsPlainRailTile(tile)) {
			total_costs[GetTileOwner(tile)] += CountBits(GetTrackBits(tile)) * (_price[PR_BUILD_RAIL] >> 4);
			total_costs[GetTileOwner(tile)] += CountBits(GetPresentSignals(tile)) * (_price[PR_BUILD_SIGNALS] >> 4);
		}		
	}

	FOR_ALL_COMPANIES(c) {
		_current_company = c->index;
		if (c->inaugurated_year + 5 < _cur_year && total_costs[c->index] > 0) {
			CommandCost cost(EXPENSES_PROPERTY, total_costs[c->index]);
			SubtractMoneyFromCompany(cost);
		}
	}

[Tekky]

You may also want to add additional maintenance cost for railroad switches, which would be realistic.

For example, if one tile contains several track pieces that are arranged in such a way that a train could choose a different route, this tile should be considered a switch and cost five times more maintenance than a regular tile. Maybe more complex switches should also cost more.

However, a tile with two diagonal parallel track pieces should not be considered a switch, since the track pieces are not connected to each other.

[bigos]

I think I know how to do it but I don't know is it good idea. Tracks are already bit expensive... Would be nice to see others opinion about this.