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
0%
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.
History
#1
Updated by Alexandro Ignatiev about 1 year ago
hm, may the algorithm needs rethinking, maybe better range airdromes by turns to repair count?
#2
Updated by Alexandro Ignatiev about 1 year ago
oops modified not the subroutine I wanted... but, well, this one also needs this fix...
#3
Updated by Alexandro Ignatiev about 1 year ago
- File aiair-no-bunny.patch aiair-no-bunny.patch added
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 about 1 year 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 about 1 year 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 about 1 year 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 about 1 year ago
Well, putting hp_gain_coord()
into common code is also a worthy improvement..
#8
Updated by Alexandro Ignatiev about 1 year 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 about 1 year ago
- File air-no-bunny.patch air-no-bunny.patch added
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 about 1 year ago
- File air-no-bunny.patch air-no-bunny.patch added
(misindented line)
#11
Updated by Marko Lindqvist about 1 year 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 about 1 year ago
Well, I'll try to port it back in a short while.
#13
Updated by Marko Lindqvist about 1 year 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 about 1 year ago
- File 0010-AI-Stop-fueled-units-from-unnecessary-hopping-betwee.patch 0010-AI-Stop-fueled-units-from-unnecessary-hopping-betwee.patch added
- Status changed from In Progress to Resolved
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 12 months ago
- File 2.6-air-no-bunny.patch 2.6-air-no-bunny.patch added
Thanks for the fix. This patch goes for 2.6 and 3.0.
#16
Updated by Alexandro Ignatiev 12 months ago
- File 2.6-air-no-bunny.patch 2.6-air-no-bunny.patch added
variant with HRM link
#17
Updated by Marko Lindqvist 12 months ago
- File 0023-AI-Stop-fueled-units-from-unnecessary-hopping-betwee.patch 0023-AI-Stop-fueled-units-from-unnecessary-hopping-betwee.patch added
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 12 months ago
Pushed to master & S3_1.
#19
Updated by Marko Lindqvist 12 months ago
- Status changed from Resolved to Closed
- Assignee set to Marko Lindqvist