While looking at makie's request in the priorities thread — extending the way
tool so a drag can bridge a river or pass through a hill — I found that most of
the engine support already exists, but the automatic bridge path has been
unreachable since January 2025.
I have attached two independent patches.
The first is a one-character regression fix and can be reviewed on its own. The
second adds a way for players to enable the existing automatic bridge and tunnel
mode.
The regressionway_builder_t::check_for_bridge() searches for a bridge when the next straight
way step is blocked. In wegbauer.cc:
if(!ziel.is_contained(end) && bridge_builder_t::check_start_tile(player_builder, gr_end, ribi_type(-zv), bridge_desc)) {
... append the candidate ...
}
else {
break;
}
check_start_tile() returns an error string, with NULL meaning that the tile is
usable.
That contract is stated in brueckenbauer.h, and the other three callers in the
source tree all negate the result: twice in brueckenbauer.cc and once in
simtool.cc.
Here the result is not negated. A valid end tile therefore enters the else branch
and terminates the search, while only an invalid tile could reach the code that
appends the bridge candidate.
The branch is not merely unlikely. A few lines earlier, can_build_bridge() must
already have succeeded, and it calls check_start_tile() on the same end tile.
By the time the condition above is evaluated, the result is therefore already
known to be NULL. The condition is false and the append branch is unreachable.
The regression was introduced in commit 050f9fdaf, the bridge-builder rework
from 2025-01-24. It replaced:
can_place_ramp(...) // bool: true means usable
with:
check_start_tile(...) // const char*: NULL means usable
The condition retained its previous polarity even though the return convention
had changed.
I also verified the behaviour in a scenario test.
With the negation restored, dragging a road across a taxiway barrier on flat
ground produces:
road bridge-head barrier-spanned bridge-head road
Without the fix, the same drag builds nothing.
The taxiway crosses every row of the test map, so the route cannot go around it.
Bridging a taxiway is legal, while bridging a runway is not; that distinction is
already covered by test_way_bridge_build_above_runway.
A separate issue exposed by the fixAn automatically built bridge leaves some maintenance behind after removal.
In one test run:
empty world 0
barrier built 51 200
automatic road and bridge 72 400
whole row removed 52 800
barrier removed 1 600
The automatic structure adds 21 200 in maintenance but only removes 19 600. The
remaining 1 600 stays assigned to the player even though no object remains on
the map; I checked every object stack at every height.
A manually built bridge over the same span, with the same road length, adds and
removes maintenance cleanly. The discrepancy therefore appears specific to the
automatic construction path.
I have not attempted to fix this because the finance bookkeeping is separate
from the inverted condition, and I would rather report the observed behaviour
than guess at its cause.
The second patch: enabling the existing modetool_build_way_t already has an automatic_tunnel_and_bridges member, but there
was no interface path that could set it.
The second patch adds:
- a toolbar toggle;
- a local preference in env_t;
- an off-by-default entry in simuconf.tab;
- command and network propagation of the selected mode.
The mode is copied into the tool command during init() and is also serialised by
rdwr_custom_data(). A network command therefore executes using the value that
was sent, rather than the receiving client's local preference.
The patch also adds tool_t::local_preference_changed().
A SIMPLE_TOOL whose init() returns false does not reinitialise the previously
selected tool. Without a notification, changing the toggle would leave an
already selected way tool using its previous value until the player selected it
again.
The notification only updates local tool state. It sends no network command and
changes nothing in the world.
The default remains off. Automatically selecting a bridge can make a drag much
more expensive than an ordinary way, and the complete cost is shown in the
drag tooltip before construction.
VerificationThe project builds without new warnings.
The reference suite passes:
207/207
This is the original 201 tests plus 6 new tests.
Both patches were also applied to a clean checkout of r12114 and built from
scratch.
The positive automatic-bridge test is currently registered last in
all_tests.nut. This is intentional: until the maintenance residue is understood,
a later test could observe maintenance that it did not create. The comment next
to the registration documents this and says that the test should be moved once
the accounting issue is fixed.
Current test coverage is limited to:
- automatic bridges;
- road and rail.
The tunnel half has only been checked manually. The build was performed with
MinGW/g++; I have not verified the patches with MSVC.
QuestionThe polarity correction is self-contained and supported by both the documented
return contract and the regression test.
However, the mode it revives has been dormant since January 2025, and the
maintenance residue suggests that the full automatic path may need further work
before it is exposed to players.
Would you prefer to:
- merge the one-character regression fix now and investigate the maintenance
issue separately; or - leave the path disabled until both the accounting issue and the user-facing
toggle have been reviewed?
Thank you very much. I submitted the fix as r12115.
The toggle tool, I am not a fan of it. If some client as the same player uses this tool on the server, the other player gets it toggled too. I think it would be better to toggle it in env_t:: which is local for the active client. This way, it would be active for any player on the same client. And there is already a default_parameter for ways with automatic tunnel and bridges. This override is somewhat counter to this. Third, there is no way to specify what bridge and tunnel to use. I think that needs some more thought on the gui. Maybe a new window with way, bridges and tunnels as image comboboxes, allowing also for only tunnel or only bridges. Liek advance building tool with a waytype as parameter?
And finally, one needs to find the bookeepping bug. I guess the error could be in the partial booking of ramps.
I traced the maintenance residue and need to correct something I said earlier:
it is not specific to automatic bridges. A manually built bridge leaks in the
same way when it is built over an existing way. My earlier control used bare
ground, which hid the problem.
CauseWays created directly by the bridge builder are booked to no player, because
the bridge itself pays the maintenance. Removal follows the same rule: before
deleting a way on a bridge tile, its owner is cleared, so the way refunds
nothing.
The missing case is a way that already existed.
build_ramp() keeps that way through
weg_erweitern(). Its maintenance has
already been charged to its owner, but the charge is not removed when the bridge
adopts it. Later, bridge removal clears the owner before deleting the way, so
that maintenance is never refunded.
The residue is therefore exactly one way maintenance charge for each bridge
head.
With
dirt_road, the raw residue is 200: 100 per head. With the world scaling
used in the test, the player sees 800. With
gavel_road, it becomes the 1600 I
reported earlier.
FixThe first patch removes the existing way maintenance when
build_ramp() adopts
the way:
player_t::add_maintenance(
weg->get_owner(),
-weg->get_desc()->get_maintenance(),
weg->get_desc()->get_finance_waytype()
);
This is seven added lines. A null owner is already a no-op, so ways created by
the bridge builder are unaffected.
I deliberately kept this out of
bruecke_t::finish_rd(). That function also
runs while loading a savegame, and changing it would alter the maintenance of
existing saved bridges. Loaded and newly built bridges may still be
inconsistent, but that is a separate issue.
There are two similar
weg_erweitern() branches for cliff or elevated-way
endpoints. I could not produce a case that reaches them, so I have not changed
them without evidence.
TestsThe test patch adds ten maintenance tests covering:
- bridges built over existing ways;
- bridges built on bare ground;
- the maintenance while the bridge exists;
- repeated build/remove cycles;
- refused construction;
- ownership;
- rail;
- automatic construction;
- equality between manual and automatic construction.
Without the fix, the first test fails with the residual maintenance. With the
fix, the test can run in normal alphabetical order and does not contaminate the
36 tests after it.
The full suite passes:
216/216
Both patches apply to a clean r12117 and build from scratch without new
warnings.
Second patchScenario scripts currently cannot activate the automatic mode correctly.
tool_build_way_t::get_desc() extracts the name from
"<way>,<0|1>" but then
performs the lookup using the original parameter, including the comma. The copy
length is also one character short.
The second patch corrects both issues. It is the same fix already included in
the earlier automatic-toggle patch; if that is merged first, this second patch
reduces to the tests.