-
Notifications
You must be signed in to change notification settings - Fork 20
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
Added difficulty-dependent "grace period" for crime and morale #1466
base: main
Are you sure you want to change the base?
Conversation
Existing behavior may change slightly in that crime won't pause if the population drops to zero after landing. I believe this change will not impact gameplay in a practical sense, but we can easily re-add the zero population check if needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the comments on Discord about possibly altering the difficulty level during gameplay, I think this design is what we want.
I made a few minor suggestions here, which either weren't thought of or weren't formalized during the last review.
There is one comment about consistent default values, and a related note about overflow, which could be relevant to future saved game loading if allowed to proliferate. For that reason I'll hold off on giving formal approval so that can be responded to. Otherwise, this looks about ready to be merged.
// MISCELLANEOUS | ||
int mTurnCount = 0; | ||
|
||
int mTurnNumberOfLanding = std::numeric_limits<int>::max(); /**< First turn that human colonists landed. If never landed, default is int max (representing the future). */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the update to use constants::ColonyShipOrbitTime
as the default value during loading of saved games, perhaps we should modify this to match. That way the default value on all code paths is the same.
In a practical sense, the turn of landing will never exceed the colony ship orbit time, since if colonists aren't landed by then, they all die in a fiery crash, and it's game over.
int turnsSinceLanding = mTurnCount - mTurnNumberOfLanding; // If negative, landing has not yet occurred. | ||
|
||
// Colony will not have a crime rate until at least n turns from landing, depending on difficulty | ||
if (turnsSinceLanding > gracePeriod[mDifficulty]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you wanted to avoid the comment about potential negative values, maybe we could still partially use the idea from the last review, and have a local variable:
moraleActiveTurn = mTurnNumberOfLanding + gracePeriod[mDifficulty]
The checks would then be:
if (mTurnCount > moraleActiveTurn)
We wouldn't need to worry about negative values in that case. Additionally, assuming the default value for mTurnNumberOfLanding
is updated to constants::ColonyShipOrbitTime
we wouldn't need to worry about potential overflow either.
As a small added bonus, it also means we avoid a double lookup of gracePeriod
within the same method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was just thinking this suggestion could have gone further, and set a bool
variable, rather than repeating a boolean expression in each if
condition.
bool isMoraleEnabled = mTurnCount > mTurnNumberOfLanding + gracePeriod[mDifficulty];
// If this is the first turn with population, then set mTurnNumberOfLanding | ||
if (mPopulation.getPopulations().size() > 0 && mTurnCount < mTurnNumberOfLanding) | ||
{ | ||
mTurnNumberOfLanding = mTurnCount; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per the comment on Discord, perhaps this should be moved to MapViewState::onDeployColonistLander
.
In that case, we know for sure there are going to be colonists landed at that point, so we don't need the population size check, which was really just a way of detecting when colonists have landed.
We would still need to retain the second check, since there can be multiple landers, and we probably want to activate on the first landing, and not reset the value upon a subsequent landing.
The second check should still work correctly with a default value for mTurnNumberOfLanding
of constants::ColonyShipOrbitTime
.
Skips morale and crime calculation until a difficulty-level-dependent "grace period" of 15-30 turns after human colonists have landed.
Fixes #1229, #1230, #1231