The client code seems to use UMT_*, TC_OCEAN, is_foo_unit(), and 
is_ocean() in several places for which I don't really understand the 
correct behaviour to support complex unit nativity.  Suggestions, 
opinions, and pointers to open bugs or patches welcome.

climisc.c:unit_color_type()
    This function uses UMT_* as a guide to set the correct background 
color for a unit, which seems only to be used in the Xaw client.  I 
would think that with complex nativity, it makes sense to assign the 
background color for tiles based on terrain, player knowledge, etc.  I 
don't have much familiarity with Xaw, so I don't understand if we can 
use logic like that used for the gtk2/3 clients' create_overlay_unit() 
(or some analog thereto) to initialise to black and then modify based on 
terrain/tileset definitions.

unitselect_common.c:usdlg_check_unit_location()
    This function has support for selecting units based on move_type 
explicitly.  I've never used this feature, so don't know common use 
cases. For complex nativity, I suspect that these become less useful: 
various amphibious units are inherently excluded, which may be 
unfortunate for rulesets with many ocean depths that allow most land 
units on the shallowest of oceans (which might not be particularly deep, 
and may not permit access by most seaworthy units).  Units with 
restrictions within classic nativity (only some land, only some oceanic, 
only roads, etc.) may be selected inappropriately, as they might be 
unsuitable for use for the intended purpose (e.g. cannot move to some 
target).

control.c:quickselect()
    This function has different heuristics for different environments. 
In rulesets that might have land transporters that carry land units, or 
sea transports that only carry sea units, or land units that can't move 
because they require roads, or similar complexities, I'm not sure that 
these heuristics are sufficient, nor that there aren't assumptions about 
available terrains and unit classes built into the heuristics.  That 
said, I don't use this, so hesitate to apply a patch that might break 
someone's gameplay.

    Naively, I would think the following might work sensibly for 
arbitrary terrains and units:

1) Native military attack units
2) Native transports
3) Other native units
4) Non-native units

    On the other hand, ordered like that, for the classic ruleset (as an 
example), Air, Helicopter, and Missile units might be selected much more 
often than was intended.  Perhaps reachability ought be added to the 
criteria checked, to decrease the chance of selection.  Given that this 
feature is most useful when the player has no time to think, it is 
somewhat important not to be surprising, either by selecting the wrong 
unit or by failing to select the expected unit because of the current 
nativity restrictions.

editor.c:editor_grab_tool()
    This just chooses ground units before sea units: I presume the 
intent is to deprioritise sea units in port (given the score reduction 
for transported units which adjusts this).  My thought here is to 
prioritise native units over non-native units (so that the order depends 
on terrain), but I don't remember using this tool either (unless it is 
responsible for unit ordering in the unit pane in the editor), so don't 
know if this would break someone's workflow.

    Separately, this function seems to use UCF_UNREACHABLE as a proxy 
for aerial units, and selects them first.  If the sorting is somewhat 
intended to describe altitude, this wouldn't work for Burrowing units 
from the alien ruleset, for example.  If the intent is to prioritise 
"important" units somehow, where aerial==important, there's no need to 
adjust it, as most unreachable units are likely to be of particular 
interest in most rulesets.  If I'm incorrect about the proxy use here, 
then nothing needs be done for UCF_UNREACHABLE.


    Those client uses of is_ocean() and TC_OCEAN not mentioned above 
seemed sane to me (and safe for complex nativity), although I wonder if 
the code ought standardise on one or the other representation of this 
idea (changelogs suggest general migration to is_ocean(), but some of 
the calls that use TC_OCEAN might need refactoring for this).  The idea 
of selecting coastal cities may also be slightly less useful for 
rulesets with particularly complex nativity, as some "coastal" cities 
might not support many "sea" units, etc.  Similarly, the current 
"coastal" check happens to also select all Oceanic cities that aren't 
built on a lake.

-- 
Emmet HIKORY

_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to