Follow-up Comment #2, patch #3721 (project freeciv):
I don't think it is safe to use !can_exist_at_tile() as the filter if
we're looking to exclude transported units: that implies an assertion that at
any time the client is sufficiently idle to take a new command, no
untransported unit cannot exist on it's tile, which wouldn't be thread-safe if
units are drowning, or possibly in certain transport situations (I haven't
reviewed the client code sufficiently to confirm this: it may be that the
client already group-moves units the server permits to move, or similar).
Additionally, units in nested transport may be selected if they are native,
even if they cannot be used (e.g. Cruise Missile in Mech. Inf. in Transport).
If the intent is to find units that are both transported and unable to
disembark, we should be using something like:
ptrans = unit_transport_get(punit);
if (ptrans != NULL
&& !(can_unit_unload(punit, ptrans)
&& can_unit_exist_at_tile(punit, utile))) {
return FALSE;
}
Using tile_city(utile) != NULL in both conditions causes any units in
cities to appear twice. Perhaps these conditions could depend instead on the
local terrain+infrastructure, so:
LAND: (!is_ocean(utile) && is_native_tile(unit_type(punit), utile)))
SEA: (is_ocean(utile) && is_native_tile(unit_type(punit), utile)))
This still causes some duplication (some units may be native to both,
either inherently, or due to roads/bases/etc.), but doesn't automatically
select all sea units in harbour when selecting LAND. On the other hand, it
will make non-native (e.g. BuildAnywhere) units unselectable, which may be
confusing.
The above said, I don't use this selector other than for tile range, so
these opinions may be ill-informed.
_______________________________________________________
Reply to this item at:
<http://gna.org/patch/?3721>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
