Adjacent Improvements #11214
Adjacent Improvements #11214
Conversation
- improvement Yields are increased by other, specific, adjacent Improvements
- fractional Yields from different Improvement Types are added together
- obsoletes Improvement_AdjacentImprovementYieldChanges, Improvement_YieldAdjacentSameType, Improvement_YieldAdjacentSameTwoType
- obsolete tables will be removed after next Congress to give modmodders time to make the swap
- CivilopediaScreen:
- The relevant pages for improvements using this table now have inputs ceiling'd to the second decimal place
- Text Frames don't automatically resize unless pushed through UpdateNarrowTextBlock.
- The Adjacent Improvement text can be pretty long between newlines, and the wrapping function doesn't seem to have hanging indents, so I made my own
SQL Cleanup:
- ImprovementChanges.sql should not include UIs (their presence in Improvement_TechYieldChanges and the PillageGold column will stay for now)
- Farms on Freshwater Hills aren't a thing, so Terrace Farm doesn't need to reference them
- clarify actual use of Improvement_AdjacentTerrainYieldChanges (the improvement boosts the adjacent terrain, not vice versa)
|
Nice, I'll get to work with the CvBuilderTaskingAI support asap |
LessRekkless
force-pushed
the
upstream/vpc-2.06a/adjacent_improvements
branch
from
75e31eb
to
61383e3
Compare
| // Special case for culture | ||
| if (eYield == YIELD_CULTURE) | ||
| { | ||
| fYield += m_iCultureAdjacentSameType; |
There was a problem hiding this comment.
I'm confused. OtherImprovement can be anything, not just the same type of improvement. Why is this Culture exception being unconditionally added when it's labelled "CultureAdjacentSameType"?
There was a problem hiding this comment.
Didn't realize it wasn't displaying, my bad.
There was a problem hiding this comment.
I removed m_iCultureAdjacentSameType entirely
|
|
||
| map<YieldTypes, map<ImprovementTypes, fraction>>::const_iterator itImprovement = m_YieldPerXAdjacentImprovement.find(eYield); | ||
|
|
||
| return itImprovement != m_YieldPerXAdjacentImprovement.end() |
There was a problem hiding this comment.
missing a semicolon, doesn't compile
CvGameCoreDLL_Expansion2/CvPlot.cpp
Outdated
| pAdjacentPlot = plotDirection(getX(), getY(), static_cast<DirectionTypes>(iI)); | ||
| if(pAdjacentPlot) | ||
| { | ||
| fRtnValue += kImprovement.GetYieldPerXAdjacentImprovement(eYield, pAdjacentPlot->getImprovementType()); |
There was a problem hiding this comment.
There's no IsImprovementPillaged check here and no verification that pAdjacentPlot->getImprovementType() doesn't return NO_IMPROVEMENT.
| if (eYield == NO_YIELD) | ||
| return (!m_YieldPerXAdjacentImprovement.empty() || m_iCultureAdjacentSameType != 0); | ||
| else if (eYield == YIELD_CULTURE) | ||
| return m_iCultureAdjacentSameType != 0; |
There was a problem hiding this comment.
this needs to be:
if (eYield == YIELD_CULTURE && m_iCultureAdjacentSameType != 0)
return true;
as it is now, normal culture adjacency yields don't work at all
There was a problem hiding this comment.
Removed m_iCultureAdjacentSameType entirely
LessRekkless
force-pushed
the
upstream/vpc-2.06a/adjacent_improvements
branch
from
61383e3
to
d5edfe9
Compare
|
I've deleted |
|
|
| const int nRequired = pResults->GetInt(3); | ||
| CvAssert(nRequired > 0); | ||
|
|
||
| m_YieldPerXAdjacentImprovement[yield_idx][improvement_idx] = fraction(yield, nRequired); |
There was a problem hiding this comment.
@LessRekkless This doesn't appear to be additive with CultureAdjacentSameType.
There was a problem hiding this comment.
Updated using the += operator, which initializes a map to the default ( fraction(0,1) ) before adding.
Tested using C++ 98:
There was a problem hiding this comment.
Updated using the += operator, which initializes a map to the default ( fraction(0,1) ) before adding.
Tested using C++ 98:
Maybe a unit test suite wouldn't be the worst idea. But not sure how to set that up, should preferably be run in the CI pipeline or whatever it's called.
There was a problem hiding this comment.
As a future improvement perhaps
- added YieldPerXAdjacentImprovement into the dll, along with relevant functions and its use in calculating yields, build tasker and improvement checking. - Adjacent Improvement yields are no longer required to be the same yield as base. - all improvements now GAIN benefits from adjacency, none of them will GIVE benefits. - Marked obsolete variables and functions for deletion. Improvement_YieldPerXAdjacentImprovement is replacing: - Improvement_YieldAdjacentSameType, - Improvement_YieldAdjacentTwoSameType, - Improvement_AdjacentImprovementYieldChanges - completely removed the m_iCultureAdjacentSameType member variable associated with the vanilla Improvement.CultureAdjacentSameType column. The column now directly inserts into m_YieldPerXAdjacentImprovement Miscellaneous: - Consolidated adjacent improvements during setImprovementType() and setImprovementPillaged() - remove adjacent plot updates for Improvement_AdjacentResourceChanges, Improvement_AdjacentFeatureChanges, as these tables are covered with this->updateYield(), not pAdjacentPlot->updateYield() - extended YieldTypes for-loops to CULTURE_LOCAL where I deemed it appropriate - combined adjacent plot for-loops where possible
LessRekkless
force-pushed
the
upstream/vpc-2.06a/adjacent_improvements
branch
from
d5edfe9
to
80a021b
Compare
New table Improvement_YieldPerXAdjacentImprovement to replace the following VP-created tables:
Everything now gains yields from adjacent improvements, based on the number required improvements. Fractional Yields from different Improvement Types are added together (truncated down to the nearest whole number).
These tables and the corresponding (commented) SQL will be deleted during the implementation phase of next Congress, so modmods have time to switch over.
@KungCheops I didn't touch AI evaluation of this, as you asked.
please keep the two commits separate.