From f1a3939bc8b32acf402f9e6038111856434d74ae Mon Sep 17 00:00:00 2001 From: Ihnatus Date: Fri, 11 Jul 2025 19:54:57 +0300 Subject: [PATCH] Correct airlift action possibility check Don't airlift onto a non-allied unit tile. Consider airlift to ally cities being not blocked by counters if the counters are not checked anywhere. Consider unseen tiles potentially providing native terrain to airliftable "boats". See RM#1564 Signed-off-by: Ihnatus --- common/movement.c | 96 ++++++++++++++++++++++++++++++---------------- common/movement.h | 53 +++++++++++++++++++++++++ common/unit.c | 68 +++++++++++++++++--------------- server/unittools.c | 2 +- 4 files changed, 156 insertions(+), 63 deletions(-) diff --git a/common/movement.c b/common/movement.c index 1b23866747..62d274fa31 100644 --- a/common/movement.c +++ b/common/movement.c @@ -245,41 +245,30 @@ bool is_city_channel_tile(const struct civ_map *nmap, const struct tile *ptile, const struct tile *pexclude) { - struct dbv tile_processed; - struct tile_list *process_queue = tile_list_new(); - bool found = FALSE; - - dbv_init(&tile_processed, map_num_tiles()); - for (;;) { - dbv_set(&tile_processed, tile_index(ptile)); - adjc_iterate(nmap, ptile, piter) { - if (dbv_isset(&tile_processed, tile_index(piter))) { - continue; - } else if (piter != pexclude - && is_native_to_class(punitclass, tile_terrain(piter), - tile_extras(piter))) { - found = TRUE; - break; - } else if (piter != pexclude - && NULL != tile_city(piter)) { - tile_list_append(process_queue, piter); - } else { - dbv_set(&tile_processed, tile_index(piter)); - } - } adjc_iterate_end; + MAP_TILE_CONN_TO_BY(nmap, ptile, piter, + piter != pexclude + && is_native_to_class(punitclass, tile_terrain(piter), + tile_extras(piter)), + piter != pexclude && NULL != tile_city(piter)); - if (found || 0 == tile_list_size(process_queue)) { - break; /* No more tile to process. */ - } else { - ptile = tile_list_front(process_queue); - tile_list_pop_front(process_queue); - } - } + return NULL != ptile; +} - dbv_free(&tile_processed); - tile_list_destroy(process_queue); +/************************************************************************//** + Check for a city channel with restriction to pov_player current knowledge +****************************************************************************/ +bool may_be_city_channel_tile(const struct civ_map *nmap, + const struct unit_class *punitclass, + const struct tile *ptile, + const struct player *pov_player) +{ + MAP_TILE_CONN_TO_BY(nmap, ptile, piter, + !tile_is_seen(piter, pov_player) + || is_native_to_class(punitclass, tile_terrain(piter), + tile_extras(piter)), + NULL != tile_city(piter)); - return found; + return NULL != ptile; } /************************************************************************//** @@ -311,6 +300,49 @@ bool can_exist_at_tile(const struct civ_map *nmap, return is_native_tile(utype, ptile); } +/************************************************************************//** + Return if a unit of utype could possibly "exist" at the city tile of pcity + given the information known to pov_player. pcity is presumed to exist. + nmap is supposed to be client map. + This means it can physically be present on the tile (without the use of a + transporter). See can_exist_at_tile() for the omniscient check. +****************************************************************************/ +bool could_exist_in_city(const struct civ_map *nmap, + const struct player *pov_player, + const struct unit_type *utype, + const struct city *pcity) +{ + struct unit_class *uclass; + struct tile *ctile; + + fc_assert_ret_val(NULL != pcity && NULL != utype, FALSE); + + ctile = city_tile(pcity); + uclass = utype_class(utype); + + if (uclass_has_flag(uclass, UCF_BUILD_ANYWHERE)) { + /* If the city stands, it can exist there */ + return TRUE; + } + adjc_iterate(nmap, ctile, ptile) { + if (!tile_is_seen(ptile, pov_player) + || is_native_tile_to_class(uclass, ptile)) { + /* Could be native. This ignores a rare case when we don't see + * only the city center and any native terrain is NoCities */ + return TRUE; + } + } adjc_iterate_end; + + if (1 == game.info.citymindist + && may_be_city_channel_tile(nmap, uclass, ctile, pov_player)) { + /* Channeled. */ + return TRUE; + } + + /* It definitely can't exist there */ + return FALSE; +} + /************************************************************************//** Return TRUE iff the unit can "exist" at this location. This means it can physically be present on the tile (without the use of a transporter). See diff --git a/common/movement.h b/common/movement.h index 611b679383..5984b3059d 100644 --- a/common/movement.h +++ b/common/movement.h @@ -68,6 +68,10 @@ bool is_city_channel_tile(const struct civ_map *nmap, const struct unit_class *punitclass, const struct tile *ptile, const struct tile *pexclude); +bool may_be_city_channel_tile(const struct civ_map *nmap, + const struct unit_class *punitclass, + const struct tile *ptile, + const struct player *pov_player); bool is_native_tile(const struct unit_type *punittype, const struct tile *ptile); @@ -98,6 +102,10 @@ bool is_native_near_tile(const struct civ_map *nmap, bool can_exist_at_tile(const struct civ_map *nmap, const struct unit_type *utype, const struct tile *ptile); +bool could_exist_in_city(const struct civ_map *nmap, + const struct player *pov_player, + const struct unit_type *utype, + const struct city *pcity); bool can_unit_exist_at_tile(const struct civ_map *nmap, const struct unit *punit, const struct tile *ptile); bool can_unit_survive_at_tile(const struct civ_map *nmap, @@ -147,6 +155,51 @@ const char *move_points_text_full(int mp, bool reduce, const char *prefix, const char *none, bool align); const char *move_points_text(int mp, bool reduce); +/* Simple algorithm that checks connectivity of _tile_ on _map_ + * to a tile where _found_ is true via adjacency of tiles where _conn_. + * _found_ and _conn_ must be boolean expressions checking tile _piter_. + * _conn_ is not checked at starting and finishing _tile_. + * Assigns the found tile to _tile_, or NULL if not found */ +#define MAP_TILE_CONN_TO_BY(_map_, _tile_, _piter_, _found_, _conn_) {\ + struct dbv tile_processed; \ + struct tile_list *process_queue = tile_list_new(); \ + bool found = FALSE; \ + \ + dbv_init(&tile_processed, map_num_tiles()); \ + for (;;) { \ + dbv_set(&tile_processed, tile_index(_tile_)); \ + adjc_iterate(_map_, _tile_, _piter_) { \ + if (dbv_isset(&tile_processed, tile_index(_piter_))) { \ + continue; \ + } else if (_found_) { \ + _tile_ = _piter_; \ + found = TRUE; \ + break; \ + } else if (_conn_) { \ + tile_list_append(process_queue, _piter_); \ + } else { \ + dbv_set(&tile_processed, tile_index(_piter_)); \ + } \ + } adjc_iterate_end; \ + \ + if (found) { \ + /* got it*/ \ + break; \ + } \ + if (0 == tile_list_size(process_queue)) { \ + /* No more tile to process. */ \ + _tile_ = NULL; \ + break; \ + } else { \ + _tile_ = tile_list_front(process_queue); \ + tile_list_pop_front(process_queue); \ + } \ + } \ + \ + dbv_free(&tile_processed); \ + tile_list_destroy(process_queue); \ +} + #ifdef __cplusplus } #endif /* __cplusplus */ diff --git a/common/unit.c b/common/unit.c index e9665ec6f2..f0a869c1fe 100644 --- a/common/unit.c +++ b/common/unit.c @@ -76,6 +76,10 @@ bool are_unit_orders_equal(const struct unit_order *order1, based on -- one player can't see whether another's cities are currently able to airlift. (Clients other than global observers should only call this with a non-NULL 'restriction'.) + Note that it does not take into account the possibility of conquest + of unseen cities by an ally. That is to facilitate airlifting dialog + usage most times. It is supposed that you don't ally ones who + won't share maps with you when needed. **************************************************************************/ enum unit_airlift_result test_unit_can_airlift_to(const struct civ_map *nmap, @@ -85,11 +89,12 @@ enum unit_airlift_result { const struct city *psrc_city = tile_city(unit_tile(punit)); const struct player *punit_owner; + const struct tile *dst_tile = NULL; + const struct unit_type *putype = unit_type_get(punit); enum unit_airlift_result ok_result = AR_OK; if (0 == punit->moves_left - && !utype_may_act_move_frags(unit_type_get(punit), - ACTION_AIRLIFT, 0)) { + && !utype_may_act_move_frags(putype, ACTION_AIRLIFT, 0)) { /* No moves left. */ return AR_NO_MOVES; } @@ -113,13 +118,15 @@ enum unit_airlift_result return AR_BAD_DST_CITY; } - if (pdest_city - && (NULL == restriction - || (tile_get_known(city_tile(pdest_city), restriction) - == TILE_KNOWN_SEEN)) - && !can_unit_exist_at_tile(nmap, punit, city_tile(pdest_city))) { - /* Can't exist at the destination tile. */ - return AR_BAD_DST_CITY; + if (NULL != pdest_city) { + dst_tile = city_tile(pdest_city); + + if (NULL != restriction + ? !could_exist_in_city(nmap, restriction, putype, pdest_city) + : !can_exist_at_tile(nmap, putype, dst_tile)) { + /* Can't exist at the destination tile. */ + return AR_BAD_DST_CITY; + } } punit_owner = unit_owner(punit); @@ -134,35 +141,36 @@ enum unit_airlift_result return AR_BAD_SRC_CITY; } - if (pdest_city - && punit_owner != city_owner(pdest_city) - && !(game.info.airlifting_style & AIRLIFTING_ALLIED_DEST - && pplayers_allied(punit_owner, city_owner(pdest_city)))) { + if (NULL != pdest_city && punit_owner != city_owner(pdest_city) + && (!(game.info.airlifting_style & AIRLIFTING_ALLIED_DEST + && pplayers_allied(punit_owner, city_owner(pdest_city))) + || is_non_allied_unit_tile(dst_tile, punit_owner))) { /* Not allowed to airlift to this destination. */ return AR_BAD_DST_CITY; } - if (NULL == restriction || city_owner(psrc_city) == restriction) { - /* We know for sure whether or not src can airlift this turn. */ - if (0 >= psrc_city->airlift - && (!(game.info.airlifting_style & AIRLIFTING_UNLIMITED_SRC) - || !game.info.airlift_from_always_enabled)) { - /* The source cannot airlift for this turn (maybe already airlifted - * or no airport). - * See also do_airline() in server/unittools.h. */ - return AR_SRC_NO_FLIGHTS; - } /* else, there is capacity; continue to other checks */ - } else { - /* We don't have access to the 'airlift' field. Assume it's OK; can - * only find out for sure by trying it. */ - ok_result = AR_OK_SRC_UNKNOWN; + /* Check airlift capacities */ + if (!game.info.airlift_from_always_enabled) { + if (NULL == restriction || city_owner(psrc_city) == restriction) { + /* We know for sure whether or not src can airlift this turn. */ + if (0 >= psrc_city->airlift + && !(game.info.airlifting_style & AIRLIFTING_UNLIMITED_SRC)) { + /* The source cannot airlift for this turn (maybe already airlifted + * or no airport). + * See also do_airline() in server/unittools.h. */ + return AR_SRC_NO_FLIGHTS; + } /* else, there is capacity; continue to other checks */ + } else { + /* We don't have access to the 'airlift' field. Assume it's OK; can + * only find out for sure by trying it. */ + ok_result = AR_OK_SRC_UNKNOWN; + } } - if (pdest_city) { + if (NULL != pdest_city && !game.info.airlift_to_always_enabled) { if (NULL == restriction || city_owner(pdest_city) == restriction) { if (0 >= pdest_city->airlift - && (!(game.info.airlifting_style & AIRLIFTING_UNLIMITED_DEST) - || !game.info.airlift_to_always_enabled)) { + && !(game.info.airlifting_style & AIRLIFTING_UNLIMITED_DEST)) { /* The destination cannot support airlifted units for this turn * (maybe already airlifted or no airport). * See also do_airline() in server/unittools.h. */ diff --git a/server/unittools.c b/server/unittools.c index c4dc36410c..3e0901e5d7 100644 --- a/server/unittools.c +++ b/server/unittools.c @@ -3908,7 +3908,7 @@ static struct unit_move_data_list *construct_move_data_list(struct unit *punit, /**********************************************************************//** Moves a unit. No checks whatsoever! This is meant as a practical function for other functions, like do_airline(), which do the checking - themselves. + either by themselves or by their callers. If you move a unit you should always use this function, as it also sets the transport status of the unit correctly. Note that the source tile (the -- 2.45.2