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

feat: Effects adding other effects on removal #3695

Merged
merged 24 commits into from
Dec 21, 2023

Conversation

Coolthulhu
Copy link
Member

@Coolthulhu Coolthulhu commented Nov 16, 2023

Purpose of change

This PR allows defining comedowns and similar effects that can coexist with the parent effect and can be defined by mods.

Describe the solution

When effect expires, it can create other effects, at set duration/intensity, or inheriting duration/intensity from the decaying effect.
New effects are added at the end of the effect processing loop, and so should not be able to cause effect map migration. They are added in the same order as the parent effects were processed.
There is an option to only affect decaying (0 duration) effects, only affect removed effects (remove_effect when duration > 0), or both.

Reimplemented adrenaline and its comedown in the new system. This means that both effects can now coexist - you can have adrenaline buff with comedown debuff applied at the same time.

Describe alternatives you've considered

  • Waiting for lua to cover it, with associated problems, such as lack of common standard of effect decay
  • Leaving it hardcoded, since we don't have many effects that could use this feature (possibly because we didn't have this feature)

Testing

  • Tests pass
  • Spawn 3x adrenaline
  • Use adrenaline
  • Wait for it to decay
  • Apply adrenaline
  • Confirm both buff and debuff exist
  • Wait for both to time out
  • Apply adrenaline
  • Use blood filtering bionic
  • Confirm that adrenaline comedown is present

Additional context

Checklist

  • Body part inheritance
  • Fixing adrenaline:
    • Rebalance duration, to account for debuff no longer being included in "common duration"
    • Remove hardcoded addition of debuff from blood filtering and possibly others
  • Docs

@github-actions github-actions bot added src changes related to source code. JSON related to game datas in JSON format. labels Nov 16, 2023
@github-actions github-actions bot added the mods PR changes related to mods. label Nov 24, 2023
Copy link
Contributor

autofix-ci bot commented Nov 24, 2023

The Autofix app has automatically formatted this Pull Request.

If you edit your PR on web UI, you can ignore this message.
If you edit your PR locally, YOU MUST DO EITHER OF THE FOLLOWING:

  • Run git pull to merge the automated commit into your local copy of the PR branch.
  • Format your code locally, and force push to your PR branch.

If you don't do this, your following work will be based on the old commit, and cause MERGE CONFLICT.

@Coolthulhu Coolthulhu marked this pull request as ready for review November 24, 2023 12:43
@github-actions github-actions bot added the docs PRs releated to docs page label Nov 24, 2023
src/effect.h Outdated Show resolved Hide resolved
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

  • Tests pass
  • Spawn 3x adrenaline
  • Use adrenaline
  • Wait for it to decay
  • Apply adrenaline
  • Confirm both buff and debuff exist

image
image
image

works as intended, however on comedown it errors with:

 DEBUG    : player_morale::perceived_pain is inconsistent.

 FUNCTION : bool player_morale::consistent_with(const player_morale &) const
 FILE     : /home/scarf/repo/cata/Cataclysm/src/morale.cpp
 LINE     : 878
 VERSION  : BN 8b14530d31ec
  • Wait for both to time out
  • Apply adrenaline
  • Use blood filtering bionic
  • Confirm that adrenaline comedown is present

image

even after using blood filter CBM, the effect does not wear off.

@github-actions github-actions bot added the tests changes related to tests label Nov 26, 2023
@scarf005 scarf005 self-requested a review November 28, 2023 23:47
@scarf005 scarf005 force-pushed the effect-adding-effect branch from a1b6779 to 022a28f Compare December 10, 2023 00:59
@scarf005 scarf005 self-assigned this Dec 14, 2023
@Coolthulhu
Copy link
Member Author

I didn't realize that blood filter was not removing adrenaline. That's why it wasn't triggering the aftereffect.

Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

image

LGTM.

@scarf005 scarf005 merged commit 3abde26 into cataclysmbnteam:main Dec 21, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs PRs releated to docs page JSON related to game datas in JSON format. mods PR changes related to mods. src changes related to source code. tests changes related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants