New map features

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

User avatar
Toni Babelony
Tycoon
Tycoon
Posts: 1389
Joined: 07 Jul 2006 09:34
Skype: toni_babelony
Location: Sagamihara-shi, Japan
Contact:

Re: New map features

Post by 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. :bow:
Retired JapanSet developer and creator of TIAS.

User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: New map features

Post by 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.
Attachments
nma-r24070.diff.gz
Simple diff
(280.17 KiB) Downloaded 91 times
nma-r24070.patch.gz
Branch (361 patches)
(592.32 KiB) Downloaded 95 times

User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: New map features

Post by cirdan »

Rebased against current trunk.
Attachments
nma-r24262.diff.gz
Simple diff
(281.49 KiB) Downloaded 97 times
nma-r24262.patch.gz
Branch (370 patches)
(598.19 KiB) Downloaded 95 times

User avatar
JacobD88
Chief Executive
Chief Executive
Posts: 675
Joined: 16 Aug 2008 17:51
Location: Long Eaton, Nottinghamshire. UK
Contact:

Re: New map features

Post by JacobD88 »

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

User avatar
HackaLittleBit
Director
Director
Posts: 550
Joined: 10 Dec 2008 16:08
Location: tile 0x0000

Re: New map features

Post by 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;
		}


User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: New map features

Post by 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?

User avatar
HackaLittleBit
Director
Director
Posts: 550
Joined: 10 Dec 2008 16:08
Location: tile 0x0000

Re: New map features

Post by 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 :roll:

User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: New map features

Post by 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.

User avatar
HackaLittleBit
Director
Director
Posts: 550
Joined: 10 Dec 2008 16:08
Location: tile 0x0000

Re: New map features

Post by 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
Attachments
cbh.zip
(5.73 KiB) Downloaded 95 times

User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: New map features

Post by cirdan »

What kind of compiler outputs a log in HTML? :shock:

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.

User avatar
HackaLittleBit
Director
Director
Posts: 550
Joined: 10 Dec 2008 16:08
Location: tile 0x0000

Re: New map features

Post by 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

User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: New map features

Post by cirdan »

Here is a new version of the patch, addressing all of the recent concerns.
HackaLittleBit wrote: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.
HackaLittleBit wrote:_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.
Attachments
nma-r24329.diff.gz
Simple diff
(284.23 KiB) Downloaded 80 times
nma-r24329.patch.gz
Branch (376 patches)
(602.71 KiB) Downloaded 81 times

User avatar
HackaLittleBit
Director
Director
Posts: 550
Joined: 10 Dec 2008 16:08
Location: tile 0x0000

Re: New map features

Post by HackaLittleBit »

A bug for the weekend. :wink:
bg.png
bg.png (32.95 KiB) Viewed 3995 times
Train reverse and will not cross bridge.
You did not change line 429 in rail_map.h (!= 0) :roll:

Edit1:
Try loading ottdcoop endgames.
192 and 193
will trigger asserts.
Attachments
458.png
458.png (6.36 KiB) Viewed 3985 times
459.png
459.png (6.48 KiB) Viewed 3985 times

User avatar
HackaLittleBit
Director
Director
Posts: 550
Joined: 10 Dec 2008 16:08
Location: tile 0x0000

Re: New map features

Post by 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. :!:
Attachments
Spijkenwater Transport, 15th Jan 1953.sav
(6.82 KiB) Downloaded 79 times
bg2.png
bg2.png (58.65 KiB) Viewed 3928 times

User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: New map features

Post by 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. :roll:

With respect to the bug in your last post, I am unable to reproduce it. Is there something specific that I must do?
Attachments
nma-r24346.diff.gz
Simple diff
(286.09 KiB) Downloaded 76 times
nma-r24346.patch.gz
Branch (379 patches)
(608.09 KiB) Downloaded 78 times

User avatar
HackaLittleBit
Director
Director
Posts: 550
Joined: 10 Dec 2008 16:08
Location: tile 0x0000

Re: New map features

Post by HackaLittleBit »

cirdan wrote: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 :oops:

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
Attachments
pa_tr.png
pa_tr.png (291.26 KiB) Viewed 3724 times

Eddi
Tycoon
Tycoon
Posts: 7447
Joined: 17 Jan 2007 00:14

Re: New map features

Post by Eddi »

HackaLittleBit wrote: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.
You might not exactly be interested in Ferion, but if you are, have fun :)

User avatar
HackaLittleBit
Director
Director
Posts: 550
Joined: 10 Dec 2008 16:08
Location: tile 0x0000

Re: New map features

Post by HackaLittleBit »

This game crashes on 2 of june :)
cirdan maybe it is not yet a good idea to test on very complex games.
Attachments
Groenhalen Transport, 14th Mar 1950.sav
(10.03 KiB) Downloaded 83 times

User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: New map features

Post by cirdan »

HackaLittleBit wrote: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.
HackaLittleBit wrote: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.

User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: New map features

Post by cirdan »

Bug squashed. In the end, it was a small oversight on my part. Thanks for the report!
Attachments
nma-r24358.diff.gz
Simple diff
(286.08 KiB) Downloaded 89 times
nma-r24358.patch.gz
Branch (379 patches)
(608.17 KiB) Downloaded 93 times

Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 11 guests