Opened 7 years ago

Closed 7 years ago

#4431 closed enhancement (fixed)

[PATCH] Petra AI - Respond to Ally / Neutral Requests

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

Description (last modified by Sandarac)

Currently if you try to ally or become neutral with an AI player it will not respond (you will become allies/neutral with them, but they will not change their stance towards you). The AI should respond to these "requests", but overall, AI players should be reluctant to accept them, and they should decline a request if it would result in the end of the game. In some cases where the AI chooses to accept, it should demand a tribute of some resource. Also, the AI should probably consider only one ally/neutral request during the game.

Attachments (8)

petra_ally_requests_v0.1.patch (9.3 KB ) - added by Sandarac 7 years ago.
Petra will respond to ally requests.
petra_ally_requests_v0.2.patch (9.8 KB ) - added by Sandarac 7 years ago.
Fix issues.
petra_ally_requests_v0.3.patch (8.9 KB ) - added by Sandarac 7 years ago.
Update, and only consider ally requests from neutrals for now.
petra_ally_requests_v0.4.patch (11.0 KB ) - added by Sandarac 7 years ago.
Fix issues in diplomacyManager pointed out by mimo.
petra_ally_requests_v0.5.patch (10.5 KB ) - added by Sandarac 7 years ago.
Update.
4431_petra_neutral_requests_v0.1.patch (13.7 KB ) - added by Sandarac 7 years ago.
Handle neutral requests from enemies.
4431_petra_neutral_requests_v0.2.patch (16.1 KB ) - added by Sandarac 7 years ago.
Apply mimo's comments.
4431_petra_neutral_requests_v0.3.patch (16.0 KB ) - added by Sandarac 7 years ago.
Remove unneeded variable.

Download all attachments as: .zip

Change History (35)

by Sandarac, 7 years ago

Petra will respond to ally requests.

comment:1 by Sandarac, 7 years ago

  • The messages.js change is needed in order to display PMs sent by AI players.
  • When an AI accepts an ally request, the allyRequests value for that player will be set to -2 (instead of -1 for an initial decline), so that if the player tries to ally again with the AI player after a failed alliance, the chat message will be different.
  • For any ally request, if the requesting player is not the same civ as the AI, there is a random chance that the AI will decline without considering any other factors.
  • The AI will demand a tribute from players with less entities, using pickMostNeededResources to determine which resource should be tributed.

comment:2 by mimo, 7 years ago

Thanks for the patch.

messages I do not know very much about messages. Would such a change also work on MP with AIs? have you tested it ? (elexis could maybe know).

gamestate it would be better to rename this function getPlayerCiv() and modify civ() to call getPlayerCiv without argument (and in a next cleaning patch replace all occurences of civ() by getPlayerCiv()).

chatHelper stance is already used for units behaviour, and may be misleading here. diplomaticStance? or any other better name to go in the direction of #4326, define rather the three possible messages in a m.chatNewDiplomacyMessages object with the 3 values of newStance as properties. same thing with the possible answer to chatAnswerRequestAlly (when multiple choices as for the case 3, use an array and you can also use m.PickRandom which is in common-api/utils.js (to stay in the mood of #4326). Then the responses 0 to 4 would be more readable as strings.

diplomacyManager line 95 and following: "newAllyData" does not seem appropriate. Why not "request"? and I don't like the fact that it can be an object or an integer. Why not add newAllyData.status which would have your -1 or -2 values? so you would not need the set in lines 109 and 112, only changing the status property. Finally, i think it would make sense if line 113 was player dependent, so another property of the newAllyData object. and what does line 112 means exactly: the other player has turned back against you after you have asked him for a tribute? in that case it should be catched in the loop on diplomacyChanged events because if it never sent you any tribute, you will never pass in line 112. lines 146-149: I suppose you can't have such diplomacyChanged events in case the teams are locked? but could be safer to only do the loop when this is not the case. Do you intent to also add a handlePeaceRequest when an enemy turns to neutral? (in an future patch). line 229-230 better replace the integer response by strings (see previous comment) and if using the status as proposed above, in the following "newAllyData !== undefined" -> !newAllyData and "newAllyData === -1" -> "newAllyData.status === -1" Lines 235-236 would be more readable if the three && conditions were on the same line and line 236 could also have a proba to decline even if same civ (with random > 0.8 or so) line 247 the first two conditions are necessary fulfilled here, so useless For code readability, you could add a comment after 252 saying that "else we are waiting to complete the tribute" lines 254-269: I'm not convinced that accepting to ally preferentially with stronger opponents is the best strategy, because at the end you will find yourself alone against a stronger enemy. Usually, it is best to ally with weaker players against the strongest one. lines 227-271 as before newAllyData would be better as request lines 274-283 as bebore newStance would be better as newDiplomaticStance

And then, but for another patch, if we implement all that, the AI should also be able to request some alliances

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

by Sandarac, 7 years ago

Fix issues.

comment:3 by Sandarac, 7 years ago

Thank you mimo for your help with my patches.

I will have to do some testing, but I think the messages.js change will work in MP. The || "local" is needed in order for PMs to be visible in SP because AIs are not assigned GUIDs in singleplayer.

For chatHelper, I followed what is done in #4326, and I used PickRandom in m.chatNewDiplomacyMessages in case more variety is added later.

I don't think the AI should accept more than one ally request per player per game, as this may lead to exploits. As such, I removed the use of a lapse timer.

I was thinking that handleNeutralRequest should be added, and I guess the AI would be more likely to consider neutral requests than ally ones.

in reply to:  2 comment:4 by elexis, 7 years ago

Replying to mimo:

I do not know very much about messages. Would such a change also work on MP with AIs? have you tested it ? (elexis could maybe know).

Sandarac changed the previous messages.js diff to the current proposal after a discussion on irc.

Since all observers have the playerID -1, chat handling based on playerID was conceptually broken (#3270) and replaced with player GUID parsing for private messages (thus being able to distinguish observers). The function GetPlayerGUID introduced r17771 utilized for that returns either the client identifier (GUID) or the empty string. In Singleplayer however, the gamesetup/session use local as a placeholder identifier in g_PlayerAssignments. I didn't want to transport this hardcoding to C++, but it is a possibility alternative to the proposal (or adding a new JS only function that returns GetPlayerGUID()
"local" to avoid some of the other "local" checks too).
Version 1, edited 7 years ago by elexis (previous) (next) (diff)

in reply to:  5 comment:6 by mimo, 7 years ago

Replying to elexis:

As in https://github.com/0ad/0ad/compare/master...elexis1:GetPlayerGUID (.diff)

yes, looks good and better than fixing it at the different places.

comment:7 by mimo, 7 years ago

Thanks for the updated patch. The code looks good now.

messages.js Hopefully, the needed changed in the playerGUID will be adressed following comment 6 in a separate commit.

chatHelper.js line 28: for me, "consider" sounds as it is not yet certain that the alliance will be accepted after the tribute is received. So we could use another word ("accept" ?).

diplomacyManager.js lines 16-17 and following: in fact, status could be a string instead of an integer for better readability? lines 257 numberOfEnemies < 3 could be changed to numberOfEnemies < 2 + numberOfMutualAllies

I do not understand how the "waitingForTribute" message can be sent? because if the other player changes its stance again, the alliance will be broken. So you should take the case of waitingForTribute into account in lines 150-155 or do it differently. My proposition would be something like

  • start a timer (1 mn or something like that) which is reset each time we receive part of the tribute from it
  • at the end of this first timer, sent the waitForTribute message and start a new 1 mn timer (also reset at each tribute received)
  • at the end of the second timer, reject the alliance
  • and accept it as soon as the tribute is complete

In addition, we should make a distinction depending on what was our previous stance with the player, and in particular if we were enemies, we should check how to cancel possible attacks already launched by the attackManager. So a possible simpler approach is to only respond to request from an enemy asking for neutral and from a neutral asking for alliance: this second case being what is done in this ticket, the former one can be a next handleNeutralRequest ticket (which will have to deal with canceling attacks), and if an enemy wants to become directly allied, we respond it to become neutral first? if such simplified approach is taken, and thus we only have to study alliance proposition from neutral in this ticket, we can be more inclined to accept depending on the number of enemies we already have, i.e. the different cuts on math.random() and the amount of tribute requested could depend on the number of enemies we have. That would give more diversity in the AI response. All these propositions are mainly to start a discussion on how to best implement it.

in reply to:  7 comment:8 by Sandarac, 7 years ago

I do not understand how the "waitingForTribute" message can be sent? because if the other player changes its stance again, the alliance will be broken.

I should have been more clear; this message is sent if the player repeatedly attempted to become allies (i.e. selects the button multiple times), but your idea of sending it when the player does not send tributes within a minute and using a timer makes more sense.

In addition, we should make a distinction depending on what was our previous stance with the player, and in particular if we were enemies, we should check how to cancel possible attacks already launched by the attackManager. So a possible simpler approach is to only respond to request from an enemy asking for neutral and from a neutral asking for alliance: this second case being what is done in this ticket, the former one can be a next handleNeutralRequest ticket (which will have to deal with canceling attacks), and if an enemy wants to become directly allied, we respond it to become neutral first? if such simplified approach is taken, and thus we only have to study alliance proposition from neutral in this ticket, we can be more inclined to accept depending on the number of enemies we already have, i.e. the different cuts on math.random() and the amount of tribute requested could depend on the number of enemies we have. That would give more diversity in the AI response.

I think this makes sense, although it might seem strange to have to go through a two-step process in order to become allies with an AI (but then it also might be strange to become allies with one request or a single resource tribute, especially if starting resources are high). When it comes to attackManager, I thought that the check for currentEnemyPlayer (i.e. line 257 in the most recent patch) was enough to deal with this issue? I thought it would make sense to outright decline any ally or neutral requests from the currently targeted player. If not, would calling attackPlan.Abort() for every plan targeting the requesting player be enough?

In any case, I think the current patch can be adapted to be for just ally requests from neutrals, and then neutral requests from enemies. For ally requests from neutrals, the AI should accept more readily if it has a relatively large number of non-allies.

comment:9 by mimo, 7 years ago

Just to be clear, the proposition to only allow neutral->ally in this patch, and then enemy->neutral in a next one is only in order to have small patches easy to be reviewed. Nothing prevent us from adding later a direct step enemy->ally (should be simple enough once the first two steps are done, as the needed changes will mainly be in the requirements to accept the deal or not). To cancel the attacks, i agree, attackPlan.Abort() for the attacks which have already started, and changing the attack.targetPlayer for those in preparation should be sufficient.

comment:10 by mimo, 7 years ago

In 19104:

petra cleanup, taken from sandarac, refs #4431

by Sandarac, 7 years ago

Update, and only consider ally requests from neutrals for now.

in reply to:  5 comment:11 by mimo, 7 years ago

Replying to elexis:

As in https://github.com/0ad/0ad/compare/master...elexis1:GetPlayerGUID (.diff)

In fact while testing with a replay, I noticed that we don't get any chat messages in replay (at least those from the ai), and i guess it would be the same also when being an observer during a game. That seems to be because of the g_IsChatAddressee which tests on Engine.GetPlayerID(). I think we should also add a function for such purpose which would return

Engine.GetPlayerID() >= 0 ? Engine.GetPlayerID() : g_ViewedPlayer;

and use it here.

comment:12 by mimo, 7 years ago

In diplomacyManager: the continue line 103 is dangerous as ideally we only want to leave the current block, not necessarilly the loop in case some other treatment is done later. I would rather put

if (request.status === "waitingForTribute")
{
 ... add your code here
}

lines 254: shouldn't we combine it with line 271? we should not be likely to decline if we have too many enemies.

line 262: should it really be automatic that we reject some alliance if already requested? if it has not payed the tribute for lack of resource for exemple? so maybe, when an alliance is rejected because the tribute was not paid, we could delete the request from the map, so that the player can try again later. (Such a delete could go in line 325).

line 320 I think data.status !== "declinedRequest" && data.status !== "allianceBroken" should be replaced by data.status === "waitingForTribute"

the full block lines 318-338 could possibly go in a function to keep the update function as a steeringfunction.

Souldn't we, during init, loop on all the initial allies and create a request with request.status = "accepted", so that if it breaks the original alliance, we have a record of it.

by Sandarac, 7 years ago

Fix issues in diplomacyManager pointed out by mimo.

comment:13 by Sandarac, 7 years ago

Hello mimo, I have updated the patch.

In this patch diplomacyManager.init() is only called in gameAnalysis, and not during deserialization in headquarters.js, in order to get just the initial allies.

I think that the process in this patch works well (as the AI now factors in the number of current enemies it has quite well). However, when testing, it was clear that the first Math.random() could possibly be a tad more severe (but I think it is a matter of preference).

I was also thinking that there could be a chance that, upon receiving all of the requested tributes, the AI might choose not to ally with the player and then send a corresponding message. But I'm not sure about this.

Also, there is a typo in r19109 in chatHelper.js line 70; the property of m.launchAttackMessages is "hugeAttack" but line 70 can try to access "HugeAttack".

comment:14 by elexis, 7 years ago

In 19112:

Return "local" from GetPlayerGUID to avoid duplicate checks and allow AIs to send private messages to the single player. Refs #4431.

Reviewed By: mimo
Differential Revision: https://code.wildfiregames.com/D38

comment:15 by mimo, 7 years ago

Hi sandarac, thanks for spotting the typo in r19109

concerning the patch, i've still a few remarks:

  • changes in messages.js no more needed as elexis commited r19112
  • init function: you restrict it to mutual ally, but what about those which are allied to us but not mutual? I think we should call a handleAllyRequest to decide what to do; I don't think that is a situation which will happen in many maps, but the code should nonetheless foresee it.
  • any particular reason why the check line 261 is on ExclusiveMutualAllies on not MutualAllies?
  • the new function allyRequestsTributesCheck will certainly be reused for other cases (enemy->neutral), so could be renamed to requestsTributesCheck (and even checkRequestedTributes would sound better to me)

When these small things are fixed, i will commit it.

I agree that the exact cut values for these diplomacy deals will certainly need some tunings, but can be done later after we get some feedback (the Math.random as you said, but also the amount of tribute requested). And as you, I'm not sure about adding such an alliance refusal after having received the requested tribute. So let's not do it for the time being.

by Sandarac, 7 years ago

Update.

comment:16 by Sandarac, 7 years ago

I was wondering, for neutral requests, should they be combined with the current this.allyRequests into a new this.diplomacyRequests with an additional requestType property? This would work well with f.e. the loop in checkRequestedTributes. Or should they be two separate Maps?

Also (for a different patch), I was thinking that every 100 turns, if an AI player has more enemies than mutual allies, it would have a random chance (maybe Math.random() > 0.8) to send a neutral/ally request to a random player with less units, and preferentially to a player with the same civ (if it has not sent a request to a player already). What do you think?

comment:17 by mimo, 7 years ago

In 19116:

petra: start implementing responses to alliance requests, patch by sandarac, refs #4431

in reply to:  16 comment:18 by mimo, 7 years ago

Thanks for the patch. I've changed two things while commiting

  • name of message: chatAnswerAllyRequestMessages -> answerAllyRequestMessages (was already changed for other messages to have shorter names)
  • in init function, !gameState.isPlayerAlly(i) was useless

And checking again the patch, there are a few things which can be improved:

  • in gameAnalysis (in startingStategy.js) the calls to diplomacyManager.init and diplomacyManager.diplomacyCheck should be merged.
  • we could also be more prone to make an ally if some common enemies.
  • in diplomacyManager.checkEvents, there are now several possible diplomacy changes (in the tributeExchanged loop, in lastManStandingCheck and in the DiplomacyChanged loop. We should keep a track of all that to not do contradictory actions, for example turning against player i in lastManStandingCheck, and then answering positively to a peace request. I suppose that at the end of lastManStandingCheck, we should reject any possible request from the player we choose as enemy.

Replying to Sandarac:

I was wondering, for neutral requests, should they be combined with the current this.allyRequests into a new this.diplomacyRequests with an additional requestType property? This would work well with f.e. the loop in checkRequestedTributes. Or should they be two separate Maps?

Yes, why not. We may then need to add another property in the request to indicate its type (ally or neutral).

Also (for a different patch), I was thinking that every 100 turns, if an AI player has more enemies than mutual allies, it would have a random chance (maybe Math.random() > 0.8) to send a neutral/ally request to a random player with less units, and preferentially to a player with the same civ (if it has not sent a request to a player already). What do you think?

Yes good idea.

by Sandarac, 7 years ago

Handle neutral requests from enemies.

comment:19 by Sandarac, 7 years ago

Hello mimo, this new patch deals with neutral requests from enemies. The factors that are considered when a neutral request is received are similar to the neutral-to-ally requests, but tributes are required for neutral requests (the actual amount required is smaller).

I moved the call to lastManStandingCheck to init in diplomacyManager.js, and removed the diplomacyCheck function that basically acted as a duplicate init function. Also, the AI will deal with any diplomacy requests it may be handling if it becomes enemies with a player in lastManStandingCheck (it will cancel them).

this.allyRequests is now this.diplomacyRequests and this Map also now contains a requestType.

I have been trying to work on a "AI sends diplomacy requests" feature; if the AI sends, for example, an ally request to another AI player, it will change its diplomacy stance, which will be handled by the diplomacyManager of the other AI. If the other AI does not accept within two turns (i.e. does not change its diplomacy in response), the first AI will switch back to its original stance. But I have noticed that the AI does not have direct access to whether any one player is an AI or not (it is not in the SharedScript); my plan is to add cmpPlayer.isAI() to the playerData returned by GuiInterface.GetSimulationState(), so that this will be accessible in the shared script. With this, the AI can send a chat message to a human player to request that they change their diplomacy, and they can use the mentioned approach to deal with AI players.

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

in reply to:  19 ; comment:20 by mimo, 7 years ago

Hi Sandarac, thanks for this new patch! A few quick comments:

Changes in attackManager:

for the upcoming attacks, it should be enough to only have

if (attack.targetPlayer === player)
	attack.targetPlayer = undefined;

there is no need to abort it.

Changes in chatHelper: I'm not sure, but could be clearer to have answerDiplomacyRequestMessages[requestType][response]?

Changes in diplomacyManager: in init, we should add the case if player is neutral and we are enemy, handleDiplomacyRequest("neutral") line 186 we must foresee the case when a player which has requested to be neutral turns back to enemy (maybe just delete the request in that case) lines 277-279: if we are really outnumbered by our enemies, shouldn't we accept more easily a neutral peace? line 332 ".entries()" is not necessary

Replying to Sandarac:

I have been trying to work on a "AI sends diplomacy requests" feature; if the AI sends, for example, an ally request to another AI player, it will change its diplomacy stance, which will be handled by the diplomacyManager of the other AI. If the other AI does not accept within two turns (i.e. does not change its diplomacy in response), the first AI will switch back to its original stance. But I have noticed that the AI does not have direct access to whether any one player is an AI or not (it is not in the SharedScript); my plan is to add cmpPlayer.isAI() to the playerData returned by GuiInterface.GetSimulationState(), so that this will be accessible in the shared script. With this, the AI can send a chat message to a human player to request that they change their diplomacy, and they can use the mentioned approach to deal with AI players.

But why would you need to differentiate if the player is AI or not? You could send the chat message and wait a bit in both cases (even if you expect a quicker answer in the case of AI). Futhermore, iirc the ai is run every 8 turns currently, so 2 ai turns means 3.2s in SP (turn = 0.2) and 8s in MP (turn = 0.5), but these turn durations may change in the future. So better test on the elapsed time rather than the number of turns. We can say if you don't get the answer after 20 to 30s (to let the time for a human player to react), you revert the diplomatic stance. The only difficulty will be to understand the answer from the AI if it requests a tribute (that is maybe why you needed to know if it was an AI in the first place :-) ), and for that, we could just add a new AI-made event "TributeRequested" in SharedScript.events which would then be accessible by all AIs (that should work, but i've never tested it) while humans will receive the request by chat. It will remain the problem to allow a human player to require a tribute before accepting. But for that, i think a redesign of the diplomacy window and interactions will be needed.

by Sandarac, 7 years ago

Apply mimo's comments.

in reply to:  20 comment:21 by Sandarac, 7 years ago

But why would you need to differentiate if the player is AI or not? You could send the chat message and wait a bit in both cases (even if you expect a quicker answer in the case of AI).

But because the AI can only send "diplomacy requests" by actually changing its stance with the other player, I thought it could lead to exploits if the AI did not switch its stance back quickly if it did not receive a response. For example, the AI might not be able to fight back against a player it had changed its stance with, but this doesn't seem to be the case.

In this patch I also changed the tribute timer to only be reset if the AI received a tribute of the specified type (before it was any tribute).

The only difficulty will be to understand the answer from the AI if it requests a tribute (that is maybe why you needed to know if it was an AI in the first place :-) ), and for that, we could just add a new AI-made event "TributeRequested?" in SharedScript?.events which would then be accessible by all AIs (that should work, but i've never tested it) while humans will receive the request by chat.

To clarify, does this mean that Engine.BroadcastMessage(MT_TributeRequested, ...) would have to be sent from diplomacyManager.js (when an AI responds to another AI's diplomacy request)?

comment:22 by mimo, 7 years ago

The new patch looks good now, i've just a style comment in attackManager, the first loop on upcoming attacks could be

for (let attack of this.upcomingAttacks[attackType])
 ...

as we no more need the i index?

There is currently a patch pending from Gallaecio in Phabricator to fix some english wordings on the chatHelper strings. I'll commit that one after, that will leave me some time to test it :-)

Concerning the diplomacyRequests (with a TributeRequested), you should not go to a BroadcastMessage which is quite slow (and not really useful if only for the AI), but you could add a new command in helpers/Command.js which would only call the AIInterface pushEvent with a homemade type and msg.

But more generally, for the diplomacy interactions between human and AIs, i think we should first redesign the diplomacy in the game and the way we interact in the diplomacyWindow. For example, i've the feeling that having only mutual-states for Ally, Neutral and Enemy would have no effect on the gameplay, but would ease a lot the possible interactions (as this would decrease a lot the combinatorial). The current A-N-E would then not show our diplomatic stance, but would be used to request a change (of course automatic when going towards E, but needing an agreement in the other direction), ... What's your feeling about it?

in reply to:  22 comment:23 by Sandarac, 7 years ago

But more generally, for the diplomacy interactions between human and AIs, i think we should first redesign the diplomacy in the game and the way we interact in the diplomacyWindow. For example, i've the feeling that having only mutual-states for Ally, Neutral and Enemy would have no effect on the gameplay, but would ease a lot the possible interactions (as this would decrease a lot the combinatorial). The current A-N-E would then not show our diplomatic stance, but would be used to request a change (of course automatic when going towards E, but needing an agreement in the other direction), ... What's your feeling about it?

I think this is a good idea, as it is a little confusing currently that you can change your diplomacy stance to "ally" with a player even if they do not have the same stance with you. It would be also nice, as you already mentioned, to allow the player to demand a tribute from an AI requesting an alliance through the diplomacy window. I guess this would be done in a separate ticket.

In any case, it would also be nice to prevent diplomacy messages from being sent multiple times. #3198 is related to this.

by Sandarac, 7 years ago

Remove unneeded variable.

comment:24 by mimo, 7 years ago

In 19161:

petra: respond to neutral requests from enemies, patch by Sandarac, refs #4431

comment:25 by mimo, 7 years ago

Thanks for the patch.

Concerning the possible changes on diplomacy, it depends on your motivation. There is a lot to do, and reaching agreement on what different people expect it to be may be difficult. I've done a quick survey in the team about the changes discussed above, and nobody objected (though the number of answers i got were limited). So it's up to you :-)

comment:26 by elexis, 7 years ago

Keywords: rfc removed

comment:27 by Sandarac, 7 years ago

Description: modified (diff)
Milestone: Work In ProgressAlpha 22
Resolution: fixed
Status: newclosed

Largely complete with r19161, and r19654 gave Petra the ability to send diplomacy requests to other players.

Note: See TracTickets for help on using tickets.