yairm210 / Unciv

Open-source Android/Desktop remake of Civ V
Mozilla Public License 2.0
8.54k stars 1.58k forks source link

Civ keeps declaring war right after declaring peace. #10809

Open The-Talik opened 10 months ago

The-Talik commented 10 months ago

Is there an existing issue for this?

Game Version

4.9.10-patch1

Describe the bug

I originally thought this was coincidence, so I've been exploring it, but every time I negotiate peace, they break it the next turn. I'm allied with a few city states and nations, so I thought it was one of their defensive pact allies, but when I don't declare peace, no one attacks me that turn. This seems to happen whether I initiate the peace offer, or they do.
It has happened half a dozen times. I'm playing with REKmod, so apologies if this is caused by that mod.

Steps to Reproduce

Attaching a save file. File is at the end of a turn. If you negotiate peace with Boers (I didn't do this in the save yet, but they will accept a basic offer) after accepting it, they will immediately declare war. If you do not negotiate peace, nothing out of the ordinary happens.

Screenshots

No response

Link to save file

https://drive.google.com/file/d/18QTJOfLOHLAXjZRLxuqSQUzqbmEJiqaK/view?usp=sharing

Operating System

Android

Additional Information

No response

The-Talik commented 10 months ago

Also in this game, my defensive pact doesn't seem to trigger. My two def pact allies have been declared war upon, but I didn't automatically declare war on their enemies.

yairm210 commented 10 months ago

Okay, so this is what's happening. You sign peace with Boers. They then check "who can we declare war on? Oh look at that, the Netherlands!" They do so You, being in a defensive pact with The Netherlands, are then brought into the fight

I'm not sure what should be the correct action in this case, I think this is actually the correct response, BUT it should DEFINITELY be clearer from the notifications I mean... image Unclear.

The SECOND strange thing is that The Netherlands think you're in a defensive pact with them... but your civ doesn't think so. That's a bug for sure, maybe when it expires it only updates one side...

yairm210 commented 10 months ago

I can't see why that would be, since nextTurnFlags() says 'when 0 turns left for defensive pact turn to allies' and signDefensivePact() says 'both sides get the same number of turns on defensive pact' A possible culprit is the 'removeFlag DefensivePact' in breakTreaties() which doesn't actually break the defensive pact in the status, we should add a new function called breakDefensivePact which does all of those together (Also in removeDefensivePacts(), but that does seem to deal with the status)

SeventhM commented 10 months ago

DEFINITELY be clearer from the notifications

Shouldn't this obviously have a way to say "we declared war on Boers"?

yairm210 commented 10 months ago

In order to not be considered a breach of defensive pact, the party that declares war on a civ is considered to have declared war on its allies as well. We should probably have an enum of "declareWarReason" to clear this up.

SeventhM commented 10 months ago

I'm more saying defensive pacts means if someone declares war on your ally, you auto declare war on them. It's similar to Kuwait declaring war in the notifications, the notification probably should say we declared war on Boers, not Boers declared war on us

github-actions[bot] commented 7 months ago

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 15 days.

SeventhM commented 7 months ago

Minimum, is there going to be an update to the notification text? I feel like this is probably "will not stale" (simple enough to fix) that we forgot about

tuvus commented 7 months ago

I can't see why that would be, since nextTurnFlags() says 'when 0 turns left for defensive pact turn to allies' and signDefensivePact() says 'both sides get the same number of turns on defensive pact'

I think this is likely because of how trades and flags work. When signing we give each side a defensive pact flag flag for X amount of turns, however that can mean that the flag has expired for one side and not the other. Is that what is happening?

A possible culprit is the 'removeFlag DefensivePact' in breakTreaties() which doesn't actually break the defensive pact in the status, we should add a new function called breakDefensivePact which does all of those together

Theres a reason for this, we need to break the defensive pacts but remember that they were broken to apply their repercussions in breakTreaties(). removeDefensivePacts() is called earlier in onWarDeclared(diplomacyManager, true) which needs to be called before we call callInDefensivePactAllies().

Would it be possible to move breakTreaties() before the actual declaring war portion? Then we can have removeDefensivePacts() inside of breakTreaties() all in one place.

yairm210 commented 7 months ago

@tuvus yes, that's what's happening. This is part of a deeper problem, which is that essentially we duplicate trade info and thus don't have a clear source of truth. The real solution would be to move trades into a gameinfo-level list, so there's one single definitive state, but that sounds like an Undertaking

Re: moving around functions, sure, do what you need