New map features

[Toni Babelony]

I know this isn't of any practical help, but, if I may, I'd like to express my gratitude and applaud the development of this patch. This is something OpenTTD has missed out on a lot!

Please, do keep up the good work! If this gets into the stable releases (nightlies) of OpenTTD, it will completely change the gaming experience and ways of thought of all players.

[cirdan]

It may not be practical help, but it helps anyway, so thank you.

I am beginning to feel quite confident about the patch, not in the sense that it is bug-free--most likely it is not--but with the design choices. Specifically, after the last round of code restructuring, I no longer have any reservations about custom rail bridgeheads and signal handling, which were my main concerns. So, while I am still tweaking the patch sequence, I expect it to remain fairly stable unless bugs show up.

[cirdan]

Rebased against current trunk.

[JacobD88]

Thank-you for your continued support of this set of patches Cirdan, your work is greatly appreciated

[HackaLittleBit]

Just a quick note cirdan

get only 1 compile error in mingw

Code: Select all

d:/development/ottd/vtile/src/train_cmd.cpp: In function 'void FreeTrainTrackRes
ervation(const Train*)':
d:/development/ottd/vtile/src/train_cmd.cpp:2236:10: error: 'ft' cannot appear i
n a constant-expression
d:/development/ottd/vtile/src/train_cmd.cpp:2236:13: error: '.' cannot appear in
 a constant-expression
d:/development/ottd/vtile/src/train_cmd.cpp:2243:10: error: 'ft' cannot appear i
n a constant-expression
d:/development/ottd/vtile/src/train_cmd.cpp:2243:13: error: '.' cannot appear in
 a constant-expression
make[1]: *** [train_cmd.o] Error 1

fixed it with

Code: Select all

		if (first) {
			if (ft.m_flag == ft.TF_BRIDGE) {
				if (ft.m_old.InWormhole()) {
					assert(IsRailBridgeTile(ft.m_old.wormhole));
				} else {
					assert(IsRailBridgeTile(ft.m_old.tile));
				}
			} else if (ft.m_flag == ft.TF_TUNNEL) {
				if (ft.m_old.InWormhole()) {
					assert(IsTunnelTile(ft.m_old.wormhole));
				} else {
					assert(IsTunnelTile(ft.m_old.tile));
				}
			}
			break;
		}

[cirdan]

How strange... The fix is obviously correct, but I wonder why I do not get that error and you do. What compiler are you using?

[HackaLittleBit]

Code: Select all

hans@H ~
$ gcc -v
Using built-in specs.
COLLECT_GCC=d:\MinGW\bin\gcc.exe
COLLECT_LTO_WRAPPER=d:/mingw/bin/../libexec/gcc/mingw32/4.5.0/lto-wrapper.exe
Target: mingw32
Configured with: ../gcc-4.5.0/configure --enable-languages=c,c++,ada,fortran,obj
c,obj-c++ --disable-sjlj-exceptions --with-dwarf2 --enable-shared --enable-libgo
mp --disable-win32-registry --enable-libstdcxx-debug --enable-version-specific-r
untime-libs --disable-werror --build=mingw32 --prefix=/mingw
Thread model: win32
gcc version 4.5.0 (GCC)

hans@H ~
$

Maybe time for an upgrade

[cirdan]

Thank you. I can successfully reproduce the error with gcc versions 4.5.0, 4.5.1 and 4.5.2, but not with 4.6.1 or 4.6.3. Therefore, I guess that gcc was stricter in the 4.5 branch than in 4.6. I will include your fix in my patch.

[HackaLittleBit]

zip file with vc++ build log plus small perl file to convert from git to svn (more or less).
disclaimer: use perl file at your own responsibility.

EDIT: Cirdan I did not do clean build so hold on pls.
EDIT2: that build log stands !!
EDIT3: warning

Code: Select all

d:/development/ottd/vtile/src/elrail.cpp: In function 'void DrawCatenaryRailway(
const TileInfo*)':
d:/development/ottd/vtile/src/elrail.cpp:339:26: warning: 'sprite_halftile' may
be used uninitialized in this function

[cirdan]

What kind of compiler outputs a log in HTML?

Anyway, thanks for your report. This is my analysis of the log.

• The error at newgrf_engine:810 looks like a missing cast. C++ is extremely picky about casts, even between simple scalar types, and it seems that your compiler is pickier than mine. I will try to fix it by adding an explicit cast.
• The errors at map_sl.cpp probably mean that your compiler chokes on the construction

  Code: Select all

  _mc[t] = { 0, 0, 0, 0, 0, 0, 0 };

  which GCC accepts. Compound literals are valid C99 code, but using them in C++ may be a GCC extension, so I will switch to an alternative approach.
• I am puzzled at the swarm of "performance warnings" regarding rail_map.h:429, in which a scalar value is returned as a boolean. This is valid code (otherwise the message would be an error), and I cannot see how it could impact performance, since it has very well-defined semantics and it is precisely what is wanted.
• The warning about sprite_halftile is harmless. The variable is never used uninitialised, but the compiler is not clever enough to see this.

I will try to post a new version of the branch with the above fixes soonish.

[HackaLittleBit]

rail_map.h:429

to me *SignalByte is pointer ==> 4bytes, you need bool == byte. so extra instruction.
EDIT: Just a hunch because you declared it as byte, I can't see the instructions.
EDIT2: return (*SignalByte(tile, TrackdirToTrack(trackdir)) & (IsSignalHighBit(trackdir) ? 0x10 : 0x08)) != 0;


_mc[t] = { 0, 0, 0, 0, 0, 0, 0 }; see this

[cirdan]

Here is a new version of the patch, addressing all of the recent concerns.

rail_map.h:429

EDIT2: return (*SignalByte(tile, TrackdirToTrack(trackdir)) & (IsSignalHighBit(trackdir) ? 0x10 : 0x08)) != 0;

Exactly. An integer type, when used as a boolean, should behave as true iff nonzero. It may be a matter of coding style, but "performance warning"? The compiler should just generate the same code in either case.

_mc[t] = { 0, 0, 0, 0, 0, 0, 0 }; see this

Thanks for the link; I have changed the code not to use a compound literal.

[HackaLittleBit]

A bug for the weekend.

bg.png (32.95 KiB) Viewed 6123 times

Train reverse and will not cross bridge.
You did not change line 429 in rail_map.h (!= 0)

Edit1:
Try loading ottdcoop endgames.
192 and 193
will trigger asserts.

[HackaLittleBit]

It is difficult to reproduce
so here is savegame.

Edit1:

One more bug. see pic
Just a hunch cirdan.
I need time to familiarize with the code.
But it smells that somewhere you are relying on DIAGDIR
Be careful with that one.

[cirdan]

Thank you for your bug reports.

The bug triggered by the openttdcoop games was quite silly, in fact. It seems that I was being a bit overzealous with my assertions; the code itself was fine, only the assertions were wrong.

With respect to the bug in your last post, I am unable to reproduce it. Is there something specific that I must do?

[HackaLittleBit]

With respect to the bug in your last post, I am unable to reproduce it. Is there something specific that I must do?

I installed hg and applied your patch to 100% clean trunk.
It is working now so I can't reproduce that bug anymore.
Sorry for false alarm

About openttdcoop game 192

when loading game tunnel reservation is changed
Don't know if this is intended behaviour but can be reason for crash.
see photo

[Eddi]

when loading game tunnel reservation is changed
Don't know if this is intended behaviour but can be reason for crash.
see photo

that looks like intended behaviour to me. if tunnels are now individual tiles with individual reservation state (this is in any case necessary for proper signals on bridges/in tunnels), then there is no reason to keep the tunnel entrance reserved. it only needs making sure that the pathfinder will correctly return a path collision for the next train.

[HackaLittleBit]

This game crashes on 2 of june
cirdan maybe it is not yet a good idea to test on very complex games.

[cirdan]

when loading game tunnel reservation is changed
Don't know if this is intended behaviour but can be reason for crash.
see photo

You are a keen observer... That is intended behaviour indeed. In trunk, bridges and tunnels are reserved as a whole, since they effectively behave like one long piece of track. With my patch, however, bridgeheads can have signals, or junctions, or two parallel pieces of track, so each bridgehead must be dealt with separately from the rest of the bridge. Tunnels receive the same treatment because it was easier to change both of them at the same time and because it will make it easier to add signals on tunnelheads.

This game crashes on 2 of june
cirdan maybe it is not yet a good idea to test on very complex games.

I think that testing complex games is precisely what it is needed. I consider the patch to be mostly finished in what it does, so any kind of testing is appreciated.

I will have a look at your savegame, although I will not be able to until next week.

[cirdan]

Bug squashed. In the end, it was a small oversight on my part. Thanks for the report!