Opened 8 years ago

Last modified 7 years ago

#4142 closed defect

[PATCH] Petra AI - Regicide Support — at Version 29

Reported by: elexis Owned by:
Priority: Nice to Have Milestone: Alpha 22
Component: AI Keywords: patch
Cc: Sandarac Patch:

Description (last modified by Sandarac)

If regicide gamemode is enabled, a player loses if the hero dies. So the AI should do its best to avoid that. There are two more options planned (prohibiting garrisoning the hero and prohibiting attacks/auras in age 1/2). The hero could be used to scout and to kill women in age 1, but needs to be retreated if too many men attack it. Some heroes like elephants are very slow at escaping, thus more vulnerable.

Change History (39)

by Sandarac, 8 years ago

Petra will attempt to garrison the hero for healing if the hero's health drops lower than 60%, including cases when the hero is part of an army (hasn't been extensively tested).

comment:1 by Sandarac, 8 years ago

Keywords: rfc patch added
Summary: Petra AI - Regicide Support[PATCH] Petra AI - Regicide Support

comment:2 by elexis, 8 years ago

Thanks for working on the AI :)

Some considerations:

  • Hero elephants can't be garrisoned (also that option might be enabled later). If it can't be garrisoned, just retreating to the civic center or a place with many defenses might be an idea.
  • Cavalry heroes can retreat easily, while hero elephants might need a really long time to get back to the CC. So perhaps the 0.6 number could be adapted (0.3 for cavalry, 0.5 for infantry, 0.7 for elephants?)
  • What does it do with the mauryan hero healer?
  • Surrounding the enemy hero is an easy way to defeat a player, but I guess that's hard to avoid with the AI.

by Sandarac, 8 years ago

Petra will try to return the hero to a base (and garrison the hero if it can) using class specific health thresholds.

by elexis, 8 years ago

Attachment: commands.txt added

In this r18614 replay the hero of the yellow AI player died in the first attack wave.

comment:3 by elexis, 8 years ago

It seems this is a UnitAI issue too. You should perhaps set the UnitAI stance of the hero to passive, so that it won't start attacking the attackers if it should retreat.

Also "50%" HP might be an improvable consideration. If there are only 5 men attacking him, he can fight them off, but if there are 200 attackers, it just shouldn't go there. Perhaps it could try to avoid enemies in range N (perhaps too hard to do).

Since the AI has a revealed map, it can't do anything useful with the hero it in the beginning (other than trying to rush some enemies if it has a cavalry or infantry hero).

There were character traits planned for the AI, like aggressiveness. Don't know if that has been implemented partially yet, likely not.

comment:4 by elexis, 8 years ago

Right, the code already sets heroes as passive on retreat.

I could notice successful retreats in some games, but in other games the heroes were still killed when they got surrounded. Retreating earlier doesn't really help to avoid the surround. Perhaps it would be better to not move the hero at all?

Your idea mentioned on 2016-08-23 in irc to use ExecuteQueryAroundPos might even be possible, considering that attackPlan.js also uses Engine.ComputePath.

by Sandarac, 8 years ago

Petra will not add the hero to an army if its destination is across water, or if the hero is hurt. The hero will retreat if there are more than six enemy units surrounding it.

comment:5 by elexis, 8 years ago

In 18731:

Simple petra AI regicide support. Patch by Sandarac, refs #4142.

The AI will retreat the hero in regicide games once it reaches less than 70% health.
It will garrison it in healing structures. Elephant heroes will retreat to the home base.
Once the hero has healed, it will be used for attacks again.

comment:6 by elexis, 8 years ago

Thanks for the patch Sandarac! I have committed your patch since it works very well (and before your patch, regicide AI games were guaranteed to be over after 10 minutes in the first attack). Hope to see more of your patches, even if it's hard to get reviews.

I have replaced the hurt() check with a healthLevel < 0.8 check, so that it would attack with heroes if they have sufficient HP. Also merged those hero type distinctions for now.

  • That check for near enemies is unfortunately extremely slow. It was lagging so much when I ran some tests with the patch, but not nearly as much without the patch. It was freezing every few turns for 2 or 3 seconds. After removing the check, it was working fine. Ideally we could use ExecuteQueryAroundPos of RangeManager, but it is out of reach currently.
  • TODO: It were great if the AI would focus it's planned attacks on the enemy hero.
Last edited 8 years ago by elexis (previous) (diff)

comment:7 by elexis, 8 years ago

Keywords: rfc removed

comment:8 by mimo, 8 years ago

I've not tried any regicide game, but just looking at the patch, here are some remarks

changes in attackPlan line 646: what is the logic of preventing a hurt hero to be affected to an army here, and not in other cases like 603, 626 and 659? and i think other heroes are not disabled in a regicide game? if yes, this restriction (specially the overseas one) should apply only to the right "hero" and not all of them. Maybe when #252 is used, a new class REGICIDE should be added to the concerned "hero" and all your current tests on "HERO" be changed to "REGICIDE". You should add a TODO here.

changes in defenseManager line 391: same remark about other heroes. Should add a TODO there.

line 485: what is the use of the stopMoving method?

lines 487-492 and 503-510: looks weird here. It would be better done in the checkEvent method of the attackPlan. Or even better, you could add a new gameTypeManager which would contains a checkEvents method responding to the different gameTypes.

I don't understand how this code can work: whatever the result of the garrisonUnitForHealing, you try to do a move if the hero was in an attack? And as the checkEvents methods are called every turn the ai is run, you will try to redo this garrisoning always and always up to the time the hero reaches its holder if it ever reaches it (as the move of line 509 can put him in a different direction than the healing structure he wanted to be garrisoned in)? that has no sense. Check what is done in garrisonManager: a unit garrisoned by the garrisonManager will have its plan changed to -2 or -3 up to the time it reaches its garrisonHolder, and then it will have the standard garrisoned flag. So all your block 583-512 should not be done if the hero has plan=-2 or -3, and just after the garrisonUnitForHealing, you should recompute its plan and do a continue if it is now -2 or -3. Futhermore garrisonUnitForHealing (as indicated by its name) only looks for a structure with some BuffHeal. You should add another function to try to garrison the hero in other useful structures as fortresses when it is attacked and already severely wounded. And also eject some unimportant garrisoned units if a nearby buffheal structure to leave room for the hero.

line 505: here also, you set all heroes to passive when "the hero" is slighly hurt, and never change back their stance (except what was already done when starting an attack). So all your other heroes will be useless when you are attacked. Here also a TODO when the special class would be added.

comment:9 by elexis, 8 years ago

Milestone: BacklogAlpha 22

Thanks for your remarks mimo!

comment:10 by mimo, 7 years ago

Cc: Sandarac added

Sandarac, do you intent to work on the remaining issues on that patch in the near future? near future being before A22, so let's say in the next two months.

comment:11 by Sandarac, 7 years ago

Yes mimo.

comment:12 by mimo, 7 years ago

OK thanks for the quick answer

comment:13 by Sandarac, 7 years ago

Several of the points in comment:8 assumed that players could be in possession of more than one hero at a given time; since r12832 this has not been possible (players always have an EntityLimit of 1 for heroes and I don't believe there are plans to change this).

  • The stopMoving method is needed because without it, if the hero is in a UnitAi state like "COMBAT.CHASING", there is a chance that it will ignore the garrison order (At least this appears to be what happens).
  • Now when the hero's health falls below 70%, the garrison order will be sent only once (instead of over and over in the original patch), but if the hero's health falls below 40%, it will try once more to find a closer structure, ignoring healing this time (non-garrisonable units will still try to find the closest base). A unit will be ejected if there is no room. These checks have been moved to a gameTypeManager, along with wonder-specific events.
  • The AI will now target enemy heroes in Regicide, and it will not garrison the hero on a ship.
  • The AI will assign some healers as guards to the hero. It will try to keep around four healers guarding the hero. I tested this fairly thoroughly, but if the hero is garrisoned (or has a different access value) then units cannot be assigned to guard the hero. I guess the gameTypeManager could check events.UnGarrison and then assign all units that have the "regicideHealer" role to guard the hero if they are not already.
  • At some point Champion units should be assigned as guards as well.
  • I know that UnitAI's canGuard function checks if the entity is already guarded as well as the entity template, but just a template check should be enough for the AI (there likely won't be extensive guarding).

comment:14 by Sandarac, 7 years ago

Keywords: rfc added

comment:15 by mimo, 7 years ago

Thanks sandarac for the patch. That's quite a complete patch, but also complex to review. I think in general it is better to provide small well-contained patches rather than such a big one. For example, during your dev process, i guess you started by creating the new gametypeManager and moved some already existing code inside it before adding new functionnalities. Such a patch would already have been really useful as easy to review, instead of the complete one with everything mixed. Also the part about guard seems to be self-contained and could make a second patch by itself once the infrastructure changes have been made. Such small patches would really ease (and fasten) the review process, and as i'm the only one reviewing ai patches currently, i would really appreciate them :-) (at least for future patches if you can't cut this one)

Concerning the comment about several heroes, it's true that in vanilla game, we have only one hero, but this limit can easily be modified and we will certainly see some mods without it in the future. So if we can easily take it into account aready now, it's better to do it, otherwise just add a TODO.

Edit: i looked a bit at the changes in attackManager.js (the AI targeting the player with the nearest hero) and do not support this change because

  • it is too much cheating as the ai is not supposed to know where other players'heroes are. I agree that what is currently done (i.e. targeting the player with the nearest cc) is also cheating, but here the cc info is only used as a representation of the enemy base which is in principle easily obtainable by a human player with just a bit of scouting, while the location of the hero is much more difficult to obtain
  • the hero can move, so you can exploit such a feature (moving your hero to the extremity of the map or on an isolated island) to avoid being attacked by the AIs.

I've not yet had time to look at the other features. Will comment on them when done.

Last edited 7 years ago by mimo (previous) (diff)

by Sandarac, 7 years ago

comment:16 by Sandarac, 7 years ago

Ok, yes I should have split the patch up. This new patch moves the wonder and regicide code to the gameTypeManager, and includes the improvements for when the hero is attacked. I removed the changes to attackManager.

I added a TODO for the "multiple heroes in Regicide" issue, and I made a function that checks the status of the hero and reduces some of the duplication in assignUnits of attackPlan. Also, in assignUnits, the variable added is not used and the return value is not needed, so I removed them. In defenseManager, I made a change to prevent the hero from being assigned as a defense unit.

comment:17 by mimo, 7 years ago

Hi sandarac, i've been able to go through your latest patch.

attackPlan changes: the changes are good, except that I would keep the "return added" in assignUnits. It is not used right now, but it may be useful to know that a unit has been added for some future use (and in fact this info was used some time ago). otherwise, line 616, i would keep

if (!ent.hasClass("Cavalry") || !this.isAvailableUnit(gameState, ent))

which is more analoguous to what is done in other blocks and allow to remove the third argument of isAvailable. and lines 852 and followings can be rewritten

 if (wondermode)
   targets = ...
 else if (regicide)
   targets = ...
 if (targets && targets.hasEntities())
   return targets;

defenseManager line 634 you could have function(gameState, unit, emergency=false) and then remove the third argument of line 533 lines 651 to 656 can be replaced by

if (m.GetLandAccess(gameState, ent) !== unitAccess)

lines 672 remove the else (not needed after return) the logic in lines 675-677 won't work as the unload is an Engine command which will only be done on the next turn, so the number of garrisoned units will only be updated the next turn the AI will run. In addition, it may be dangerous to try to update it like that as numberOfGarrisonedUnits contains the units really garrisoned and those being garrisoned by the garrison manager. So it could happen that nearest.garrisoned() has no units while nearest is full (garrisoned will only be updated on the next turn). You should count the pop cap of units you can unload (i.e. which are already in the garrisoned() array) and if that's smaller than what is needed to garrison your hero, go to the next structure. line 679: it think it should be garrisonManager.garrison(gameState, unit, nearest, (nearest.buffHeal() ? "protection" : "emergency")); or you should change line 225 of garrisonManager to return enemiesAround && ent.needsHeal();

gameTypeManager lines 21-38 this loop could have a if (wonder) or maybe rewrite the function with a switch(gameState.getGameType()) line 31 could become " if (builders)"

line 72 the first condition (plan == -2 line 92 could be cut after
plan == -3) is always satisfied here
to avoid too long line

line 110 putting the hero in passive stance is a bit dangereous if never put back to a standard stance. Maybe having some check which, every 50 turns or so, put back the hero in its normal stance if its health > 0.7 ?

One important check to do is the serialization of the regicide. Did you try to save a game and reload it without problems

by Sandarac, 7 years ago

Fixes.

comment:18 by Sandarac, 7 years ago

Hello mimo, thank you for the review, this new patch should fix all of the remarks.

I also added

if (m.GetLandAccess(gameState, ent) !== unitAccess)

to the garrisonSiegeUnit function in defenseManager.

For garrisonAttackedUnit, the function will not select a structure if it is full according to the garrisonManager and there are no units physically in the structure (i.e. in ent.garrisoned()) to eject. But if it is full and there is at least one unit in ent.garrisoned(), it will eject one to make room. I believe that the population cost is irrelevant when it comes to space taken up in a garrison holder (for example, many heroes have a population cost of two, but they always take up one spot in a garrison holder). So in any case only one unit will need to be ejected.

I do not believe there are serialization issues. I tried reloading several save games without issue, and in any case I followed what is done in the other Petra managers for serialization/deserialization.

Edit: After some more testing, the changes in the new patch do not really help the garrison issue (the number of garrisoned units isn't updated in time after a unit is ejected, so the hero doesn't end up being garrisoned, which you pointed out). And even if the AI waits for one turn, there is no guarantee that some other unit won't be assigned to the new available spot in the holder during that turn instead of the hero, so I'm not sure about adding the "eject" feature.

Last edited 7 years ago by Sandarac (previous) (diff)

comment:19 by mimo, 7 years ago

In 19085:

Petra: improve regicide support, patch by Sandarac, refs #4142
add a gametypeManager and improve garrisoning of hurt hero

comment:20 by mimo, 7 years ago

Thanks for the last patch. As it looks good now, i've commited it, with the few changes listed below. Let me know if you agree with them. changes compared to your patch in gametypemanager line 71-72, replaced emergency by this.heroGarrisonEmergency line 77 leaveGarrison was not supposed to be called outside the garrisonManager as it would then require to also update the holderList. I've added a new function cancelGarrison.

possible cleanings in gametypeManager for future patches

  • when regicide, cache in a gametypemanager Map the hero id and its garrsion emergency, with a generic name so that the same array can be reused in possible future herocide, regicide and whatever xxxxcide mode, and instead of testing if the entity has the Hero class in the various places, test that it is in this cached Map. That would incidentally solve the case of a mod allowing several heroes, as only the initial ones would be cached and thus taken into account for the herocide, and if we would like to include the new ones, it would be enough to update the Map in gametypeManager.checkevents when a new one is created.
  • caching the stance when changed in the Map would allow to revert the stance (line 127-129) only if needed, or maybe we should add a common-api function getStance.

and in garrisonManager

  • possibly improve the naming of the garrison cases (for example "protection" -> "healing", and "emergency" -> "protection"), and the Siege+!Melee could go in the emergency case when garrisoned from garrisonSiegeUnit, so removing the need to have its special case in the keepGarrisoned function.

Concerning serialization, i didn't mean that there was a problem :) (in fact the code looks good and i did a quick test myself), i just wanted to be sure you also tested it and that i didn't miss anything.

comment:21 by elexis, 7 years ago

Offtopic, but r19050 setAccessIndices instead of SetAccessIndices as pointed out by Sandarac.

comment:22 by mimo, 7 years ago

already fixed in r19080 :)

by Sandarac, 7 years ago

Healers will be assigned to guard the hero.

comment:23 by Sandarac, 7 years ago

The changes were all good; in particular I should have realized I could have used this.heroGarrisonEmergency = target.healthLevel() < 0.4 instead of another variable.

For the Map object in gameTypeManger, if I understand correctly, the map's keys would be hero IDs (not entities?), and the values would be objects holding garrisonEmergency and the heroes' current stances? i.e. on init: this.heroes.set(heroEnt.id(), { "heroGarrisonEmergency": false, "stance": "aggressive" }); ?

This new patch includes the "healer guards" portion. The last three comments in comment:13 still hold for this patch, and so if the hero happens to be garrisoned when a new healer is spawned (or if the AccessValue is different), it cannot be added as a guard, so the counter increases but no new guards are assigned.

Also, there was an error in my previous patch. In the for (let evt of events.Garrison) loop in gameTypeManager, it did not check that the hero entity belonged to the AI player, so I fixed that.

Last edited 7 years ago by Sandarac (previous) (diff)

in reply to:  23 comment:24 by mimo, 7 years ago

Replying to Sandarac:

For the Map object in gameTypeManger, if I understand correctly, the map's keys would be hero IDs (not entities?), and the values would be objects holding garrisonEmergency and the heroes' current stances? i.e. on init: this.heroes.set(heroEnt.id(), { "heroGarrisonEmergency": false, "stance": "aggressive" }); ?

Yes that is what i had in mind, but that could be improved if you have better idea. We could also possibly add there the list of guards (which should just become a list of ids, see following comments).

Concerning your new patch, i had no time to look at it in details, but here are a few suggestions entity.js

  • move canGuard above, after canGarrison where all internal functions are, while the last part is for the Engine functions.

gametypeManager

  • may be better if this.heroGuards only contains the guards id and not the full entity. That would be much simpler for serialization (i'm not sure having entities will work on deserialization as all entities are recreated from the AIInterface info) and i'm also not sure how indexOf works with such a complex object (could lead to poor performance).
  • lines 108-113: what will happen to the healer if not in the same accessibility? requiring a transport may be more efficient than training a new healer, specially in naval map where you may not have a temple in the island where you have the hero. Same thing when a hero is garrisoned in a ship and its guards are removed.

A temporary solution would to check every 50 turns (in the block 184-189) if you have some healer on the same accessibility than the hero and not guarding it, and make them guard it, and if not accessible, require a transport.

  • assignHealers: this function is not really assigning healers, but training new healers

the AI currently tries to build a temple only at phase 3 (or if it's ready to update to phase 3 but needs town structures). You may want to start building temples already at phase 2 when in regicide (see buildtemple in headquarter.js).

  • line 168 I very rarely play with healer, so i'm not good at guessing the needed number, but something like 2 + Math.round(this.Config.personality.defensive * 2)

would look better.

by Sandarac, 7 years ago

Update and apply mimo's remarks, except without Maps.

comment:25 by mimo, 7 years ago

Looks good. I will commit it. There are nonetheless a few things which can be improved:

  • needed: before assigning a guard to a hero, check that it has not already required a transport (ent.getMetadata(PlayerID, "transport") !== undefined if waiting for a transport, !ent.position() if inside a transport). Maybe check that on top of the assignGuardToRegicideHero function.
  • nice to have: line 190 of gametypeManager have a hardcoded healer template. There are already a lot of such hardcoded template names, so not a real problem to leave it here, but it would be nicer to avoid it by having some function which loops on the productionQueues for entities with the Healer class. We already have such functions (see findBestTrainableUnits in headquarters or getBestShip in navalManager).

and thanks for fixing my typo in cancelGarrison.

Edit: while commiting, i've added the missing check on ent.getMetadata(PlayerID, "transport")

Last edited 7 years ago by mimo (previous) (diff)

comment:26 by mimo, 7 years ago

In 19089:

petra: add healer guards in regicide mode, patch by Sandarac, refs #4142

comment:27 by elexis, 7 years ago

I don't know how feasible this is (and it's cheap to give feedback after the commit), but perhaps the code that is only relevant to regicide mode could be moved to a new file (I imagine the file could become confusing if we add more gamemodes).

comment:28 by mimo, 7 years ago

r19085 added a new gametypeManager file to deal with the specific code of the different gametypes (currently wonder and regicide). I think that's sufficient for the time being. If this file grow too much, it will be easy to split it later.

by Sandarac, 7 years ago

Handle multiple heroes in regicide, and cache entities important for specific victory conditions.

comment:29 by Sandarac, 7 years ago

Description: modified (diff)

In this patch, the heroes spawned at the start of the game in regicide will be considered the "critical" ones necessary for the game, and the AI will assign healers to each of them. A generic Map is used that can be utilized for other victory conditions.

But there is a small issue: when init is called for the various managers at the start of a new game (turn 0), the trigger script has not yet spawned the hero unit(s). So the "init" of gameTypeManager (caching the gameType-critical ents like heroes in regicide) has to be called on the first turn. In this patch this issue is handled in update of gameTypeManager.js.

There is a possibility that the AI will train too many healers relative to its population cap if there are many units that it needs to guard; I guess this could be avoided by setting the max number of healers/guards in total that it can train.

Note: See TracTickets for help on using tickets.