Freeciv

hm, may the algorithm needs rethinking, maybe better range airdromes by turns to repair count?

oops modified not the subroutine I wanted... but, well, this one also needs this fix...

Marko Lindqvist wrote:

You should not have duplicated logic in hp_gain_coord() and new unit_hp_gain_at(). Better to change former to take tile parameter (like one would assume it to do, from its name!) and use that.

get_unit_bonus(punit, EFT_HP_REGEN) will return bonus in unit's current tile, not at the tile you're interested about

There is a kludge that this function does: we temporarily change the unit's tile on the another tile and hope for the best (that "HP_Regen" does not have "MaxUnitsOnTile" requirement or something). A good function that calculates effect values in case of a unit is in another place (or has another health or mp, or that it is another turn fragment) probably worth a sepatate C file, hardly even creating a virtual clone will resolve all of the problems. I have put here a simplified local solution to avoid disturbing or slowing down the rest of the game.

&& def_ai_city_data(pcity, ait)->grave_danger

unit_list_size(ptile->units) << 1) {

This might be a sensible approximation, but you are aware that it makes a city with several defenseless diplomats, that can even be all wiped at once if the ruleset implements Wipe Units action, seem like one defended well enough?

Well, probably the main part of the fix here is starting iteration from the current tile, so we don't look for the next city in a grave danger as long as we have mp. But I think gathering all bombers of a continent together in one city because a trireme has approached to it is not optimal. We have some functions that supply land (presumably better) defenders to cities, if they often don't work, it's another problem. Maybe we could take into account ai_city.danger value and compare it to what we already have in the city, but this will complicate the code and will be actually a change beyound a simple fix.

Marko Lindqvist wrote:

Empty line between variable declarations and the following code lines is missing in several places

So this is the only one of my comments that you didn't defend against.
Also, no C++ comments ('//') in C code for branches earlier than master (and not needed in master either, so you don't need to do different version for that)

Well, putting hp_gain_coord() into common code is also a worthy improvement..

I'll publish a new diff when I test it. There are some other flaws in this one. The improved logic of "strategic airbase" will be:
1. If the unit is damaged, don't rebase on a base that will make (full) regeneration slower than another one.
2. If among bases matching point 1. there is a city in grave danger (over 1:2) nearby, don't rebase to a base that is not the case.
3. If there are no bases matching 2. criterium, among ones matching 1. select one with best FSTB rating.
4. Select the closest matching base.

"Nearest airdrome" also would care only full regeneration speed, for simplicity.

(Hm, maybe it's not so good considering "lure" effect? Domestic city (no Airport) regenerates 6 hp for a 20hp Fighter, empty enemy airbase will promise 2 hp, if the unit has lost 2 hp in a battle, it will select just the closer one... But this problem needs its own logic any way.)

Well, I'll try to port it back in a short while.

I noticed one style issue:

There should be empty line between these lines:

int start_worth;
        unit_tile_set(pvirtual, unit_tile(punit));

Pushed to master & S3_1.