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

Capping shakes effect #51250

Merged
merged 21 commits into from
Sep 3, 2021
Merged

Conversation

jim-huynh
Copy link
Contributor

@jim-huynh jim-huynh commented Aug 31, 2021

Summary

Bugfixes "Capping shakes duration to prevent month-long shakes after withdrawal is gone"

Purpose of change

Currently the shakes effect is uncapped and leads to people starting with addictions having the shakes effect last for a month if not longer - after their withdrawal is done with. This change caps the shakes effect to 2 hours so that it goes away pretty quickly after the withdrawal effect is gone.

I picked 2 hours because 1 hour, while good, cuts close to the max initial duration of speed with high intensity (in * 2_minutes).

Describe the solution

Set a max_duration of 7200 for the shakes effect. Also set a dur_add_perc of 30%, so that it takes more than one or two triggers to hit max duration.

Describe alternatives you've considered

Add coding to the addictions.cpp to put the cap there. The JSON change is simpler and doesn't seem to have a noticeable effect on other conditions that trigger the shakes effect. The closest problem I saw is for Jittery + stim, but that has a max duration of 30_minutes + 1_turns * stim, so a 2 hour limit should allow for really high stim + jittery shakes.

Testing

Created a new character and verified that the shakes duration for the character does not go over 7200.

Additional context

Issue reported in:
#50554
#48520
#51063
#48171

jim-huynh and others added 21 commits June 14, 2020 08:44
…point multiple times. Adjust wall support value to be more supportive for higher up floors. Make stairs up support roofs because the connected downstairs is the roof being supported.
Co-authored-by: Anton Burmistrov <Night_Pryanik@mail.ru>
@actual-nh actual-nh added [JSON] Changes (can be) made in JSON Game: Balance Balancing of (existing) in-game features. Mechanics: Character / Player Character / Player mechanics Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies labels Aug 31, 2021
@actual-nh
Copy link
Contributor

actual-nh commented Aug 31, 2021

Ping: @Venera3 - any comment? I can see capping it to no more than the withdrawal period - provided the withdrawal period is a realistic length for the addiction in question; uncertain if it is.

@jim-huynh
Copy link
Contributor Author

Ping: @Venera3 - any comment? I can see capping it to no more than the withdrawal period - provided the withdrawal period is a realistic length for the addiction in question; uncertain if it is.

Yeah, I agree with that. This change is just to resolve a confusion point that is brought up in many tickets - a player's withdrawal tag is gone but their shakes effect persists for a over a month afterward.

The default heroin addict start puts you at 40 intensity addiction, with 6 hours per addiction point decrement - so less than 2 weeks to be cured of the addiction, but the shakes for that will last a month, with RNG's blessing/curse.

@actual-nh
Copy link
Contributor

actual-nh commented Aug 31, 2021

It should (at least now) be possible to have the shakes automatically terminate once the character has finished withdrawing. @Ramza13? NOTE: This will require having a different shakes effect (even if named the same, with a different id) so that the shakes can be induced from something else.

@Venera3
Copy link
Member

Venera3 commented Aug 31, 2021

Addictions could use a realism pass in general (not it.), but this closes a weird random fail state that's not intuitive for the player.

Copy link
Member

@Maleclypse Maleclypse left a comment

Choose a reason for hiding this comment

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

If I understand correctly this will limit shakes to lasting 2 hours past when the withdrawal effect is gone from the character? If so I think this makes sense at least until if/when withdrawals are moved to the eoc system. I think most of the hardcoded_effects.cpp could be moved to EOC but I'm probably not the brave soul to undertake that.

@jim-huynh
Copy link
Contributor Author

@I-am-Erk @Maleclypse Is there something else that I need to do for this PR to be committed, or should it be closed/cancelled?

@Maleclypse
Copy link
Member

Sorry there are very few people with merge rights on the team and currently even easy changes are often taking weeks to get merged. Mostly due to random failures in our test suite that they are trying to track down. We appreciate if you are able to be patient with them. A note, we have roughly a hundred people or more a month who are contributing PRs and three active mergers. They have been merging fifty of more PRs each week and we can’t get below 150 active PRs for a full day.

@jim-huynh
Copy link
Contributor Author

Ah ok. Yeah I noticed that the PR count goes back up pretty quickly after it dips a bit. I get how time consuming it is to do PR reviews with any thoroughness, I was just wondering if there was something else that I needed to do.

Thanks for the update!

@ZhilkinSerg ZhilkinSerg merged commit 9ee1f8a into CleverRaven:master Sep 3, 2021
Venera3 pushed a commit to Venera3/Cataclysm-DDA that referenced this pull request Sep 21, 2021
BrettDong added a commit that referenced this pull request Nov 27, 2021
ZhilkinSerg pushed a commit that referenced this pull request Nov 27, 2021
@jim-huynh jim-huynh deleted the capping_shakes_effect branch June 28, 2024 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Game: Balance Balancing of (existing) in-game features. [JSON] Changes (can be) made in JSON Mechanics: Character / Player Character / Player mechanics Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants