Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement "Make Peace With" TradeOffer Type #11448

Closed

Conversation

TommasoPetrolito
Copy link
Contributor

@TommasoPetrolito TommasoPetrolito commented Apr 11, 2024

This PR aims to introduce the "make peace with /civname/" proposal within trade.
I don't know if there were critical aspects that I might have missed with regard to this feature, I was wondering why this was not implemented before, then if I am missing some super important critical details please let me know.

The need for this feature that is present in Civ5 is documented here:
#4697
In particular

Diplomacy:

  • A civ can ask another civ to make peace with a third civ

The implementation is basically the same of WarDeclaration in terms of adding civs to offers, evaluating costs etc.

  • ADDING CORRECT CIVS TO OFFERS
    There is one new minor change affecting WarDeclaration as well in TradeLogic.kt:
    val civsWeBothKnow = civInfo.getDiplomacyManager(otherCivilization).getCommonKnownCivs()
    is used in this code change for both War and Peace declarations instead of previous:
    val civsWeBothKnow = otherCivsWeKnow .filter { otherCivilization.diplomacy.containsKey(it.civName) }
    otherCivsWeKnow is meant to provide a list of third civs (not including city-states and defeated civs) that is then used for the "Introduction" part, and in that case is fine.
    For Peace and War declaration in Civ5, if I remember well, you can propose to ask to declare war or make peace with major civs but also with city-states as well, then new code for PeaceDeclaration now fixes this point as well for WarDeclaration.
    Of course while WarDeclaration is then filtered for Civs not yet at war with (and with no current truce periods) to offer them, in the case of PeaceDeclaration instead we simply filter for Civs we are currently at war with.

  • TRADE EVALUATION
    I chose to reverse more or less the evaluation of WarDeclaration, then if the war-opponent is a Strong Threat the evaluated cost is low, while if the opponent is a Weak Threat the cost is high, because continuing the war is much more convenient for the trading-counterpart than accepting the offer.
    I basically used exactly the same scores, just reversed:
    Buy Cost (War)
    ThreatLevel.VeryLow -> 0 ThreatLevel.Low -> 0 ThreatLevel.Medium -> 100 ThreatLevel.High -> 500 ThreatLevel.VeryHigh -> 1000
    Buy Cost (Peace)
    ThreatLevel.VeryLow -> 1000 ThreatLevel.Low -> 500 ThreatLevel.Medium -> 100 ThreatLevel.High -> 0 ThreatLevel.VeryHigh -> 0
    Sell Cost (War)
    ThreatLevel.VeryLow -> 100 ThreatLevel.Low -> 250 ThreatLevel.Medium -> 500 ThreatLevel.High -> 1000 ThreatLevel.VeryHigh -> 10000 // declaring war to a super power is NO WAY
    Sell Cost (Peace)
    ThreatLevel.VeryLow -> 1000 // making peace with someone very weak, for just some turns is not a nightmare ThreatLevel.Low -> 750 ThreatLevel.Medium -> 500 ThreatLevel.High -> 250 ThreatLevel.VeryHigh -> 100
    Here I changed slightly the values after reversing the War cost, because declaring war to a super power basically can lead to disappear from the world, while a temporary 10-turns truce with a weak opponent is still something acceptable for a certain price.

  • EFFECTS
    Here is where the implementation differs the most from WarDeclaration, that is basically immediate in its effects and just leads to the War status for both and, starting from that moment, things just go like if the Civ accepting the offer of WarDeclaration (against a third civ) had directly declared war in the first place, nothing more.
    PeaceDeclaration is a bit more complicated, in fact, in my first tests of the implemented feature I faced the following issue: after just one turn the civ went back declaring war to the third civ, or the third civ declared war again on the offer-recipient civ.
    After some checking I realized that what I was missing was the side effects of an actual PeaceTreaty, in terms of duration and turnsToPeaceTreaty.

This was particularly evident with this feature enabled because if you offer the "Make the peace with"/PeaceDeclaration to the weak opponent, he will likely accept, but then the next turn the strong opponent will declare war immediately (making the whole trade exchange meaningless) because it is not the one who offered the PeaceTreaty and somehow then seamed to be not bound to wait by canDeclareWar check.

Going back to PeaceDeclaration TradeOffer, I needed to make it add "placeholder" PeaceTreaties TradeOffers to the trades inside the respective diplomacyManagers of the two civs engaged in the war (the civ accepting the PeaceDeclaration offer and the third civ with which it was in war).
Like this, the PeaceTreaty duration effects are propagated correctly when a PeaceDeclaration is traded as well.
After this addition the feature works properly in the tests done. The civ declares peace and does not come back declaring war the next turn, nor the opponent declares war back immediately, and its allied city-states declare peace as well, everything fine, I would say.

I don't know if I am missing other relevant super critical aspects then again please let me know if I am.
Thank you in advance for any feedback.

@TommasoPetrolito
Copy link
Contributor Author

TommasoPetrolito commented Apr 11, 2024

I'm wondering...

One thing that I might be missing and that might make sense: maybe we could define the offer "ACCEPTABILITY" as well.

Implementing something like "peaceDeclaratiom offer can only be offered if the recipient is the higher threat among the two war-involved civs", or at least the difference among the two civs' strength should be not big.

Because of course the weak part would accept such an offer and make the war pause/stop in a way that is not much realistic in true life world.

I don't know/remember if Civ5 had such a check though.

Any feedback?

PS: maybe this could be eventually handled even before in the "add offers" phase, only adding, to the offering menu, civs the recipient civ is at war with, that are not a bigger threat for it (>low or > medium).

@SomeTroglodyte
Copy link
Collaborator

After reading the diff I had this:
"
template.properties:
Declaration of peace =
Make peace with [civName] =

Explain your change to DiplomacyManager.turnsToPeaceTreaty? Why the union? Doesn't the original loop justifiably assume all trades are mirrored both ways in both participant's Trade collections?
"

... and that explanation is right there in your OP. So it boils down to - as your tests seem to prove - that trades are not always symmetrically mirrored to all participants? Hmmm... I'd probably create a honeypot - have turnsToPeaceTreaty evalue the old and the new way, and set a debugger breakpoint where these differ. Then playtest until it hits, then look closely at the Trade lists to get a picture why. I'm not the Diplomacy or Trade expert (at all), but I vaguely remember durations were some countdown thing with "flag" in the name? Yea look right there - DiplomacyManager.flagsCountdown - why does turnsToPeaceTreaty not use that then?

Tldr: Looks good to me (taking into account I don't quite 'get' how Diplomacy ticks)

@TommasoPetrolito
Copy link
Contributor Author

TommasoPetrolito commented Apr 11, 2024

After reading the diff I had this: " template.properties: Declaration of peace = Make peace with [civName] =

Explain your change to DiplomacyManager.turnsToPeaceTreaty? Why the union? Doesn't the original loop justifiably assume all trades are mirrored both ways in both participant's Trade collections? "

... and that explanation is right there in your OP. So it boils down to - as your tests seem to prove - that trades are not always symmetrically mirrored to all participants? Hmmm... I'd probably create a honeypot - have turnsToPeaceTreaty evalue the old and the new way, and set a debugger breakpoint where these differ. Then playtest until it hits, then look closely at the Trade lists to get a picture why. I'm not the Diplomacy or Trade expert (at all), but I vaguely remember durations were some countdown thing with "flag" in the name? Yea look right there - DiplomacyManager.flagsCountdown - why does turnsToPeaceTreaty not use that then?

Tldr: Looks good to me (taking into account I don't quite 'get' how Diplomacy ticks)

@SomeTroglodyte
I will debug the topic further like you suggested using a debugger breakpoint in that code point, I have a suitable game save that will fit well to do it fast.

In general trades are saved like this:
the "from" Civ gets the trade saved let's say (illustrative) with {ourOffers: [A, B], theirOffers: [C]} and "to" Civ gets the "reversed" trade with content of ourOffers moved to theirOffers and viceversa, so: {ourOffers: [C], theirOffers: [A, B]}.
In general this is OK, I'm just not sure if this was an issue in general with the PeaceTreaty or only my bad missing of something.

The countdown should be working fine but is based on the TradeOffer.duration that in the tradeautomation is decreased by 1 every turn (at least I have understood this by just reading the code). I will have a look and revise using the debugger this part as well anyway so that I can get a deeper knowledge of all this mechanism. If needed we can also clean up it a bit and apply the usage of DiplomacyManager.flagsCountdown in a coherent way also for this feature, even if related with Trade as well.

I read that comment that you reported:

Explain your change to DiplomacyManager.turnsToPeaceTreaty? Why the union? Doesn't the original loop justifiably assume all trades are mirrored both ways in both participant's Trade collections?

And I actually completely agree with the comment, I will debug it in depth, but the most meaningful thing to do would be having the offer contained both in ourOffers AND theirOffers, so that in DiplomacyManager.turnsToPeaceTreaty we can assume all trades related to treaties are mirrored and checking ourOffers is enough. This is the right way to have it working.

I'll investigate a bit more, it is possible that the issue was on my feature when I tested, but by debugging in depth (previous and post code versions) I will be able to have a look to what exactly we find inside "trades" in that moment and understand if union is redundant (or maybe union is anyway redundant, if a fix is needed, it is on the trades values filling, to make sure that they are complete and symmetrical).

Thank you for your feedback. I will let you know guys.

@SomeTroglodyte
Copy link
Collaborator

Something like that. It's good you set out to learn, I would have to do the same to properly judge your implementation. And having someone independent and lacking prejudices analyze - good thing.

Mind you, I would not hesitate to merge this if it were up to me, as the feature is welcome, the general style is good, and from that I would operate on the prejudice that your testing is also good. 🥳

@TommasoPetrolito
Copy link
Contributor Author

TommasoPetrolito commented Apr 11, 2024

Mind you, I would not hesitate to merge this if it were up to me, as the feature is welcome, the general style is good, and from that I would operate on the prejudice that your testing is also good. 🥳

Then I will just make some checks and most likely revise at least the union/symmetrical trade filling thing. While I'm confident for everything else (trade.duration is OK and that is currently making the standard PeaceTreaty work without issues).

For eventual clean-up to check if everything can be handled with DiplomacyManager.flagsCountdown, I will postpone it and keep it out for the moment. Like this, we can also guarantee a fast release of this much wanted feature in a first simple version.

I want to go back to this feature in the future anyway to handle more meaningfully some specific cases (like the one in my first comment) so we don't need to go with a perfect thing immediately.

I'll put it in draft while I check better the union/symmetrical trade filling thing then I'll let it become mergeable again.

Thank you again.

@TommasoPetrolito TommasoPetrolito marked this pull request as draft April 11, 2024 16:48
@TommasoPetrolito TommasoPetrolito marked this pull request as ready for review April 11, 2024 17:44
@TommasoPetrolito
Copy link
Contributor Author

TommasoPetrolito commented Apr 11, 2024

@SomeTroglodyte, done!

With regad to this:

Then I will just make some checks and most likely revise at least the union/symmetrical trade filling thing. While I'm confident for everything else (trade.duration is OK and that is currently making the standard PeaceTreaty work without issues).

Standard Peace Treaties work fine.

The issue was in my implementation. In short: we have civ A, B and C, B and C are at war.
We are A and we ask "Make Peace With C" to B offering some gold, B accepts. The main trade occurring is among A and B.
If B accepts and I add a Peace Treaty like I had done before my last commit what I was doing was:

A-B {trades: {theirOffers: [Trade[TradeOffer"MakePeaceWith"]], ourOffers: [Trade[TradeOffer"Gold"]]}
B-A {trades: {theirOffers: [Trade[TradeOffer"Gold"]], ourOffers: [Trade[TradeOffer"MakePeaceWith"]]}
then the addition of puppet PeaceTreaty that I had inserted was incomplete and I had:
B-C {trades: {theirOffers: [], ourOffers: [Trade[TradeOffer"PeaceTreaty"]]}
C-B {trades: {theirOffers: [Trade[TradeOffer"PeaceTreaty"]], ourOffers: []}

What I had faced happening was C declares war against B the following turn, and this actually makes sense because its ourOffers was empty.
This only applied to my wrongly written feature.

Standard PeaceTreaties work fine becase in that case you have simply let's say B offering a PeaceTreaty to C and that Trade is configured to have symmetrical double offer of PeaceTreaty from both sides then you have:

B-C {trades: {theirOffers: [Trade[TradeOffer"PeaceTreaty"]], ourOffers: [Trade[TradeOffer"PeaceTreaty"]]}
C-B {trades: {theirOffers: [Trade[TradeOffer"PeaceTreaty"]], ourOffers: [Trade[TradeOffer"PeaceTreaty"]]}

then in this case everything works fine.

With my last commit I have fixed this wrong behavior in my feature and in particular with lines:

TradeType.PeaceDeclaration -> { val nameOfCivToDeclarePeaceOn = offer.name from.getDiplomacyManager(nameOfCivToDeclarePeaceOn).makePeace() //let's get the civilization to declare peace on val civToDeclarePeaceOn = from.gameInfo.civilizations.first() { it.civName == nameOfCivToDeclarePeaceOn} // let's create a "puppet" peaceTreaty to propagate turnsToPeaceTreaty duration-related effects val peaceTrade = Trade() peaceTrade.ourOffers.add(TradeOffer(Constants.peaceTreaty, TradeType.Treaty, 1, com.unciv.UncivGame.Current.gameInfo!!.speed)) peaceTrade.theirOffers.add(TradeOffer(Constants.peaceTreaty, TradeType.Treaty, 1, com.unciv.UncivGame.Current.gameInfo!!.speed)) // let's add peaceTreaties to both diplomacyManagers from.getDiplomacyManager(civToDeclarePeaceOn).apply { trades.add(peaceTrade) } civToDeclarePeaceOn.getDiplomacyManager(from).apply { trades.add(peaceTrade) } }

Now it should be good to go.
I will come back to improve it further for eventual migration from TradeOffer.duration decreasing approach to DiplomacyManager.flagsCountdown and specific handling of certain cases later in future PRs.

@TommasoPetrolito TommasoPetrolito force-pushed the diplomacy_make_peace branch 2 times, most recently from f64bec0 to 6822da0 Compare April 11, 2024 18:32
@TommasoPetrolito
Copy link
Contributor Author

Hi @yairm210 do you have any feedback about this PR?

val civToMakePeaceWith = civInfo.gameInfo.getCivilization(offer.name)

return when (Automation.threatAssessment(civInfo, civToMakePeaceWith)) {
ThreatLevel.VeryLow -> 1000
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should the AI be interested in this offer...? A human wouldn't be? "Hi I'm going to make peace with X please give me money" - sounds like no

TradeType.PeaceDeclaration -> {
val civToMakePeaceWith = civInfo.gameInfo.getCivilization(offer.name)

return when (Automation.threatAssessment(civInfo, civToMakePeaceWith)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a way to exploit the AI, honestly. Would you agree to this?
We're trying to make an AI that plays to win, so it should be dependent on e.g
how close I am to taking over a city

@@ -139,6 +149,23 @@ class TradeLogic(val ourCivilization: Civilization, val otherCivilization: Civil
val nameOfCivToDeclareWarOn = offer.name
from.getDiplomacyManager(nameOfCivToDeclareWarOn).declareWar()
}
TradeType.PeaceDeclaration -> {
val nameOfCivToDeclarePeaceOn = offer.name
from.getDiplomacyManager(nameOfCivToDeclarePeaceOn).makePeace()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need agreement from both sides to make peace! This should be only "make a trade offer to make peace". Otherwise I can force the other player!

Imagine a 3 player game, 2 humans one AI. You, human A, see that human B is about to take over a city. So what do you do? You trade the AI and say "take this money and make peace", and then there's peace, you've screwed the other human's plans!
Same thing for AI. We should not be treating them differently.

@yairm210
Copy link
Owner

The concept of "unilaterally make peace" doesn't make sense, that's not how peace works, should be changed to "offer peace".
Even if Civ V did it this way, that's no excuse. We differ from them when their design decisions have problems.

@TommasoPetrolito
Copy link
Contributor Author

TommasoPetrolito commented Apr 11, 2024

I'm wondering...

One thing that I might be missing and that might make sense: maybe we could define the offer "ACCEPTABILITY" as well.

Implementing something like "peaceDeclaratiom offer can only be offered if the recipient is the higher threat among the two war-involved civs", or at least the difference among the two civs' strength should be not big.

Because of course the weak part would accept such an offer and make the war pause/stop in a way that is not much realistic in true life world.

I don't know/remember if Civ5 had such a check though.

Any feedback?

PS: maybe this could be eventually handled even before in the "add offers" phase, only adding, to the offering menu, civs the recipient civ is at war with, that are not a bigger threat for it (>low or > medium).

@yairm210 as you can see I was reasoning about the same limits and critical points you are referring. I think defining in details the mechanism that we want to implement might require a discussion before actually implementing it in the proper way, I totally agree. Of course my implementation started with very simple assumptions on the Offer evaluation part.

For sure after a naive approach I realized why this had not be implemented yet 😅, doing it properly requires a deeper study and work on the mechanism.

We have A offering B "Make Peace with C".
And we should choose what we should make happen in the background.
Do you think it should happen a secondary trade between B & C (like if they were actually trading a direct PeaceTreaty) that needs to be resolved to resolve the A-B main trade?

I might work on this idea, if you think it could lead to a more meaningful result. Thank you very much for your feedback any hints or suggestions are very welcome.

PS:
One idea I mentioned in my comment above was, making the offer available only towards the Civ that is a higher threat for the other, maybe this is simpler then actually collecting the agreement on both sides but might make already a bit more sense because at least it gets more the shape and meaning of a ceasefire request to the stronger civ, maybe this could be a first compromise to improve this, also raising a lot to 10000 the cost for the weak opponent that I had put to 1000, mirroring completely the costs of the War Declaration, I don't know if this would be already enough an improvement @yairm210 ?

Truth is that there are more options and ideas that could be implemented and we need to get a clear idea of how we expect the whole process to behave in order to make sense and then we can implement it.

@yairm210
Copy link
Owner

What should happen in the background is that B sends a peace request to C - a trade request with the peace treaties, just like in automation.
Then C, on its own turn, is free to decide whether to accept or decline

@yairm210
Copy link
Owner

See

internal fun offerPeaceTreaty(civInfo: Civilization) {

The question of "is this offer actually good enough that C would accept" is a good question and one we should be asking ourselves!

@TommasoPetrolito
Copy link
Contributor Author

TommasoPetrolito commented Apr 11, 2024

See

internal fun offerPeaceTreaty(civInfo: Civilization) {

The question of "is this offer actually good enough that C would accept" is a good question and one we should be asking ourselves!

I see, I will reason about it. The simplest way would be seeing it just like you proposed: we pay to get B "offer peace to C" with the major downside being the risk to pay to basically get nothing in the worst case scenario if then C declines.

Another idea might be taking into account C evaluations and availability to peace within the A-B trade evaluations, so that B won't accept if C would not accept or something like that.

Another idea might be introduce a new concept in the mechanism like a three-ends trade (but I need to explore more the idea). It could be something like the effects of the Trade are enabled only if we get approval by both. One might accept the other decline and we don't pay anything due to the one declining.

@TommasoPetrolito TommasoPetrolito marked this pull request as draft April 14, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants