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

City-State quest "Invest" does not end after its turn limit is reached #11426

Closed
1 task done
the-s-is-silent opened this issue Apr 9, 2024 · 2 comments · Fixed by #11475
Closed
1 task done

City-State quest "Invest" does not end after its turn limit is reached #11426

the-s-is-silent opened this issue Apr 9, 2024 · 2 comments · Fixed by #11475
Labels

Comments

@the-s-is-silent
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Game Version

4.11.2

Describe the bug

This bug has existed since at least 4.10.16 according to comments in the bugs discussion channel on the Discord server. Only this one specific CS quest is affected. Invest is effectively permanent once given as long as the CS isn't conquered and you don't demand tribute or get into a war with it.

Mod/ruleset dependencies: None. It can be observed on G&K, DeCiv Redux, Alpha Frontier, Rekmod, etc.

Steps to Reproduce

  1. Open any saved game or start a new game.
  2. Play until you see a CS assign the Invest quest and continue play for the normal duration of that quest on your game speed.
  3. Observe that once it reaches the last turn for the quest, it gets stuck there and will read 0 turns left for the rest of the game, but will never expire no matter how many turns pass after it would normally expire provided the CS remains alive and unprovoked by you.

Screenshots

No response

Link to save file

No response

Operating System

Linux

Additional Information

No response

@SpacedOutChicken
Copy link
Contributor

I can confirm that this problem still exists as of version 4.11.4. Here is a saved game in which Jerusalem has just started offering an Invest quest:
JerusalemInvestStart.json
And here is the game many turns later, with the Invest quest still on the table:
JerusalemInvestForever.json

@SomeTroglodyte
Copy link
Collaborator

SomeTroglodyte commented Apr 15, 2024

Likely since #11224 - and the new WinnersAndLosers class not classifying all civs as either winners or losers. So - could happen to non-Invest quests as well if some Civs have score 0.

Index: core/src/com/unciv/logic/civilization/managers/QuestManager.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/com/unciv/logic/civilization/managers/QuestManager.kt b/core/src/com/unciv/logic/civilization/managers/QuestManager.kt
--- a/core/src/com/unciv/logic/civilization/managers/QuestManager.kt	(revision 7ce291329318b86ce346765e88dcb95eacaf8e0f)
+++ b/core/src/com/unciv/logic/civilization/managers/QuestManager.kt	(date 1713182374326)
@@ -272,8 +272,7 @@
         winnersAndLosers.winners.forEach { giveReward(it) }
         winnersAndLosers.losers.forEach { notifyExpired(it, winnersAndLosers.winners) }
 
-        assignedQuests.removeAll(winnersAndLosers.winners)
-        assignedQuests.removeAll(winnersAndLosers.losers)
+        assignedQuests.removeAll { it.questNameInstance == questName }  // removing winners then losers would leave those with score 0
     }
 
     private fun handleIndividualQuests() {

needs test ... done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants