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

[WIP] [CR] Initial Cardio implementation. #44506

Closed
wants to merge 17 commits into from
Closed

[WIP] [CR] Initial Cardio implementation. #44506

wants to merge 17 commits into from

Conversation

gkarfakis19
Copy link
Contributor

@gkarfakis19 gkarfakis19 commented Sep 29, 2020

Summary

SUMMARY: Infrastructure "Adds a cardio modifier to the player character that affects stamina max and regeneration"

Purpose of change

Look at #44370!!
Adds a general purpose Cardio modifier to the player character that affects the max stamina, stamina regeneration and weariness stats. Currently WIP, details of implementation based on issue linked, credit to @I-am-Erk.
I do NOT plan on adding the exercise (cardio_acc) aspect of Cardio in this PR. It will come later.

TODO List:

  • Make cardio properly affect max_stamina
  • Make cardio properly affect stamina regen
  • Figure out when to update_cardio() -- currently updates every 10 min. <-- better suggestions requested.
  • Add a second json stamina regen modifier
  • Make cardio properly interact with weariness
  • Make traits increase/decrease cardio
  • Stop indefatigable/other stamina traits from influencing stamina, move to just cardio <-- comments requested, partially implemented.
  • Investigate traits "double-dipping" max_stamina.
  • Make sure all raw json accesses of MAX_STAMINA and STAMINA_REGEN have been replaced by encapsulated function calls
    ↓↓ These 3 should be investigated further in a new PR.
  • Make sure stamina changes properly interact with mutations
  • Look at rebalancing absolute value max stamina changing effects <-- Proper investigation in a new PR.
  • Change heartbeat_bpm <-- Moved to next PR, once cardio_acc gets added.
    ↑↑ These 3 should be investigated further in a new PR.
  • Make proficiencies increase cardio. <-- Currently no suitable proficiencies exist. Add a new one??
  • Make sure NPC's properly interact ( i.e. don't ) with this new system.

Describe the solution

Many changes to many files. Old functions refactored to use the Character::get_stamina_max() function and not to read directly from json.

Describe alternatives you've considered

Testing

Currently builds ( a true miracle ), although actual balancing cannot be undertaken until the addition of cardio_acc, which is going to be in a separate PR. Consider all values below to be of totally sedentary humans.

Character with Indefatigable, great health and 10 swimming skill.
image

Same character, without that trait and with 0 in swimming.
image

^^ EXTREMELY WIP NUMBERS (traits and skills shouldn't have that big an impact).

Additional context

Currently need comments and a lot of help on the implementation details. I have never attempted such far reaching code changes before. Specifically:

Comments Requested:

How often should update_cardio() be ran? Should it be recalculated after a certain amount of time (far more efficient but slow-to-react), say, every time bmr is recalculated, or should it be recalculated before every single instance of it's use (high performance impact, far more accurate)? Currently being calculated every 10 min.

How should stamina now be modifiable in json? Currently I make use of existing MAX_STAMINA json modifier, and I just use it as the base stamina before adding to it the cardio value, multiplied by another json modifier.

^^ Same question about stamina regen.

Check NPC behavior. Make sure these values don't apply to them.

New athletic proficiency?

@gkarfakis19 gkarfakis19 marked this pull request as draft September 29, 2020 18:12
@CountAlex
Copy link
Contributor

I suggest you also take a look and maybe change heartrate_bpm() at character.cpp as it might give you some ideas and probably should be changed to reflect your work.

@I-am-Erk I-am-Erk added 0.F Feature Freeze <Enhancement / Feature> New features, or enhancements on existing [C++] Changes (can be) made in C++. Previously named `Code` Game: Balance Balancing of (existing) in-game features. Mechanics: Character / Player Character / Player mechanics Mechanics: Effects / Skills / Stats Effects / Skills / Stats labels Sep 29, 2020
@I-am-Erk
Copy link
Member

Make sure to note this doesn't actually close #44370, it should say "partially fixes"

@Hatfish
Copy link

Hatfish commented Sep 30, 2020

Since this will be affecting the stamina pool, is there any chance that the indefatigable and languorous traits will get reworked to use this system?

@gkarfakis19
Copy link
Contributor Author

That's the plan.

@gkarfakis19 gkarfakis19 changed the title [WIP] [CR] Draft - Initial Cardio implementation. [WIP] [CR] Initial Cardio implementation. Oct 1, 2020
@gkarfakis19 gkarfakis19 marked this pull request as ready for review October 1, 2020 20:50
@stale
Copy link

stale bot commented Nov 1, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

@stale stale bot added the stale Closed for lack of activity, but still valid. label Nov 1, 2020
@I-am-Erk
Copy link
Member

I-am-Erk commented Nov 3, 2020

Go away stalebot, you're supposed to ignore feature freeze.

@stale stale bot removed the stale Closed for lack of activity, but still valid. label Nov 3, 2020
@TheDjur
Copy link

TheDjur commented Nov 26, 2020

I was just thinking about how to go about this myself (holy hell can i not code), but my idea was to tie it to one or more stats, since I'd assume most people would be using STS or STK

@gkarfakis19
Copy link
Contributor Author

I was just thinking about how to go about this myself (holy hell can i not code), but my idea was to tie it to one or more stats, since I'd assume most people would be using STS or STK

Please feel free to take this code and improve it. Due to new responsibilities I don't have much time to work on CDDA these days, and I'd hate for my work to be wasted.
However, I'd prefer sticking to the original plan of #44370 for now, unless senior developers change their mind on how it should be implemented. You cannot assume every person is running STS or STK and balance the game that way.

@I-am-Erk
Copy link
Member

I would very much like this merged, are you still around?

@gkarfakis19
Copy link
Contributor Author

Hey there. Unfortunately I am unable to dedicate any serious amount of time to modding anymore due to other obligations, but I’d be happy to see this merged, as long as someone else is there to implement the other half of this PR (cardio_acc).

@actual-nh actual-nh added PUBLIC TEST Come and try this! and removed 0.F Feature Freeze labels Jul 16, 2021
@@ -8265,9 +8267,12 @@ int Character::get_stamina() const
int Character::get_stamina_max() const
{
static const std::string player_max_stamina( "PLAYER_MAX_STAMINA" );
// ^^ this is now base max stamina, name not changed (for now) to avoid code changes.
Copy link
Member

Choose a reason for hiding this comment

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

comments go above the line of code you're referring to, not below.

Comment on lines +8273 to +8274
const int baseStamina = get_option< int >( player_max_stamina );
const int staminaMod = get_option< int >( player_cardio_stamina_mod );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const int baseStamina = get_option< int >( player_max_stamina );
const int staminaMod = get_option< int >( player_cardio_stamina_mod );
const int baseStamina = get_option<int>( player_max_stamina );
const int staminaMod = get_option<int>( player_cardio_stamina_mod );

Comment on lines +8394 to +8404
int Character::get_cardio() const
{
return cardio;
}

void Character::set_cardio( int new_cardio )
{
cardio = new_cardio;
}

void Character::update_cardio()
Copy link
Member

Choose a reason for hiding this comment

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

i would have this be a function that calculates cardio on the fly, instead of updating a cached value.

Copy link
Contributor

@actual-nh actual-nh Jul 30, 2021

Choose a reason for hiding this comment

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

It isn't caching cardio; it's adjusting - actually, simply setting - cardio. OK, I see what you mean.

Comment on lines +8435 to +8439
void Character::update_cardio_acc()
{
//WIP- LATER PR
cardio_acc = 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

maybe dont' include this until said later pr

Copy link
Contributor

Choose a reason for hiding this comment

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

If cardio_acc is 0, that leaves cardio based on half of BMR only.

@actual-nh actual-nh removed the PUBLIC TEST Come and try this! label Jul 30, 2021
@KorGgenT KorGgenT mentioned this pull request Jul 30, 2021
@stale
Copy link

stale bot commented Sep 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

@stale stale bot added the stale Closed for lack of activity, but still valid. label Sep 6, 2021
@wapcaplet
Copy link
Contributor

This pull request is now included in and superseded by #51880

@wapcaplet wapcaplet closed this Sep 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` <Enhancement / Feature> New features, or enhancements on existing Game: Balance Balancing of (existing) in-game features. Mechanics: Character / Player Character / Player mechanics Mechanics: Effects / Skills / Stats Effects / Skills / Stats stale Closed for lack of activity, but still valid.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants