YAPP - Yet Another PBS Patch (now in trunk!)

[Mchl]

I think I don;t follow you...
How do you imagine game with YAPP signals compatible with unpatched trunk? Should YAPP signals revert to standard signals? Or perhaps they should disappear?

[AndersI]

Actually I wasn't thinking (very much) of landscape data, but mostly of the "save settings for this patch". Taking care of infrastructure modifications is 'a bit' more difficult...

[Mchl]

OK. This makes more sense.

[Kiloman]

I just gave this a try with the latest nightly (13400) and it works great. A little fuzz in settings_type.h and openttd.cpp, and the project file is broken, but other than that it applied just fine. I'm loving the new options. I have a request and a few bug reports:

Request: Show track reservations in another color besides PALETTE_CRASH. The point (it seems to me) is to make the reserved tracks stand out, which they don't really do when changed to a slightly darker gray. I did "sed -i -e s/PALETTE_CRASH/PALETTE_SEL_TILE_RED/ ../yapp-v7.1_r13377-red.patch" before applying, and they stand out very well

Bug 1: Inclined tiles are not highlighted. Not a huge problem, but only flat tiles, stations, and depots are highlighted. Tiles on an incline remain unmarked.

Bug 2: Changing a train's path after the route is reserved leaves orphan reserved segments. If a section of track is reserved by a train, and the path is changed by deleting or replacing track, the new path is not reserved, and the old path's reservation is never cleared. The tiles will stay reserved until I force a train across them. You can see this effect by deleting a section of track before the train crosses it, forcing it to turn around and exit the reserved section... or by diverting it to another section of track before it passes through all of the reservation.

The obvious answer to bug 2 is "don't delete track while it's reserved", but the fact is that it's going to happen... and without a visible reservation overlay, the user would never know why their train is getting stuck, or what to do about it. The proper fix (IMHO) would be to keep track of the reserved tiles, and free them if the path changes for any reason.

Thanks for some really fun features; here's to hoping it makes it into trunk some time soon!

[Michi_cc]

Request: Show track reservations in another color besides PALETTE_CRASH. The point (it seems to me) is to make the reserved tracks stand out, which they don't really do when changed to a slightly darker gray. I did "sed -i -e s/PALETTE_CRASH/PALETTE_SEL_TILE_RED/ ../yapp-v7.1_r13377-red.patch" before applying, and they stand out very well

Bug 1: Inclined tiles are not highlighted. Not a huge problem, but only flat tiles, stations, and depots are highlighted. Tiles on an incline remain unmarked.

Inclined tracks are not highlighted because OpenTTD misses appropriate sprites for them. And I'm not sure if everybody wants their reserved tiles to stick out like a sore thumb

The obvious answer to bug 2 is "don't delete track while it's reserved", but the fact is that it's going to happen... and without a visible reservation overlay, the user would never know why their train is getting stuck, or what to do about it. The proper fix (IMHO) would be to keep track of the reserved tiles, and free them if the path changes for any reason.

Reserved tracks are protected from removing when using the remove rail tool. They are not protected from the bulldozer tool in case a bug in YAPP makes it neccessary to manually remove such tiles. When YAPP is sufficently bug-free, the protection should probably be enabled for the bulldozer as well.

-- Michael Lutz

[Kiloman]

Inclined tracks are not highlighted because OpenTTD misses appropriate sprites for them.

Is this true with just the crash palette, or what? I seem to be able to highlight inclined sections just fine when I use the rail removal tool...

And I'm not sure if everybody wants their reserved tiles to stick out like a sore thumb

Just a suggestion. I initially had trouble telling that it was highlighted at all; hence the change to a more obvious palette

Reserved tracks are protected from removing when using the remove rail tool. They are not protected from the bulldozer tool in case a bug in YAPP makes it neccessary to manually remove such tiles. When YAPP is sufficently bug-free, the protection should probably be enabled for the bulldozer as well.

That explains it - I default to the demolish tool over the remove tool. Making reserved tracks protected seems a little counter-intuitive, especially if reservation highlighting defaults to off and is advertised as a debugging option. The pathfinders seem to handle track removal properly, would it be too much trouble to have YAPP do likewise?

[planetmaker]

Reserved tracks are protected from removing when using the remove rail tool. They are not protected from the bulldozer tool in case a bug in YAPP makes it neccessary to manually remove such tiles. When YAPP is sufficently bug-free, the protection should probably be enabled for the bulldozer as well.

suggestion: make that a(nother) patch option

That explains it - I default to the demolish tool over the remove tool. Making reserved tracks protected seems a little counter-intuitive, especially if reservation highlighting defaults to off and is advertised as a debugging option. The pathfinders seem to handle track removal properly, would it be too much trouble to have YAPP do likewise?

If I understand it correctly (I may be wrong here!), tthere're fundamental differences as of when the pathfinder is called, though. You're basically asking for a steep rise in computations or at least checks and thus a decrease in performance.

[Kiloman]

suggestion: make that a(nother) patch option

The answer is always YAPO

If I understand it correctly (I may be wrong here!), tthere're fundamental differences as of when the pathfinder is called, though. You're basically asking for a steep rise in computations or at least checks and thus a decrease in performance.

I should take a look at the code before I go talking about crazy things... but from the brief glance I had at it when poking at the highlight color, it looks like it just goes through and tags the track segments as reserved during the signal entry loop, and then marks them as free whenever they're crossed by a train. Would it be too much work to have the train keep a list of reserved segments, do some sort of check when the pathfinder is called to see if the reserved path is still valid, and free them if not?

[Michi_cc]

Thanks to r13469 OpenTTD now has the neccessary sprites to highlight sloped tracks. This means new version for you!

Changes:

• Highlighting of reserved tracks on slopes.
• 8.1: Highlighting of reserved tracks on steep slopes.
• 8.1: When 90 degree turns were forbidden, a train could free the reservation of another train.
• 8.1: Fixed an assert that could happen when forcing a train over an already present reservation.
• 8.1: Extending a path reservation could ignore compatible railtypes.
• 8.1: All reserved tracks can now be freely removed. Trains are automatically rerouted.
• 8.2: Fixed a bug when removing a reserved track if the train was just leaving a station.
• 8.2: A value of 255 for pf.path_backoff_interval will disable train look-ahead, freeing a little cpu time if you don't use pbs signals.
• 8.3: In certain circumstances a train could inadvertedly free the reservation of another train in a station.
• 8.3: The vehicle window was sometimes not redrawn properly.
• 8.4: Self-crossing paths would lead to not properly freed reserved path fragments.

-- Michael Lutz

[pshemko]

Hi,

In my opinion it should be option 1 (with ability to remove it using a bulldozer). The pop-up message should be enough to explain that the track is reserved and still you can remove it if you really want to. If the track gets forcefully removed - I guess the train should just attempt find a path towards destination, if a reservation can't be made - it should continue regardless. This is exactly the same as with normal signals, if you remove a signal that was stopping train from crashing into another train - it's the player's own fault.

And thank you for great job on this patch

[Ammler]

#openttdcoop public server testing **CLOSED**


our final statement: 98% (stable)


IRC: [url=irc://irc.oftc.net/openttdcoop]#openttdcoop[/url] (server password)
Blog: http://www.openttdcoop.org/blog/2008/06 ... pbs-patch/

Binaries

OTTD-linux-i686-r13501M.tar.bz2

(2.43 MiB) Downloaded 296 times

OTTD-win32-r13501M.exe.zip

(2.16 MiB) Downloaded 542 times

OSX: only intel / universal (ppc)
Linux 64Bit: http://www.tt-forums.net/viewtopic.php?p=700099#p700099

Asserts
src/pbs.cpp:137: void UnreserveRailTrack(TileIndex, Track): Assertion `(GetTileTrackStatus(tile, TRANSPORT_RAIL, 0) & TrakToTrackBits(t)) != 0' failed.
src/train_cmd.cpp void FreeTrainTrackReservation(const Vehicle*, TileIndex, Trackdir): Assertion `IsFrontEngine(v)' failed. (combination with NewGRF)
src/train_cmd.cpp void TrainController(Vehicle*, Vehicle*, bool): Assertion `chosen_track & bits' failed. (details / 2.example)
src/yapf/yapf_node_rail.hpp:194: failed assertion `KillFirstBit(ft.m_new_td_bits) == TRACKDIR_BIT_NONE' (details)

Glitches
Halftiles seems not to be supported. (details)
Waiting for ever...

Suggestions
I miss a patch setting to disable YAPP at all, well as long as YAPP is only a patch, it's ok, but who knows, it could go to trunk...

Saves
http://ps.openttdcoop.org/dev/save/yapp_test_1.sav (needs our GRFPack 7.0)
http://ps.openttdcoop.org/dev/save/yapp_test_2.sav (no NewGRFs used)

[FooBar]

Additionally, could you all say how removing reserved tracks should work in your opinion.

I think the way it is now is just fine. It prevents you from doing stupid things in the first place, but if you really want to, you can still grab the old dynamite in order to get things done. This way it's relatively safe for the inexperienced users, but still versatile enough for powerusers.

A whole new option would be a dialog asking if you want to continue when destroying a reserved path: "You are about to remove a reserved path. Removing a reserved path may result in train crashes. Continue anyways" with an OK and Cancel button. The biggest problem here would probably be that the game doesn't really support such dialogs in general, but there's one when quitting a game.

[Tautrimas]

I have an idea: show error, that track is reserved and hint player to press ctrl for forced removal. I saw several patches that use ctrl for different actions.

[Roujin]

CTRL is already used to toggle removing...

[Tautrimas]

I mean't demolishing tool. But still, for forced rail removing ctrl+shift could be used.

[Roujin]

shift+click is reserved for cost estimation - so ctrl+shift+click is reserved for estimating the cost of what ctrl+click does

[Tautrimas]

Ok, haven't played for a week and totaly forgot everything...

Actually, I don't really see any bad points in the current implementation. Signals existed for a long time and everybody knows that removing them when trains are near is not that good.

[Roujin]

True... say, what is currently the error message that is displayed if you try to remove a reserved tile? "Train in the way"? If so, you could change it to something else, since a train is not really in the way... maybe "Track is reserved by a train" or so..

If it's already like that, I have nothing else to suggest..

[Eddi]

tracks with a (strong) reservation should under no circumstances be removable (possibly remove reservation if a train has been manually stopped, and not let it start unless a new reservation can be made)

[planetmaker]

For those who need a MacOS build, here it is.