LoneGazebo / Community-Patch-DLL

Community Patch for Civilization V - Brave New World
Other
285 stars 157 forks source link

Issue with food from City states #10944

Closed RemitoL closed 3 months ago

RemitoL commented 4 months ago

1. Mod Version (X.Y.Z). Current Version: 4.7.3 Version 4.7.3

3. List of Other Mods Community Events for VP

4. Describe the Issue I noticed in my latest game that many of my cities were suddenly starving. I was a bit surprised but then I realized it never stopped, even after the population loss. I looked at the Food details to understand, and it seems the Food from maritime city states is calculated in a wrong way, with very negative numbers. It appears in the Modern era, worsens in the atomic era nad becomes unplayable in the information era (-107 food per turn !)


5. Save Game From 1 Turn Before (ALWAYS ATTACH THIS IF POSSIBLE) This is not from 1 turn before as the bug is there all along the game but I attach 1 save from the modern era, 1 from the atomic and 1 from the information era Pacal_saves.zip

6. Logs (ALWAYS ATTACH THESE IF POSSIBLE) Not sure I have activated those

9. Screenshots of the Issue (Optional) Screenshots from a city (non capital) in modern, atomic and information era

City screen modern era City screen atomic era

City screen information era

azum4roll commented 4 months ago

Maritime CS yields add to the city tile and shouldn't be represented as "from City-States". Something has to be wrong there.

bsesar commented 4 months ago

I don't see negative numbers in my cities, but I can confirm the "from City-States" part. Also, while the food from City States in my other cities matches the tooltip associated with my only Maritime CS ally, Quebec City (3 Food in other cities), the amount of food in my capital, (Rome) from City States should be 8, but the city tooltip says 12. I do have 4 City States as allies, so maybe that's where the additional 4 Food in Capital come from? I do not have any Statecraft policies, just Authority and Fealty trees filled up.

Rome_food Rome_Maritime_CS_food

I am also playing with 4.7.3 and the only other mod I have is Improved City View (v20). Logs and a savefile are attached.

Logs.zip CS_food.zip

bsesar commented 4 months ago

@azum4roll, when fixing issue #10667, ilteroi noted that

"bonus food from maritime city states is now booked correctly as food from city states and not just added to the city plot"

https://github.com/LoneGazebo/Community-Patch-DLL/issues/10667#issuecomment-1957808507

@ilteroi, any idea what may be causing discrepant food numbers from CS as in the above examples?

ilteroi commented 4 months ago

i would like to see this fix, i suspect it's a missing *100 somewhere?

anyway i cleaned up the code a bit and found some other problems as well ...

axatin commented 4 months ago

i would like to see this fix, i suspect it's a missing *100 somewhere?

no, something completely different. edge case of having a sphere of influence while not being allied. every turn, the game thought the ally status had just been removed and subtracted the food bonus

RemitoL commented 4 months ago

Thanks ! Indeed I can confirm I had at least 2 Maritime CS under Sphere of Influence that were not allied at the time of the vote.

bsesar commented 4 months ago

Nice to see that fixed 🙂 Is the 12 Food in Rome (see screenshot above) expected behavior given that I have only one CS ally that gives 8 food?

axatin commented 4 months ago

@bsesar Do you have Scrivener‘s Office in Rome?

bsesar commented 3 months ago

I do. That explains it, thanks! 🙂

azum4roll commented 3 months ago

i would like to see this fix, i suspect it's a missing *100 somewhere?

no, something completely different. edge case of having a sphere of influence while not being allied. every turn, the game thought the ally status had just been removed and subtracted the food bonus

Didn't something similar happen before, but only subtracted once on sphere gain?

ilteroi commented 3 months ago

i'm not sure about this fix, did you test it?

look at the checks a few lines above:

    // Resolve Allies status with sphere of influence or open door
    bool bHasOtherPermanentAlly = false;
    if (IsNoAlly() || (GetPermanentAlly() != NO_PLAYER && GetPermanentAlly() != ePlayer))
    {
        bNowAllies = false;
        bHasOtherPermanentAlly = true;
    }

the GetPermanentAlly() != ePlayer case is already covered?

axatin commented 3 months ago

these two are not in the same if-else block.

ilteroi commented 3 months ago
  1. bHasOtherPermanentAlly is the link
  2. you did not answer the question
axatin commented 3 months ago

That change works fine, I tested it. But I agree it's not the most elegant solution, I'll rewrite it.

Looking into the topic a bit deeper, there are also some other bugs when open doors / SoI are set or removed. Working on a fix.

ilteroi commented 3 months ago

please wait a while i have also rewritten a lot of this setAlly logic, will push later today

axatin commented 3 months ago

ok, let me know when it's pushed

RecursiveVision commented 3 months ago

@ilteroi https://github.com/LoneGazebo/Community-Patch-DLL/commit/6aa9cde9793a86d04a7410c1a47e8d33c3ab70e1#r142926241