Follow-up Comment #8, patch #3901 (project freeciv):
Thanks again for the detailed review.
How is move nativity checked before calling single_move_cost? Without calling
is_native_move(), a unit may move between any two tiles that are native to
that unit, even if the two tiles are only native because of a road that does
not permit that type of movement (e.g. a cardinal-only road on two diagonal
tiles, with no between tiles, and considering a diagonal move, in otherwise
non-native terrain). Since the actual move processing prohibits this, this
could result in a best path being calculated that cannot be executed.
I don't think it is safe to limit city channel checking to only the source
tile. With the implementation you have, it means that every city is an
acceptable destination for every unit, even non-native cities with no
surrounding native terrain that are not part of a city channel. With an
implementation that only guards the city channel check, it still allows units
to disembark from transport to non-native cities or prohibits units from
disembarking from transport to cities at the far end of a city channel
(depending on implementation). Yes, it would be redundant for cases of moving
between two cities, or between native tiles and cities, but there are cases
where it is not redundant.
I'm fine with the overlap change: it's too hard to usefully distinguish when
there is going to still be an active blocade later :)
Two changes I noticed beyond the list: not initialising pcity to NULL in
pf_is_ok_move_tile (why?), and removing the redundant check of whether the
potential transport owner and moving player were the same (good catch!).
Reply to this item at:
Message sent via/by Gna!
Freeciv-dev mailing list