Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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] Create cardio #50339
[WIP] Create cardio #50339
Changes from 21 commits
1689652
45fd624
9761db7
9fc09bc
ee3fa54
d6dfe29
b569fc4
aa4aae0
5faa42f
0b05641
6435ec9
1730a0f
3394a5d
1bd40b9
e951cfe
52ee3b2
6ee0725
4980121
95f36e9
6c11ee9
5d3ae6a
ce61cd5
6a25317
9563840
4eec05f
287623c
0bd5a9d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is this function ever called? I don't see any invocations; this looks like dead code.
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.
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
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.
I see now the comment from gkarfakis19 on the previous unfinished PR (repeated several times):
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 theadvance_daily_calories()
call; that way it ought to include the previous 24 hours of activity from the diary.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.
Since you don't need an iterator here,
front()
would be more appropriate I think.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.
Also, if I'm not mistaken, the front of this array is only the spent calories since midnight - not the previous 24 hours.
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.
I don't think so:
in character_body.cpp and:
in avatar.cpp.
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.
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 fromCharacter::update_body
just beforeadvance_daily_calories()
, to capture the previous day's cardio level.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.
Following up on my comment:
Here is where the
cardio_acc
initialization should occur - but it is gated byjson_flag::is_ready()
, which for whatever reason is false when theCharacter
constructor is called. I believe the reason for stipulating this condition is thatbase_bmr()
needsmetabolic_rate_base()
, which reads fromgame_balance.json
the setting forPLAYER_HUNGER_RATE
(default 1.0) - so it needs thisis_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 onbase_bmr()
, that should resolve the problem with new characters having wildly random cardio values. Or, find a way to ensurejson_flag::is_ready()
is true before we reach this constructor.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.
I will figure out a solution for this in the new PR.