Project

Profile

Help

HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...

Feature #924328

Feature #658896: Restore AI ability to build Fighters (fuel = 1 units)

Don't rebase air units if it disturbs repairing

Added by Alexandro Ignatiev 4 months ago. Updated 2 months ago.

Status:
Closed
Priority:
Normal
Category:
AI
Sprint/Milestone:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

Air unit noticed to stuck at low hp if different cities around are menaced by enemy forces, so they constantly fly around. Let they consider rebasing only if it gives them an advantage in the speed of repairing to full hp.

aiair-no-bunny.patch (4.02 KB) aiair-no-bunny.patch Alexandro Ignatiev, 2021-03-28 10:41 PM
aiair-no-bunny.patch (6.54 KB) aiair-no-bunny.patch Alexandro Ignatiev, 2021-04-01 10:45 PM
air-no-bunny.patch (13 KB) air-no-bunny.patch Alexandro Ignatiev, 2021-05-10 10:22 PM
air-no-bunny.patch (12.9 KB) air-no-bunny.patch Alexandro Ignatiev, 2021-05-10 10:29 PM
0010-AI-Stop-fueled-units-from-unnecessary-hopping-betwee.patch (13 KB) 0010-AI-Stop-fueled-units-from-unnecessary-hopping-betwee.patch Marko Lindqvist, 2021-05-24 02:26 AM
2.6-air-no-bunny.patch (13 KB) 2.6-air-no-bunny.patch Alexandro Ignatiev, 2021-05-24 07:06 PM
2.6-air-no-bunny.patch (13 KB) 2.6-air-no-bunny.patch Alexandro Ignatiev, 2021-05-24 07:19 PM
0023-AI-Stop-fueled-units-from-unnecessary-hopping-betwee.patch (13.1 KB) 0023-AI-Stop-fueled-units-from-unnecessary-hopping-betwee.patch S3_0, S2_6 Marko Lindqvist, 2021-05-25 01:57 AM

History

#1 Updated by Alexandro Ignatiev 4 months ago

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

#2 Updated by Alexandro Ignatiev 4 months ago

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

#3 Updated by Alexandro Ignatiev 4 months ago

Modified also dai_find_strategic_airbase(). No hopping if:
  • the unit is up to regenerate where it is;
  • the regeneration site repairs the unit slower than the current one;
  • the city we want to defend from a grave danger already has twice more units that menace it this turn.

(patch for master provided, not tested on 2.6)

#4 Updated by Marko Lindqvist 4 months ago

  • Tracker changed from Task to Feature

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

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

&& 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?


I'd consider this too big a not-strictly-bugfix change to mature S2_6 if it wasn't about AI air units. As AI has not really known how to use them in earlier 2.6 versions, there hardly can be any regression caused by changing their behavior? So keeping 2.6.5 target for now.

#5 Updated by Alexandro Ignatiev 4 months ago

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.

#6 Updated by Marko Lindqvist 4 months ago

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)

#7 Updated by Alexandro Ignatiev 4 months ago

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

#8 Updated by Alexandro Ignatiev 3 months ago

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.)

#9 Updated by Alexandro Ignatiev 3 months ago

Done. Well, sometimes with this patch a fighter won't rebase from an (otherwise empty) city that is in danger of a trireme attack, but in the previous version it will mostly just fly back and forth from it that is not better, so I propose fix other things elsewhere.

#10 Updated by Alexandro Ignatiev 3 months ago

(misindented line)

#11 Updated by Marko Lindqvist 3 months ago

  • Status changed from New to In Progress

Will you port this to S3_0 at least, or should we target S3_1 & master only?

#12 Updated by Alexandro Ignatiev 3 months ago

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

#13 Updated by Marko Lindqvist 3 months ago

I noticed one style issue:

There should be empty line between these lines:

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

#14 Updated by Marko Lindqvist 2 months ago

Marko Lindqvist wrote:

I noticed one style issue:

There should be empty line between these lines:

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

Fixed in attached patch for master & S3_1. I plan to push to those branches soon, but to leave the ticket open for later backports to stable branches.

#15 Updated by Alexandro Ignatiev 2 months ago

Thanks for the fix. This patch goes for 2.6 and 3.0.

#16 Updated by Alexandro Ignatiev 2 months ago

variant with HRM link

#17 Updated by Marko Lindqvist 2 months ago

Same style correction for S2_6/S3_0 patch. Set also commit message to same as in the S3_1/master patch so people searching for the same commit from another branch can find it more easily.

#18 Updated by Marko Lindqvist 2 months ago

Pushed to master & S3_1.

#19 Updated by Marko Lindqvist 2 months ago

  • Status changed from Resolved to Closed
  • Assignee set to Marko Lindqvist

Also available in: Atom PDF