JGR's Patch Pack

[SimYouLater]

Sometimes the NewGRF you want to use, whether you want it because of aesthetics or region theme, doesn't fit that requirement. It's easier to provide a toggleable feature to automatically apply it than to convince a NewGRF creator to add the feature by hand. Especially if said set is no longer being actively developed, or worse, the creator is gone. Also, some sets have licenses which don't even allow you to create a fork with running costs.

It may be easier for you, but additional patches and settings just move the cost/effort onto me.
The benefit/cost ratio of this sort of niche feature looks poor to me.

You do realise that long trains have a distinct advantage over short ones without that patch, right? When I say "easier" I mean "possible". It is literally illegal in some cases to get the desired result through anything other than this patch.

[JGR]

It is literally illegal in some cases to get the desired result through anything other than this patch.

There really is no shortage of train GRFs.

On the topic of legalities, given that OpenTTD is GPL there's nothing stopping you from applying that patch or any other to your own copy.

[SimYouLater]

It is literally illegal in some cases to get the desired result through anything other than this patch.

There really is no shortage of train GRFs.

On the topic of legalities, given that OpenTTD is GPL there's nothing stopping you from applying that patch or any other to your own copy.

Which of the following would you rather have someone do?

a) Modify a train set for a particular country/region, which there are no other train sets for, for personal use. This requires learning how to modify NewGRFs, and may be illegal to publicly distribute which would require everyone to modify it themselves.
b) Learn how to apply a patch, which is very hard to do on windows thanks to the far from easy-to-read info on the wiki about compiling patches.
c) Download a patch pack where everything is ready for them.

I can't forsee that patch requiring much future modification. If you want, put it on the backburner, but don't cast it aside just because it looks like its not worth the effort. Believe me, it is, but some people can't wrap their heads around programming at all which puts it out of their reach.

[andythenorth]

@SimYouLater you're not likely to persuade a patch-pack maintainer who's said no.

If you want a patch-pack built to suit you, start your own

There are two key things that drive developers away from working on OpenTTD:
1. they just get bored of it
2. players demanding stuff

Maintaining a patch-pack is a PITA, we're lucky to have a couple of people maintaining patch-packs right now.

b) Learn how to apply a patch, which is very hard to do on windows thanks to the far from easy-to-read info on the wiki about compiling patches.

^ this one. Learning is good.

[HackaLittleBit]

Thanks much appreciated!

Having IsTunnelInWay be O(N) in the total number of tunnels seems potentially problematic, but that could be a non-issue in practice.

I will fix this with something like : z > 0 ? true : false

EDIT: if (t->is_chunnel > not_allowed)
0 > 0 or 0 > 1 == false, so I think that part stands.
Question for me here is more "what is faster".
The old way or my way or maybe a combination of the two.
But that I will investigate later.

There are still some corner cases that need to be fixed, so I am back at the drawing board.
I need to get this beast under control.

[SimYouLater]

@SimYouLater you're not likely to persuade a patch-pack maintainer who's said no.

If you want a patch-pack built to suit you, start your own

There are two key things that drive developers away from working on OpenTTD:
1. they just get bored of it
2. players demanding stuff

Maintaining a patch-pack is a PITA, we're lucky to have a couple of people maintaining patch-packs right now.

Alright, fair enough. I'll stop talking about it. Just please consider it, JGR. It's a very useful feature.

b) Learn how to apply a patch, which is very hard to do on windows thanks to the far from easy-to-read info on the wiki about compiling patches.

^ this one. Learning is good.

If you want people to learn how to patch, and you know how to do it on Windows 10, kindly update the wiki. Right now it's a mess.

[HackaLittleBit]

If you want people to learn how to patch, and you know how to do it on Windows 10, kindly update the wiki. Right now it's a mess.

Here ya go.
Install Linux Mint Cinnamon.
Forget windows 10!


P.S.
I managed to compile trunk with msys2.
The effort was enough to make me switch operating system!

[burty]

If you want people to learn how to patch, and you know how to do it on Windows 10, kindly update the wiki. Right now it's a mess.

Here ya go.
Install Linux Mint Cinnamon.
Forget windows 10!


P.S.
I managed to compile trunk with msys2.
The effort was enough to make me switch operating system!

This, im currently working on building a publicly available compile farm so setting up different env templates and so far linux distros take me about 45 mins from getting virtual box downloaded, setting up the vm and installing linux and getting the build-dep installed and another 5 to build OpenTTD


Sent from my iPhone using Tapatalk

[JGR]

Thanks much appreciated!

Having IsTunnelInWay be O(N) in the total number of tunnels seems potentially problematic, but that could be a non-issue in practice.

I will fix this with something like : z > 0 ? true : false

EDIT: if (t->is_chunnel > not_allowed)
0 > 0 or 0 > 1 == false, so I think that part stands.
Question for me here is more "what is faster".
The old way or my way or maybe a combination of the two.
But that I will investigate later.

There are still some corner cases that need to be fixed, so I am back at the drawing board.
I need to get this beast under control.

Code: Select all

t->is_chunnel > not_allowed

Given that these are both bools, specifying it as

Code: Select all

t->is_chunnel && !not_allowed

is probably simpler to understand, or perhaps inverting/renaming the argument and using something like the following?

Code: Select all

t->is_chunnel && chunnel_allowed

On IsTunnelInWay being O(N), I was more thinking that as the tunnel pool contents is not contiguous, you'll be doing a lot of pointer-chasing.
Given that you're effectively filtering for tunnels which axially intersect the tile of interest with the

Code: Select all

TileX(t->tile_n) != x
TileY(t->tile_n) != y

tests, you could remove the need to iterate the entire tunnel pool by indexing X and Y tunnels by their axial coordinate (hash table, btree, sorted vector or something like that perhaps).
Given that read frequency is >>> write frequency, and it would not seem too difficult to make an index cache friendly, that seems like it ought be a worthwhile optimisation to consider.

Question for me here is more "what is faster".
The old way or my way or maybe a combination of the two.

The old way is not especially efficient, although it does have the arguable advantage of better memory access patterns.

Another thought on the m2 limitation, if 16 bits is not enough or we later need some bits for something else, a possibility is to have a sentinel value meaning: look up tunnel index in a hash table using the tile index, or some such.

[HackaLittleBit]

Attached two more patches in a zip file.
You should be able to apply them on top of the previous.
patch 1 fixes chunnels passing tunnels.
patch 2 adds tile description.
I hope I did not forget anything.

Code: Select all

t->is_chunnel > not_allowed

Given that these are both bools, specifying it as

Code: Select all

t->is_chunnel && !not_allowed

is probably simpler to understand, or perhaps inverting/renaming the argument and using something like the following?

Code: Select all

t->is_chunnel && chunnel_allowed

Sorry about that, but I can't kick the habbit.(assembler)

One thing which I have some slight concern with though is the hard limit of 64k tunnels.

64000 comes from FFFF hex m2 has 16 bits.
I tested with ottdcoop savegame 312 256 x 256 map size with 163 tunnels.
==> map of 4098 x 4098 is 256 x 163 = 34304 tunnels I think 64000 is ok.

If not, other solutions have to be found.


IsTunnelInWay : Yup a faster solution may be possible.
In due time I will look into that.
My priority was to get the thing working.
After these two last patches it is complete. I think


Again thanks for looking at it.

[JGR]

I propose to add patch for money transfer via company instead of player: viewtopic.php?f=33&t=63899

This patch is very useful in case of infrastructure sharing provided by this patch pack.
Assume, that you are using someone tracks and stations, and he have temporary money problems, but player is not online.
You cannot save other company by sending some money, nor purchase it due to high company value, as result their track and stations would gone together with your trains, important links will be broken and your company also start to sink.

I've merged this with some modifications, it'll be in the next release.

Attached two more patches in a zip file.
You should be able to apply them on top of the previous.
patch 1 fixes chunnels passing tunnels.
patch 2 adds tile description.
I hope I did not forget anything.

Thanks for these.

I've created a branch on github named 'chunnel' with your patches and some minor changes of my own on top.

64000 comes from FFFF hex m2 has 16 bits.
I tested with ottdcoop savegame 312 256 x 256 map size with 163 tunnels.
==> map of 4098 x 4098 is 256 x 163 = 34304 tunnels I think 64000 is ok.

256 x 163 is 41728 surely?
My patchpack supports map sizes up to 2^28 (16k x 16k square to 256 x 1M), this is more where I suspect that this may be problematic.
This should be straightforward to sort out at my end though.


I'm a bit on the fence about the change to allow tunnels crossing chunnels after the ramp distance, that seems a bit exploitable to get around the prohibition of crossing tunnels, it also looks a bit weird when the vehicles visible in tunnels transparency setting is enabled.
Chunnels crossing other chunnels at right-angles seems like it ought to not be allowed for the same reason.
Lastly, chunnels cost the same as ordinary tunnels, some kind of extra cost in terms of construction or maintenance seems prudent.

[HackaLittleBit]

Lastly, chunnels cost the same as ordinary tunnels, some kind of extra cost in terms of construction or maintenance seems prudent.

I'm a bit on the fence about the change to allow tunnels crossing chunnels after the ramp distance, that seems a bit exploitable to get around the prohibition of crossing tunnels

So I also think the cost should go up.
Lets make it double?
You have to imagine chunnel going below level 0.
-1 or even -2 when they cross each other. (Think of London tube)
I also can make a minimum length!
That would make sense also.

it also looks a bit weird when the vehicles visible in tunnels transparency setting is enabled.

Ehhh I don't know what to say here.

Chunnels crossing other chunnels at right-angles seems like it ought to not be allowed for the same reason

It was done in order to avoid sabotage.
See Planetmakers post here

Lastly a good game for testing is no 19 in the Pro Zone of openttdcoop.
1197 tunnels on a map 512 x 512

[JGR]

So I also think the cost should go up.
Lets make it double?

That's implemented now.

it also looks a bit weird when the vehicles visible in tunnels transparency setting is enabled.

Ehhh I don't know what to say here.

I've made some changes to vehicle positioning within chunnels to get round this, and prevented building chunnels which intersect with other chunnels (unless crossing tunnels is enabled).

I've removed the 64k tunnel limit and made some changes which should speed up tunnel intersection detection, though I haven't actually measured anything yet.
I've updated the branch on github.


Also after doing a bit of testing, it seems slightly odd that plopping a single canal tile down is enough to trigger chunnel creation, and that land between the chunnel entrances and the water can be lowered after construction, but not before.
The chunnel concept is arguably useful without having any water at all, though I'm reluctant to overcomplicate things too much.
That said I also don't want to encourage people to start using dummy canal tiles to bypass the crossing tunnels restriction.

[SimYouLater]

So I also think the cost should go up.
Lets make it double?

That's implemented now.

it also looks a bit weird when the vehicles visible in tunnels transparency setting is enabled.

Ehhh I don't know what to say here.

I've made some changes to vehicle positioning within chunnels to get round this, and prevented building chunnels which intersect with other chunnels (unless crossing tunnels is enabled).

I've removed the 64k tunnel limit and made some changes which should speed up tunnel intersection detection, though I haven't actually measured anything yet.
I've updated the branch on github.


Also after doing a bit of testing, it seems slightly odd that plopping a single canal tile down is enough to trigger chunnel creation, and that land between the chunnel entrances and the water can be lowered after construction, but not before.
The chunnel concept is arguably useful without having any water at all, though I'm reluctant to overcomplicate things too much.
That said I also don't want to encourage people to start using dummy canal tiles to bypass the crossing tunnels restriction.

I've never understood the crossing tunnels restriction. If one tunnel is at height level 1 and another at height level 2 then that should not count as "crossing", but IIRC it does.

[JGR]

I've never understood the crossing tunnels restriction. If one tunnel is at height level 1 and another at height level 2 then that should not count as "crossing", but IIRC it does.

Crossing tunnels does refer to tunnels intersecting each other at the same height level. The setting is in the cheats window.

[SimYouLater]

I've never understood the crossing tunnels restriction. If one tunnel is at height level 1 and another at height level 2 then that should not count as "crossing", but IIRC it does.

Crossing tunnels does refer to tunnels intersecting each other at the same height level. The setting is in the cheats window.

Oh, well in that case I guess I've got nothing to complain about. As for chunnels, I think you should make two different chunnels able to cross each other, but regular tunnels remain as they are.

[HackaLittleBit]

Also after doing a bit of testing, it seems slightly odd that plopping a single canal tile down is enough to trigger chunnel creation, and that land between the chunnel entrances and the water can be lowered after construction, but not before.
The chunnel concept is arguably useful without having any water at all, though I'm reluctant to overcomplicate things too much.
That said I also don't want to encourage people to start using dummy canal tiles to bypass the crossing tunnels restriction.

Maybe the return value of IsTunnelInWay should be different.
Maybe the tileindex of the tunnel tile.
That way we could test if it was a tunnel or a chunnel.
So when player want to do terraforming on top of chunnel she or he gets a warning! "Chunnel in the way"
Holland is full with this kind of tunnels.
See attachment for an example. (kmz in google earth).
Maybe also we should make a difference between the axis
DIAGDIR_SW 2 down DIAGDIR_SE 1 down.
For the rest things look good but I need some time to study your work.

Big thanks

[HackaLittleBit]

I think I understand what are your doubts.
See following situation.
Old situation


With the following line it will be solved.

Code: Select all

		if (!_cheats.crossing_tunnels.value && IsTunnelInWay(end_tile, start_z, (tiles + 1 < 4 || (is_chunnel && head_tiles < 4) ? false : true))) {

New situation


Right?

[HackaLittleBit]

You have to imagine chunnel going below level 0.
-1 or even -2 when they cross each other. (Think of London tube)

Please try this code.
Level -1 trains wil bang agains the hull of a ship, so we have to go deeper.
-2 one axis
-3 the other axis

Code: Select all

	int north_coord, south_coord, pos_coord;
	Slope slope = SLOPE_FLAT;
	int delta;
	if (t->tile_s - t->tile_n > MapMaxX()) {
		// tunnel extends along Y axis (DIAGDIR_SE from north end), has same X values
		north_coord = TileY(t->tile_n);
		south_coord = TileY(t->tile_s);
		pos_coord = TileY(pos_tile);

		if ((delta = pos_coord - north_coord) <= 3) {
			this->z_pos = TILE_HEIGHT * (delta == 3 ? -2 : -1);
			if (delta != 2) {
				slope = SLOPE_NW;
				SetBit(this->gv_flags, this->direction == DIR_NW ? GVF_GOINGUP_BIT : GVF_GOINGDOWN_BIT);
			}
		} else if ((delta = south_coord - pos_coord) <= 3) {
			this->z_pos = TILE_HEIGHT * (delta == 3 ? -2 : -1);
			if (delta != 2) {
				slope = SLOPE_ELEVATED ^ SLOPE_NW;
				SetBit(this->gv_flags, this->direction == DIR_NW ? GVF_GOINGDOWN_BIT : GVF_GOINGUP_BIT);
			}
		}
	} else {
		// tunnel extends along X axis (DIAGDIR_SW from north end), has same Y values
		north_coord = TileX(t->tile_n);
		south_coord = TileX(t->tile_s);
		pos_coord = TileX(pos_tile);
		
		if ((delta = pos_coord - north_coord) <= 3) {
			this->z_pos = TILE_HEIGHT * (delta == 3 ? -3 : (delta == 2 ? -2 : -1));
			slope = SLOPE_NE;
			SetBit(this->gv_flags, this->direction == DIR_NE ? GVF_GOINGUP_BIT : GVF_GOINGDOWN_BIT);
		} else if ((delta = south_coord - pos_coord) <= 3) {
			this->z_pos = TILE_HEIGHT * (delta == 3 ? -3 : (delta == 2 ? -2 : -1));
			slope = SLOPE_ELEVATED ^ SLOPE_NE;
			SetBit(this->gv_flags, this->direction == DIR_NE ? GVF_GOINGDOWN_BIT : GVF_GOINGUP_BIT);
		}
	}

EDIT
And.......
The whole thing is legal now.
The tunnels don't cross each other.
EDIT2
Code was a bit adjusted. less vars

[JGR]

I can see where you're coming from, and the code looks reasonable, but more fundamentally I just don't see why crossing tunnels, physically sensibly or otherwise, needs to be supported.

What is the use case other than making it easier for players to cheat without using the cheats window?
I don't want to add features which multiplayer server admins will just turn off as they're too exploitable.

Allowing chunnels to cross ordinary tunnels already seems quite generous.