Page 35 of 51
Re: YAPP - Yet Another PBS Patch (New version 7 out!)
Posted: 05 Jun 2008 20:18
by 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?
Re: YAPP - Yet Another PBS Patch (New version 7 out!)
Posted: 05 Jun 2008 20:59
by 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...
Re: YAPP - Yet Another PBS Patch (New version 7 out!)
Posted: 05 Jun 2008 23:08
by Mchl
OK. This makes more sense.
Re: YAPP - Yet Another PBS Patch (New version 7 out!)
Posted: 08 Jun 2008 03:22
by 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!
Re: YAPP - Yet Another PBS Patch (New version 7 out!)
Posted: 08 Jun 2008 12:30
by Michi_cc
Kiloman wrote:
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
Kiloman wrote:
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
Re: YAPP - Yet Another PBS Patch (New version 7 out!)
Posted: 09 Jun 2008 03:59
by Kiloman
Michi_cc wrote:
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...
Michi_cc wrote:
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
Michi_cc wrote:
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?
Re: YAPP - Yet Another PBS Patch (New version 7 out!)
Posted: 09 Jun 2008 05:54
by planetmaker
Michi_cc wrote:
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
Kiloman wrote: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.
Re: YAPP - Yet Another PBS Patch (New version 7 out!)
Posted: 09 Jun 2008 06:32
by Kiloman
planetmaker wrote:
suggestion: make that a(nother) patch option
The answer is
always YAPO
planetmaker wrote:
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?
Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 12 Jun 2008 21:55
by 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
Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 12 Jun 2008 22:21
by 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

Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 13 Jun 2008 00:00
by 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
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)
Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 13 Jun 2008 10:25
by FooBar
Michi_cc wrote: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.
Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 13 Jun 2008 10:55
by 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.
Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 13 Jun 2008 11:07
by Roujin
CTRL is already used to toggle removing...

Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 13 Jun 2008 11:19
by Tautrimas
I mean't demolishing tool. But still, for forced rail removing ctrl+shift could be used.
Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 13 Jun 2008 11:21
by Roujin
shift+click is reserved for cost estimation - so ctrl+shift+click is reserved for estimating the cost of what ctrl+click does

Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 13 Jun 2008 11:30
by 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.
Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 13 Jun 2008 11:36
by 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..

Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 13 Jun 2008 13:25
by 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)
Re: YAPP - Yet Another PBS Patch (New version 8 out!)
Posted: 13 Jun 2008 22:10
by planetmaker
For those who need a MacOS build, here it is.