From 58aa95f3c4a6f57e0260211c2c6bf682074b82b1 Mon Sep 17 00:00:00 2001 From: victor_18993 Date: Tue, 28 Jul 2026 22:34:00 +0200 Subject: [PATCH] CHG: (victor_18993) bridge_planner_x::find_end takes optional max_length and flat_ends The script binding called bridge_builder_t::find_end_pos with a hardcoded maximum length of 10 and also_flat_ends set to false, so a script could never find a bridge end further away than ten tiles, nor an end on flat ground, no matter what the bridge descriptor or way_max_bridge_len allowed. This was not a deliberate API decision: the binding originally had no length limit at all, the literal 10 was introduced in r11608 when find_end_pos gained an explicit max_length parameter and the binding was adapted mechanically. find_end_pos has only these two script callers, the bridge tool of the player is a two click tool calling can_build_bridge/build_bridge directly, so only scripts were affected. find_end now accepts two optional parameters, using the same dispatch as command_x::build_station. The five parameter form keeps the old behaviour, so sqai and sqai_rail are unchanged until they opt in. The requested length is always clamped to way_max_bridge_len, to the maximum length of the descriptor plus one (see can_span_bridge) and to a hard limit of 254, since find_end_pos counts its loops in an uint8 and 255 would never terminate. Five tests added to test_way_bridge.nut: length below, at and above ten tiles, ends at a slope and on flat ground, a descriptor with and one without its own length limit, min_length larger than max_length, a tile forbidden by a scenario rule, an object of another player on the start tile, and repeated queries giving the same answer. Co-Authored-By: Claude Opus 5 (1M context) --- src/simutrans/script/api/api_pathfinding.cc | 56 +++++- src/simutrans/script/api/changelog.h | 1 + tests/all_tests.nut | 5 + tests/tests/test_way_bridge.nut | 193 ++++++++++++++++++++ 4 files changed, 252 insertions(+), 3 deletions(-) diff --git a/src/simutrans/script/api/api_pathfinding.cc b/src/simutrans/script/api/api_pathfinding.cc index fcf6f74ab..48be4e001 100644 --- a/src/simutrans/script/api/api_pathfinding.cc +++ b/src/simutrans/script/api/api_pathfinding.cc @@ -13,6 +13,7 @@ #include "../api_function.h" #include "../../builder/brueckenbauer.h" #include "../../builder/wegbauer.h" +#include "../../dataobj/settings.h" #include "../../descriptor/bridge_desc.h" #include "../../descriptor/way_desc.h" #include "../../tpl/binary_heap_tpl.h" @@ -121,7 +122,13 @@ bool way_builder_is_allowed_step(way_builder_t *bob, grund_t *from, grund_t *to) } -koord3d bridge_builder_find_end_pos(player_t *player, koord3d pos, my_ribi_t mribi, const bridge_desc_t *bridge, uint32 min_length) +/** + * Highest length a script may ask for: bridge_builder_t::find_end_pos counts the + * tested lengths in an uint8, so 255 would never terminate. + */ +static const uint32 MAX_SCRIPT_BRIDGE_LEN = 254; + +koord3d bridge_builder_find_end_pos(player_t *player, koord3d pos, my_ribi_t mribi, const bridge_desc_t *bridge, uint32 min_length, sint32 max_length, bool allow_flat_ends) { sint8 height; ribi_t::ribi ribi(mribi); @@ -129,13 +136,46 @@ koord3d bridge_builder_find_end_pos(player_t *player, koord3d pos, my_ribi_t mri if (player == NULL || bridge == NULL) { return koord3d::invalid; } - if (bridge_builder_t::find_end_pos(player, pos, ribi, height, bridge, min_length, 10, false) != NULL) { + // a script must not search further than the player could build + uint32 limit = min( welt->get_settings().way_max_bridge_len, MAX_SCRIPT_BRIDGE_LEN ); + if (bridge->get_max_length() > 0) { + // a bridge of max_length spans a distance of max_length+1, see bridge_builder_t::can_span_bridge + limit = min( limit, (uint32)bridge->get_max_length() + 1 ); + } + // max_length <= 0 means: as far as this bridge and the settings allow + const uint32 length = max_length <= 0 ? limit : min( (uint32)max_length, limit ); + + if (min_length > length) { + return koord3d::invalid; + } + if (bridge_builder_t::find_end_pos(player, pos, ribi, height, bridge, min_length, length, allow_flat_ends) != NULL) { return koord3d::invalid; } return pos; } +typedef koord3d (*bfe_type)(player_t*, koord3d, my_ribi_t, const bridge_desc_t*, uint32, sint32, bool); + +SQInteger bridge_planner_find_end(HSQUIRRELVM vm) +{ + /* possible calling conventions: + * + * find_end(pl, pos, dir, bridge, min_length) - top == 6 + * find_end(pl, pos, dir, bridge, min_length, max_length) - top == 7 + * find_end(pl, pos, dir, bridge, min_length, max_length, flat_ends) - top == 8 + */ + if (sq_gettop(vm) == 6) { + // keep the length the old binding used + sq_pushinteger(vm, 10); + } + if (sq_gettop(vm) == 7) { + sq_pushbool(vm, false); + } + return embed_call_t::call_function(vm, bridge_builder_find_end_pos, false); +} + + void export_pathfinding(HSQUIRRELVM vm) { /** @@ -210,14 +250,24 @@ void export_pathfinding(HSQUIRRELVM vm) create_class(vm, "bridge_planner_x", 0); /** * Find suitable end tile for bridge starting at @p pos going into direction @p dir. + * + * The search never goes further than the player could build: it is limited by the + * maximum length of @p bridge and by the @c way_max_bridge_len setting. + * * @param pl who wants to build a bridge * @param pos start tile for bridge * @param dir direction * @param bridge bridge descriptor * @param min_length bridge should have this minimal length + * @param max_length (optional parameter) bridge should not be longer than this, zero or negative means as long as allowed. Defaults to 10. + * @param flat_ends (optional parameter) if true then an end on flat ground is accepted too, not only an end at a slope. Defaults to false. * @returns coordinate of end tile or an invalid coordinate + * @typemask coord3d(player_x,coord3d,dir,bridge_desc_x,integer,integer,bool) */ - STATIC register_method(vm, bridge_builder_find_end_pos, "find_end", false, true); + STATIC register_function(vm, bridge_planner_find_end, "find_end", -6 /* at least 6 parameters */, + func_signature_t::get_typemask(false).c_str(), true /* static */); + + log_squirrel_type(func_signature_t::get_squirrel_class(false), "find_end", func_signature_t::get_squirrel_type(false, 0)); end_class(vm); } diff --git a/src/simutrans/script/api/changelog.h b/src/simutrans/script/api/changelog.h index fdba5bc9c..0cf6da01a 100644 --- a/src/simutrans/script/api/changelog.h +++ b/src/simutrans/script/api/changelog.h @@ -15,6 +15,7 @@ * * @section api-trunk Current trunk * + * - Changed @ref bridge_planner_x::find_end which accepts two optional parameters now, to search for longer bridges and for ends on flat ground * - Added @ref command_x::grid_lower, @ref command_x::grid_raise * - Added @ref settings::has_double_slopes, @ref settings::get_way_height_clearance * - Added @ref tile_x::is_crossing diff --git a/tests/all_tests.nut b/tests/all_tests.nut index 545840cbf..e116a0cb9 100644 --- a/tests/all_tests.nut +++ b/tests/all_tests.nut @@ -202,6 +202,11 @@ all_tests <- [ test_way_bridge_build_above_way, test_way_bridge_build_above_runway, test_way_bridge_planner, + test_way_bridge_planner_max_length, + test_way_bridge_planner_desc_limits, + test_way_bridge_planner_flat_ends, + test_way_bridge_planner_ownership, + test_way_bridge_planner_forbidden_by_scenario, test_way_road_build_single_tile, test_way_road_build_straight, test_way_road_build_bend, diff --git a/tests/tests/test_way_bridge.nut b/tests/tests/test_way_bridge.nut index 234990f6c..0e25bbbf5 100644 --- a/tests/tests/test_way_bridge.nut +++ b/tests/tests/test_way_bridge.nut @@ -634,3 +634,196 @@ function test_way_bridge_planner() ASSERT_EQUAL(pl.get_current_maintenance(), 0) RESET_ALL_PLAYER_FUNDS() } + + +// helper for the bridge planner tests: puts a ramp at the end of the corridor, +// asks for the end of the bridge and restores the ground again +function BRIDGE_FIND_END(pl, bridge_desc, start_pos, len, max_length, flat_ends) +{ + local end_pos = start_pos + coord3d(0, len, 0) + local has_ramp = len > 0 + if (has_ramp) { + ASSERT_EQUAL(command_x.set_slope(pl, end_pos, slope.north), null) + } + + local res + if (max_length == null) { + // the call as it was available before max_length/flat_ends were added + res = bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 0) + } + else if (flat_ends == null) { + res = bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 0, max_length) + } + else { + res = bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 0, max_length, flat_ends) + } + + if (has_ramp) { + ASSERT_EQUAL(command_x.set_slope(pl, end_pos, slope.flat), null) + } + return res.tostring() +} + + +function test_way_bridge_planner_max_length() +{ + local pl = player_x(0) + local start_pos = coord3d(11, 1, 0) + local bridge_desc = bridge_desc_x.get_available_bridges(wt_road)[0] + local invalid = coord3d(-1, -1, -1).tostring() + + // this test needs a bridge that can span more than ten tiles + ASSERT_TRUE(bridge_desc != null) + ASSERT_TRUE(bridge_desc.get_max_length() >= 12) + + ASSERT_EQUAL(command_x.set_slope(pl, start_pos, slope.south), null) + + // the old five parameter call keeps its length limit of ten tiles + ASSERT_EQUAL(BRIDGE_FIND_END(pl, bridge_desc, start_pos, 5, null, null), coord3d(11, 6, 0).tostring()) + ASSERT_EQUAL(BRIDGE_FIND_END(pl, bridge_desc, start_pos, 10, null, null), coord3d(11, 11, 0).tostring()) + ASSERT_EQUAL(BRIDGE_FIND_END(pl, bridge_desc, start_pos, 11, null, null), invalid) + + // with an explicit maximum length longer bridges are found + ASSERT_EQUAL(BRIDGE_FIND_END(pl, bridge_desc, start_pos, 11, 11, null), coord3d(11, 12, 0).tostring()) + ASSERT_EQUAL(BRIDGE_FIND_END(pl, bridge_desc, start_pos, 12, 12, null), coord3d(11, 13, 0).tostring()) + + // ... but the maximum length is obeyed + ASSERT_EQUAL(BRIDGE_FIND_END(pl, bridge_desc, start_pos, 12, 11, null), invalid) + + // zero means: as long as this bridge allows + ASSERT_EQUAL(BRIDGE_FIND_END(pl, bridge_desc, start_pos, 12, 0, null), coord3d(11, 13, 0).tostring()) + + // min_length larger than max_length finds nothing + ASSERT_EQUAL(command_x.set_slope(pl, coord3d(11, 6, 0), slope.north), null) + ASSERT_EQUAL(bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 8, 5).tostring(), invalid) + // same query twice gives the same answer + local first = bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 0, 12).tostring() + local second = bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 0, 12).tostring() + ASSERT_EQUAL(first, coord3d(11, 6, 0).tostring()) + ASSERT_EQUAL(second, first) + ASSERT_EQUAL(command_x.set_slope(pl, coord3d(11, 6, 0), slope.flat), null) + + // clean up + ASSERT_EQUAL(command_x.set_slope(pl, start_pos, slope.flat), null) + ASSERT_EQUAL(pl.get_current_maintenance(), 0) + RESET_ALL_PLAYER_FUNDS() +} + + +function test_way_bridge_planner_desc_limits() +{ + local pl = player_x(0) + local start_pos = coord3d(11, 1, 0) + local invalid = coord3d(-1, -1, -1).tostring() + + // a bridge without a length limit of its own, and a short one + local unlimited = null + local short_one = null + foreach (b in bridge_desc_x.get_available_bridges(wt_road)) { + if (unlimited == null && b.get_max_length() == 0) { + unlimited = b + } + if (short_one == null && b.get_max_length() > 0 && b.get_max_length() < 10) { + short_one = b + } + } + ASSERT_TRUE(unlimited != null) + ASSERT_TRUE(short_one != null) + + ASSERT_EQUAL(command_x.set_slope(pl, start_pos, slope.south), null) + + // a bridge without own limit is only bounded by what the caller asks for + ASSERT_EQUAL(BRIDGE_FIND_END(pl, unlimited, start_pos, 12, 0, null), coord3d(11, 13, 0).tostring()) + ASSERT_EQUAL(BRIDGE_FIND_END(pl, unlimited, start_pos, 12, 10, null), invalid) + + // a bridge with own limit spans get_max_length()+1 tiles, no matter what the caller asks for + local max_span = short_one.get_max_length() + 1 + ASSERT_EQUAL(BRIDGE_FIND_END(pl, short_one, start_pos, max_span, 20, null), (start_pos + coord3d(0, max_span, 0)).tostring()) + ASSERT_EQUAL(BRIDGE_FIND_END(pl, short_one, start_pos, max_span + 1, 20, null), invalid) + + // clean up + ASSERT_EQUAL(command_x.set_slope(pl, start_pos, slope.flat), null) + ASSERT_EQUAL(pl.get_current_maintenance(), 0) + RESET_ALL_PLAYER_FUNDS() +} + + +function test_way_bridge_planner_flat_ends() +{ + local pl = player_x(0) + local start_pos = coord3d(9, 12, 0) + local bridge_desc = bridge_desc_x.get_available_bridges(wt_road)[0] + local invalid = coord3d(-1, -1, -1).tostring() + + ASSERT_EQUAL(command_x.set_slope(pl, start_pos, slope.south), null) + + // there is no ramp in the corridor, only flat ground + ASSERT_EQUAL(bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 2, 3, false).tostring(), invalid) + ASSERT_EQUAL(bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 2, 3, true).tostring(), coord3d(9, 14, 0).tostring()) + + // clean up + ASSERT_EQUAL(command_x.set_slope(pl, start_pos, slope.flat), null) + ASSERT_EQUAL(pl.get_current_maintenance(), 0) + RESET_ALL_PLAYER_FUNDS() +} + + +function test_way_bridge_planner_ownership() +{ + local pl = player_x(0) + local start_pos = coord3d(11, 1, 0) + + // player 1 is the public player, its property may be used by everybody + if (!player_x(2).is_valid()) { + world.create_player(2, 1) + } + local other = player_x(2) + + local bridge_desc = bridge_desc_x.get_available_bridges(wt_road)[0] + local remover = command_x(tool_remover) + local invalid = coord3d(-1, -1, -1).tostring() + + ASSERT_TRUE(other.is_valid()) + ASSERT_TRUE(bridge_desc != null) + + ASSERT_EQUAL(command_x.set_slope(pl, start_pos, slope.south), null) + ASSERT_EQUAL(command_x.set_slope(pl, coord3d(11, 6, 0), slope.north), null) + + // the start tile now carries an object of another player + ASSERT_EQUAL(label_x.create(start_pos, other, "Foo Bar"), null) + + ASSERT_EQUAL(bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 0, 12).tostring(), invalid) + ASSERT_EQUAL(bridge_planner_x.find_end(other, start_pos, dir.south, bridge_desc, 0, 12).tostring(), coord3d(11, 6, 0).tostring()) + + // clean up + ASSERT_EQUAL(remover.work(other, start_pos), null) + ASSERT_EQUAL(command_x.set_slope(pl, coord3d(11, 6, 0), slope.flat), null) + ASSERT_EQUAL(command_x.set_slope(pl, start_pos, slope.flat), null) + RESET_ALL_PLAYER_FUNDS() +} + + +function test_way_bridge_planner_forbidden_by_scenario() +{ + local pl = player_x(0) + local start_pos = coord3d(11, 1, 0) + local bridge_desc = bridge_desc_x.get_available_bridges(wt_road)[0] + local invalid = coord3d(-1, -1, -1).tostring() + + ASSERT_EQUAL(command_x.set_slope(pl, start_pos, slope.south), null) + ASSERT_EQUAL(command_x.set_slope(pl, coord3d(11, 6, 0), slope.north), null) + + ASSERT_EQUAL(bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 0, 12).tostring(), coord3d(11, 6, 0).tostring()) + + rules.forbid_way_tool_rect(0, tool_build_bridge, wt_road, "", coord(11, 0), coord(11, 15), "Foo Bar") + ASSERT_EQUAL(bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 0, 12).tostring(), invalid) + rules.clear() + + ASSERT_EQUAL(bridge_planner_x.find_end(pl, start_pos, dir.south, bridge_desc, 0, 12).tostring(), coord3d(11, 6, 0).tostring()) + + // clean up + ASSERT_EQUAL(command_x.set_slope(pl, coord3d(11, 6, 0), slope.flat), null) + ASSERT_EQUAL(command_x.set_slope(pl, start_pos, slope.flat), null) + ASSERT_EQUAL(pl.get_current_maintenance(), 0) + RESET_ALL_PLAYER_FUNDS() +} -- 2.55.0.windows.3