[WIP] Create cardio #50339

KorGgenT · 2021-07-30T22:37:50Z

Summary

Features "Implements cardio to adjust weariness and stamina based on fitness"

see #44506 for all relevant discussion and original PR.
Closes #44506

Co-authored-by: George Karfakis <[email protected]>

Broken27 · 2021-08-02T15:33:53Z

What are the expected values of Cardio/stamina at different stages of the game?

Eg, an out of chargen survivor with 0 athletics, or a midgame survivor with athletics 5 and a good diet?

I-am-Erk · 2021-08-17T18:05:56Z

See #44370 for original issue and design.

I-am-Erk

Added a max cardio stat, and per some of Kevin's concerns, perhaps we should change the general name to 'cardiofit' for 'cardio fitness' to keep meanings clear.

src/character.cpp

src/vehicle.cpp

into pr/50339

actual-nh

Looking good! (There do not appear to be any effects of health on NPC stamina, but I doubt it matters.)

src/avatar.cpp

actual-nh · 2021-09-08T23:50:32Z

src/character.cpp


-    return string_format( "Weariness: %s Max Full Exert: %s Mult: %g\nBMR: %d %s Thresh: %d At: %d\nCal: %d/%d Fatigue: %d Morale: %d Wgt: %d (BMI %.1f)",
-                          amt, max_act, move_mult, bmr, weary_internals, thresh, current, get_stored_kcal(),
-                          get_healthy_kcal(), fatigue, morale, weight, bmi );
+    return string_format( "Weariness: %s Max Exertion: %s Mult: %g\nCARDIO FITNESS: %d %s Thresh: %d At: %d\n Calories: %d/%d Fatigue: %d Morale: %d Wgt: %d (BMI %.1f)",
+                          amt, max_act, move_mult, cardio_mult, activity_history.debug_weary_info(), thresh, current,
+                          get_stored_kcal(), get_healthy_kcal(), fatigue, morale, weight, bmi );
 }


After finishing testing cardio, could the original information be restored in addition to cardio (ideally in lines that aren't too terribly long)?

We should probably have both I would think, I added both back last time. Would you be willing to post a suggestion? My screen time is at an end.

actual-nh · 2021-09-09T01:11:27Z

The test failure for the Basic Build is that, when it tries to access metabolic_rate_base() for base_bmr(), via a float option, something goes wrong (and causes a coredump, so I don't think it's just it not able to find something).

actual-nh

A couple of places where base_bmr(), possibly modified, may be what is needed instead of get_bmr(), which includes effects from the current activity level.

src/character.cpp

Co-authored-by: actual-nh <[email protected]>

actual-nh

Here's a preliminary suggestion.

src/character.cpp

Co-authored-by: actual-nh <[email protected]>

I-am-Erk · 2021-09-09T16:00:51Z

Those seem reasonable to me for now. It's a debug line so if it's a bit hard to read it's not the end of the world.

src/character.cpp

This allows it to scale differently than base stamina so it doesn't always take 6 minutes to catch your breath.

src/character.cpp

I-am-Erk · 2021-09-17T23:15:15Z

OK. This is intense, I think this actually works. I had to adjust how the stamina regeneration equation works from my initial proposal, which actually simplifies this PR. your stamina regeneration rate now starts at 20 as before, but is multiplied by (get_cardiofitness()/base_bmr()), so that as your cardio fitness exceeds BMR your regen increases, or vice versa. This means if you suddenly get bigger your regeneration drops.

With the present balance, I did three tests:

A character with 0 health, 0 athletics, no traits, etc - can sprint for about 1:30 and recovers to full stamina in 6:45 This is comparable to previous.
A character with -200 health, 0 athletics - can sprint for about 1:10 and recovers to full stamina in 7:03. This doesn't represent a very low drop in cardiofit, it would be nice to test the lower ranges better.
A character with 0 health, 200 athletics - can run for 2:44, recovers to full stamina in 5:12.

The previous standard has a newly generated character running for 1:10 and recovering to full stamina in 7:30, which means I may need to slightly adjust the default stamina regeneration as I don't want to buff starting characters too strongly. Otherwise this seems to be working properly and well. The magnitude of effect might be low, if anything. With further debug tools to directly set cardio it would be easier to test but I do not think play balance will tank if we merge this as is.

Co-authored-by: Eric Pierce <[email protected]>

I-am-Erk · 2021-09-17T23:56:53Z

I am sure there will be some CI tests very angry about this.

From my perspective aside from solving the upcoming failed tests, this PR is ready to merge. I think we need a follow up PR relatively soon with:

some kind of @ menu information on your current cardio fitness, not a direct report but a little bit of information.
a debug command to view and set your current cardio_acc
the ability to adjust starting cardio_acc in professions so that some professions can begin more fit than others
Then we'll also need some new tests. However, I think this PR as it exists is probably ready for playtesting by the unsuspecting player base.

data/core/game_balance.json

wapcaplet · 2021-09-18T00:37:37Z

The "Custom" sidebar stamina graphs aren't quite accurate now with the flexible maximum - but the fix is easy, just remove the "var_max" field from both "stamina_graph" and "stamina_graph_classic", and the graphs will default to using Character::get_stamina_max() for their maxima:

Cataclysm-DDA/data/json/ui/sidebar.json

Line 240 in 6b325a9

"var_max": 10000,

Cataclysm-DDA/data/json/ui/sidebar.json

Line 251 in 6b325a9

"var_max": 10000,

P.S. This is resolved now

actual-nh

Barring unexpected errors, I have the first stage of accommodating sleep better with weariness (as in not in 5-minute blocks of "sleeping" or "not sleeping"). The next stage will be intermediate effects between LIGHT_EXERCISE, NO_EXERCISE, and SLEEP_EXERCISE. After that, mutations.

src/avatar.cpp

Co-authored-by: Eric Pierce <[email protected]>

Co-authored-by: actual-nh <[email protected]>

into pr/50339

src/activity_actor.cpp

wapcaplet · 2021-09-18T21:30:53Z

src/avatar.cpp

+void avatar::update_cardio_acc()
+{
+    const int prev_cardio_acc = get_cardio_acc();
+    const int last_24h_kcal = calorie_diary.begin()->spent;


Suggested change

const int last_24h_kcal = calorie_diary.begin()->spent;

const int last_24h_kcal = calorie_diary.front().spent;

Since you don't need an iterator here, front() would be more appropriate I think.

Also, if I'm not mistaken, the front of this array is only the spent calories since midnight - not the previous 24 hours.

I don't think so:

if( is_avatar() && ticks_between( from, to, 24_hours ) > 0 ) { as_avatar()->advance_daily_calories(); }

in character_body.cpp and:

void avatar::advance_daily_calories() { calorie_diary.push_front( daily_calories{} ); if( calorie_diary.size() > 30 ) { calorie_diary.pop_back(); } }

in avatar.cpp.

That's what I mean - every 24 hours, the front of this list gets a new (empty) calorie log. In order for this update_cardio_acc to work properly, I think it would need to be invoked from Character::update_body just before advance_daily_calories(), to capture the previous day's cardio level.

wapcaplet · 2021-09-18T21:33:32Z

src/avatar.cpp

@@ -1461,6 +1461,19 @@ bool avatar::invoke_item( item *used, const std::string &method )
    return Character::invoke_item( used, method );
 }

+void avatar::update_cardio_acc()


Is this function ever called? I don't see any invocations; this looks like dead code.

Afaict that's an error if so, I wonder if it got missed during one of the conflict merges

As I recall the first implementation wasn't going to include the accumulator so I wonder if it's not fully done. Seems like it would be easy to fix

I see now the comment from gkarfakis19 on the previous unfinished PR (repeated several times):

I do NOT plan on adding the exercise (cardio_acc) aspect of Cardio in this PR. It will come later.

It looks like it was implemented as requested, just not hooked up to anything. As noted in my other comment on this function, I think it would make sense to invoke it from Character::update_body just before the advance_daily_calories() call; that way it ought to include the previous 24 hours of activity from the diary.

wapcaplet · 2021-09-19T21:19:15Z

I have been doing some playtesting with this using #51701 to see what's going on with get_cardiofit and get_cardio_acc. This is a freshly created default scenario character:

I believe this large random value may be because cardio_acc is uninitialized in the Character constructor.

After saving and loading, the values are much more reasonable:

gkarfakis19 · 2021-09-20T10:29:18Z

I just saw this pull request. It's great to see my old code (hopefully it wasn't too messy!) get used by someone more experienced that is able to properly support such a game-changing feature.

Excited to see where this leads!

wapcaplet · 2021-09-25T01:01:54Z

src/character.cpp

@@ -482,6 +482,7 @@ Character::Character() :
    }
    // Only call these if game is initialized
    if( !!g && json_flag::is_ready() ) {
+        set_cardio_acc( base_bmr() / 2 );


Following up on my comment:

I believe this large random value may be because cardio_acc is uninitialized in the Character constructor.

Here is where the cardio_acc initialization should occur - but it is gated by json_flag::is_ready(), which for whatever reason is false when the Character constructor is called. I believe the reason for stipulating this condition is that base_bmr() needs metabolic_rate_base(), which reads from game_balance.json the setting for PLAYER_HUNGER_RATE (default 1.0) - so it needs this is_ready() flag to be true in order to read that balance setting.

If we can find a way to initialize cardio_acc to a reasonable value without depending on base_bmr(), that should resolve the problem with new characters having wildly random cardio values. Or, find a way to ensure json_flag::is_ready() is true before we reach this constructor.

I will figure out a solution for this in the new PR.

wapcaplet · 2021-09-25T19:16:18Z

After consulting with @KorGgenT and @I-am-Erk , I went ahead and started another fresh pull request with a clean rebase from a more recent point in master, including everything from this PR as well as some test cases and new sidebar widgets: #51880

Sorry to play musical chairs on everyone following this feature, but let us continue the review and conversation over there.

Create cardio

1689652

Co-authored-by: George Karfakis <[email protected]>

KorGgenT changed the title ~~Create cardio~~ [WIP] Create cardio Jul 30, 2021

KorGgenT removed Mechanics: Enchantments / Spells Enchantments and spells Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies labels Jul 31, 2021

I-am-Erk reviewed Aug 17, 2021

View reviewed changes

I-am-Erk added 7 commits August 17, 2021 13:07

Add cap to cardio

45fd624

Apply suggestions from code review

9761db7

Merge remote-tracking branch 'upstream/master' into pr/50339

9fc09bc

Update game_balance.json

ee3fa54

astyle and shut off some NPC functions

d6dfe29

Merge branch 'cardio-stuff' of https://github.com/KorGgenT/Cataclysm-DDA

b569fc4

into pr/50339

variable name consistency

aa4aae0

actual-nh reviewed Sep 9, 2021

View reviewed changes

src/character.cpp Outdated Show resolved Hide resolved

src/character.cpp Outdated Show resolved Hide resolved

I-am-Erk and others added 2 commits September 8, 2021 21:27

Apply suggestions from code review

5faa42f

Co-authored-by: actual-nh <[email protected]>

Apply suggestions from code review

0b05641

actual-nh reviewed Sep 9, 2021

View reviewed changes

src/character.cpp Show resolved Hide resolved

src/character.cpp Outdated Show resolved Hide resolved

I-am-Erk and others added 2 commits September 9, 2021 08:59

Update src/character.cpp

6435ec9

Co-authored-by: actual-nh <[email protected]>

Update src/character.cpp

1730a0f

Co-authored-by: actual-nh <[email protected]>

wapcaplet reviewed Sep 17, 2021

View reviewed changes

src/character.cpp Outdated Show resolved Hide resolved

redo how we calculate stamina regen

4980121

This allows it to scale differently than base stamina so it doesn't always take 6 minutes to catch your breath.

wapcaplet reviewed Sep 17, 2021

View reviewed changes

src/character.cpp Outdated Show resolved Hide resolved

I-am-Erk and others added 3 commits September 17, 2021 16:19

missed a spot

95f36e9

Apply suggestions from code review

6c11ee9

Co-authored-by: Eric Pierce <[email protected]>

astyle

5d3ae6a

wapcaplet reviewed Sep 18, 2021

View reviewed changes

data/core/game_balance.json Outdated Show resolved Hide resolved

actual-nh reviewed Sep 18, 2021

View reviewed changes

src/avatar.cpp Outdated Show resolved Hide resolved

I-am-Erk and others added 4 commits September 17, 2021 18:20

Update data/core/game_balance.json

ce61cd5

Co-authored-by: Eric Pierce <[email protected]>

Update src/avatar.cpp

6a25317

Co-authored-by: actual-nh <[email protected]>

per wapcaplet recommendation

9563840

Merge branch 'cardio-stuff' of https://github.com/KorGgenT/Cataclysm-DDA

4eec05f

into pr/50339

cainiaowu reviewed Sep 18, 2021

View reviewed changes

src/activity_actor.cpp Outdated Show resolved Hide resolved

fix an error with stamina calculations in archery

287623c

wapcaplet reviewed Sep 18, 2021

View reviewed changes

src/activity_actor.cpp Outdated Show resolved Hide resolved

use a flat stamina calculation for muscle powered weapons

0bd5a9d

wapcaplet reviewed Sep 18, 2021

View reviewed changes

This was referenced Sep 19, 2021

Add widgets for cardio fitness and cardio accumulator #51701

Closed

Add cardio test cases #51702

Closed

Termineitor244 mentioned this pull request Sep 20, 2021

Athletics practice #51730

Merged

wapcaplet reviewed Sep 25, 2021

View reviewed changes

wapcaplet mentioned this pull request Sep 25, 2021

Implement cardio, use cardio for stamina, and adjust cardio fitness over time #51880

Merged

5 tasks

wapcaplet closed this Sep 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Create cardio #50339

[WIP] Create cardio #50339

KorGgenT commented Jul 30, 2021 •

edited by I-am-Erk

Loading

Broken27 commented Aug 2, 2021

I-am-Erk commented Aug 17, 2021

I-am-Erk left a comment

actual-nh left a comment •

edited

Loading

actual-nh Sep 8, 2021

I-am-Erk Sep 9, 2021

actual-nh commented Sep 9, 2021

actual-nh left a comment

actual-nh left a comment

I-am-Erk commented Sep 9, 2021

I-am-Erk commented Sep 17, 2021 •

edited

Loading

I-am-Erk commented Sep 17, 2021

wapcaplet commented Sep 18, 2021 •

edited

Loading

actual-nh left a comment

wapcaplet Sep 18, 2021

wapcaplet Sep 18, 2021

actual-nh Sep 19, 2021

wapcaplet Sep 19, 2021

wapcaplet Sep 18, 2021

I-am-Erk Sep 19, 2021 •

edited

Loading

wapcaplet Sep 19, 2021

wapcaplet commented Sep 19, 2021

gkarfakis19 commented Sep 20, 2021

wapcaplet Sep 25, 2021 •

edited

Loading

wapcaplet Sep 25, 2021

wapcaplet commented Sep 25, 2021

	const int last_24h_kcal = calorie_diary.begin()->spent;
	const int last_24h_kcal = calorie_diary.front().spent;

[WIP] Create cardio #50339

[WIP] Create cardio #50339

Conversation

KorGgenT commented Jul 30, 2021 • edited by I-am-Erk Loading

Summary

Broken27 commented Aug 2, 2021

I-am-Erk commented Aug 17, 2021

I-am-Erk left a comment

Choose a reason for hiding this comment

actual-nh left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

actual-nh commented Sep 9, 2021

actual-nh left a comment

Choose a reason for hiding this comment

actual-nh left a comment

Choose a reason for hiding this comment

I-am-Erk commented Sep 9, 2021

I-am-Erk commented Sep 17, 2021 • edited Loading

I-am-Erk commented Sep 17, 2021

wapcaplet commented Sep 18, 2021 • edited Loading

actual-nh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

I-am-Erk Sep 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wapcaplet commented Sep 19, 2021

gkarfakis19 commented Sep 20, 2021

wapcaplet Sep 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wapcaplet commented Sep 25, 2021

KorGgenT commented Jul 30, 2021 •

edited by I-am-Erk

Loading

actual-nh left a comment •

edited

Loading

I-am-Erk commented Sep 17, 2021 •

edited

Loading

wapcaplet commented Sep 18, 2021 •

edited

Loading

I-am-Erk Sep 19, 2021 •

edited

Loading

wapcaplet Sep 25, 2021 •

edited

Loading