Unciv
Unciv copied to clipboard
Buying Castles and associated ideas
Before creating
- [X] This is NOT a gameplay feature from Civ VI, BNW, or outside - see Roadmap
- [X] This is NOT a gameplay feature from Vanilla Civ V or from G&K - If so, it should be a comment in https://github.com/yairm210/Unciv/issues/4697
Problem Description
As we know, buying Happiness buildings takes a lot of time compared to other buildings - ~because someone whined about starving cities~ because the limit for nullifies growth may be passed and cities might need food again, so all of them may perhaps need their worked tiles re-optimized. That may well be several seconds on a fast phone and ~20 cities.
Apart from optimizing that code (e.g. intelligently parse mods to see where happiness limits affecting nullify growth really are) -
- Doesn't happen for Castles and their follow-ups with Professional Army and/or Neuschweinstein: The code might need to go and scan all StatsFromObject uniques from the Civ too!
Related Issue Links
#9799, #9425, #9238, #10397 - PR #9395 introduced the re-eval
Desired Solution
Advice... How can we best deal with that
Alternative Approaches
- Could that recalculation be done in a Coroutine while the user stays in the CityScreen, complete with preemption when they buy another Happiness building or queue reordering when they change to view one of the affected cities? And background processing continuing to run to finish while user changes views, potentially updating WorldScreen map tiles overlays while the user is looking at them?
Additional Context
- I see a StatsFromBuildings unique where a StatsFromObject should suffice from an implementation viewpoint - is that for prettier language only? (Sovereignty policy)
So would an extra
if (getMatchingUniques(UniqueType.StatsFromObject, StateForConditionals.IgnoreConditionals).any {
it.stats[stat] > 0 && matchesFilter(it.params[1])
}) return true
...in Building.isStatRelated be the ticket? Could take ages to test properly.
And as for the monster 'hanging' delays - maybe rethink the line #10397 points out - "Note that negative food will also be nullified. Pretty sure that's conform civ V, but haven't checked." - (or decide to tune the Unique to allow nullifying positive growth and/or starvation independently, so mods or our base rulesets can control the delay), because if starvation isn't nullified, the entire reason for reallocating citizens is moot?
Buying castles should change the happiness is the civ which would trigger recalc for all cities If we manually check for happiness buildings, we can get rid of that check
That's an extra recalc we can throw away
Not sure I get what you mean.
Also - that code up there would in fact extend the meaning of "Science" and "Culture" buildings (in fact I think that would be correct and is slightly wrong now), making Castles meet the Culture building filter after Neuschwanstein is owned - but not make Neuschwanstein itself a Happiness/Culture building which it theoretically should be if and only if the player already has at least one Castle... But then it can't be bought. But mods could create such a situation.
...in Building.isStatRelated be the ticket?
I'm not so sure about doing that tbh
-
Building.getMatchingUniques
only checks the uniques of the object itself, not any uniques the civ may have by this point. E.g., Neuschwanstein wouldn't make castles into a culture building, but"[stats] from every [Castle]"
on the building itself would. You would need to pass in the city/civ intoBuilding.isStatRelated
to do what you want - From what I'm aware, Civ 5 itself wouldn't consider Castles as culture buildings with Neuschwanstein, so this seems slightly wrong, not what we currently do
- Neuschwanstein wouldn't male
Yeah you're right, I actually noticed later my last statement was wrong, inverted actually, but I was at the moment too far away from a browser[^1] to post, then forgot. Treating Castles as Culture buildings would indeed feel wrong. But still - there's a very small window to still get the "starving after buying" effect despite the fix we attempted for #9238.
[^1]: Yeah, I started dreaming about Unciv code some while ago. Horribly embarrassing and a true deep horror, the "I have no mouth but I must scream" kind...
I'm still in favour of a rule change - make unhappiness not nullify starvation, then throw away that forced worker reassignment...
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.
Can't find the PR Yairm did to reduce the amount of recalculations, but this should be solved by now. Does this need to stay open?
Apart from optimizing that code (e.g. intelligently parse mods to see where happiness limits affecting nullify growth really are)
Yair did that nicely. I remember that too.... git blame on allHappinessLevelsThatAffectUniques
says #11034.
Doesn't happen for Castles and their follow-ups with Professional Army and/or Neuschweinstein
Don't know - but looking at the critical path now it seems there is no problem. Interactive buying always ends up in CityConstructionsTable.purchaseConstruction
which always calls reassignPopulation
, which should take care of happier city needing more food??? Anyway, if there's a window someone else will notice sooner or later.