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

Rework Morale Display #1708

Merged

Conversation

KheirFerrum
Copy link
Collaborator

@KheirFerrum KheirFerrum commented Jul 8, 2022

Summary

SUMMARY: Bugfixes "Fix Morale Calculations."

Purpose of change

Originally this PR was to fix a display oddity, as fatigue would display on the morale screen as a penalty when in reality it capped the focus equilibrium display and increased/decreased fatigue penalties accordingly. Leading to unintuitive situations where the more you tried to increase your morale the higher the penalty became.

While converting it to display as a cap. It was found that morale calculations were broken. Fatigue itself did not affect the focus equilibrium, but rather applied extra per tick decrements directly to focus change calculations. Furthermore, the caps for each tier of fatigue were different from the player side display.

Describe the solution

Created new function calc_fatigue_cap() to unify the calculations, so that only one calculation needs to be altered to affect both the display and the effect on focus.

Removed direct effect on focus change calculations and applied the fatigue cap to effective morale when calculating focus equilibrium, as I assume was intended.

Changed the display to show the fatigue cap instead of fatigue penalties, and show regardless of whether morale is below or above cap to prevent player confusion.

Streamlined the pain_penalty display so it didn't call the entire focus equilibrium calculation.

Describe alternatives you've considered

  • Run away and live with the knowledge that morale and focus calculations are wrong.
  • Use calc_fatigue_cap() to affect the focus equilibrium directly instead of the eff_morale int.
    This could probably be done and would allow me to remove the - 100 from the calculations, but I'm on the fence.

Testing

Added pain to character, checked that it displayed properly. Added Fatigue to character, checked that cap was enforced but did not interfere with pain. (20 pain and tired level fatigue is still 80 morale, while increasing pain beyond the threshold decreases morale accordingly.) Check that fatigue cap was displayed so long as fatigue thresholds were reached.

Additional context

@Coolthulhu Coolthulhu self-assigned this Jul 9, 2022
src/player.h Outdated
/** Uses morale and other factors to return the player's focus target goto value */
int calc_focus_equilibrium( bool ignore_pain = false ) const;
/** Calculates the player's morale cap due to fatigue */
int calc_fatigue_cap() const;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better if it was just a standard function and not a method. It only requires the fatigue level.
In general, only those functions which get or set values that are (or should be) protected/private should be methods. If it only operates on public stuff, taking it out of the class makes it more useful.

For example, if you wanted the expected penalty after some action to warn the player that you'll be penalized at the time it matters (the moment you finish a surgery or something), the method would not work unless you duplicated the function contents or hacked player's fatigue to get the other value. With a separate function, it's trivial.

@Coolthulhu Coolthulhu merged commit c8c336a into cataclysmbnteam:upload Jul 21, 2022
@@ -4407,7 +4407,21 @@ bool player::query_yn( const std::string &mes ) const
return ::query_yn( mes );
}

int player::calc_focus_equilibrium( bool ignore_pain ) const
int calc_fatigue_cap( const player &p )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better if you had just the fatigue as int here.

When passing classes as references, it's good practice to pass the most base class that will certainly work here. In this case, it would be Character.

@KheirFerrum KheirFerrum deleted the Fatigue-Focus-Cap-Display branch July 23, 2022 17:58
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.

2 participants