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

Added difficulty-dependent "grace period" for crime and morale #1466

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

andrewsg
Copy link

@andrewsg andrewsg commented Oct 1, 2024

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

@andrewsg
Copy link
Author

andrewsg commented Oct 1, 2024

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.

OPHD/States/MapViewStateIO.cpp Outdated Show resolved Hide resolved
OPHD/States/MapViewStateTurn.cpp Show resolved Hide resolved
OPHD/States/MapViewStateTurn.cpp Show resolved Hide resolved
OPHD/States/MapViewState.h Show resolved Hide resolved
Copy link
Member

@DanRStevens DanRStevens left a 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). */
Copy link
Member

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.

Comment on lines +719 to +722
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])
Copy link
Member

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.

Copy link
Member

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];

Comment on lines +767 to +771
// If this is the first turn with population, then set mTurnNumberOfLanding
if (mPopulation.getPopulations().size() > 0 && mTurnCount < mTurnNumberOfLanding)
{
mTurnNumberOfLanding = mTurnCount;
}
Copy link
Member

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.

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

Successfully merging this pull request may close these issues.

Crime shouldn't start until some amount of time after landing
2 participants