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: <http://gna.org/patch/?3901> _______________________________________________ Message sent via/by Gna! http://gna.org/ _______________________________________________ Freeciv-dev mailing list Freecivfirstname.lastname@example.org https://mail.gna.org/listinfo/freeciv-dev