I've been working on nativity fixes for pathfinding, and am currently trying to consolidate the callbacks for get_MC(), but discovered a few questions I've been unable to answer from code inspection.
Firstly, my candidate nativity-assumption-free get_MC() is: static int normal_move_unit(const struct tile *src, enum direction8 dir, const struct tile *dst, const struct pf_parameter *param) { bool native_dst = is_native_tile_to_class(param->uclass, dst); if ( (is_non_allied_unit_tile(dst, param->owner) || is_non_allied_city_tile(dst, param->owner)) /* There is someone already there. */ && (native_dst || (uclass_has_flag(param->uclass, UCF_ATTACK_NON_NATIVE) && !BV_ISSET(param->unit_flags, UTYF_ONLY_NATIVE_ATTACK))) /* Destination tile can be attacked. */ && (BV_ISSET(param->unit_flags, UTYF_MARINES) || uclass_has_flag(param->uclass, UCF_ATT_FROM_NON_NATIVE) || is_native_tile_to_class(param->uclass, src))) { /* Source tile can be used to launch attack. */ /* This looks like an attack. */ if (BV_ISSET(param->unit_flags, UTYF_ONEATTACK) || uclass_has_flag(param->uclass, UCF_MISSILE)) { return param->move_rate; /* No more moves this turn. */ } else { return SINGLE_MOVE; /* Normal attack cost. */ } } else if (native_dst /* Destination is native. */ || 0 < unit_class_transporter_capacity(dst, param->owner, param->uclass) /* There is a transporter available for use. */ || (tile_city(dst) && (uclass_has_flag(param->uclass, UCF_BUILD_ANYWHERE) || is_native_near_tile(param->uclass, dst) || (1 == game.info.citymindist && is_city_channel_tile(param->uclass, dst))))) { /* Destination is a native city. */ /* This looks like a reasonable move. */ return single_move_cost(param, src, dst); } else { /* Nobody to attack, nowhere to move. */ return PF_IMPOSSIBLE_MC; } } Aside from UTYF_IGTER (which I put in single_move_cost()) and UTYF_TRIREME (which was already in single_move_cost()), have I missed any considerations that should be applied? Note that this is organised for logical analysis, rather than speed: I would expect to broaden the tree to test less expensive conditionals first for actual use (but this makes it a little harder to see the logic for each possibility). Secondly, I'm a bit unsure what to do with the attack and overlap variants. These could be handled by setting TB callbacks (don't move from non-native to non-native, don't move through occupied tiles, etc.), but pathfinding only seems to allow a single TB to apply, so this might mean having a fair number of TB callbacks for all the combinations of behaviour one might want. Alternately, I could construct more variants for MC callbacks, with inherent TB assumptions, which brings the balance of duplication from TB callbacks to MC callbacks. Lastly, I could create a bundle of helper functions to be used by different TB or MC callbacks and use either of the solutions above, but this increases the stack depth for the code, and so may not be ideal. I lean towards fewer TB callbacks because it means not calling things like is_allied_unit_tile() multiple times per step, but remain unsure because the MC callbacks are already complex enough without adding additional TB logic, and I'd hope the result of whatever I do would be readable. Does anyone have suggestions on whether to put more differentiation in MC callbacks or TB callbacks? Would more helper functions to reduce the verbosity of the code be good, or just slow things down to no advantage? Thirdly, most of the existing code seems to use is_non_allied_tile to differentiate attacks from non-attacks. Given that there are three potential states between players (WAR, PEACE, ALLIANCE), should these be adjusted so that allied tiles are considered normal movement, enemy tiles are considered attacks, and peaceful tiles return PF_IMPOSSIBLE? Or would such a change cause horrible confusion to pathfinding units if the peaceful units moved? Thank you. -- Emmet HIKORY _______________________________________________ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev