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

Fix Doctors Delight metabolism Rate #32297

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BramvanZijp
Copy link
Contributor

@BramvanZijp BramvanZijp commented Sep 19, 2024

About the PR

Combined the Drink and Medicine metabolism types to just be Drink, which now includes the health change effect in order to avoid it using 0.5 per second twice and thus 1 per second, while still only using 0.5 per second for the healing, aka halving the effectiveness.

Why / Balance

It currently using 1 per second is likely an unintentional side-effect of it having both the medicine metabolism and the drink metabolism.

Technical details

N/A

Media

This PR does not require an ingame showcase.

Requirements

Changelog

🆑 BramvanZijp

  • fix: Fixed an issue that caused Doctors Delight to metabolize twice the normal speed, which also halved its effectiveness.

@github-actions github-actions bot added the No C# For things that don't need code. label Sep 19, 2024
@thebadman4662
Copy link

thebadman4662 commented Sep 19, 2024

The core issue is how organ metabolism works, while it will fix it compared to original for non-core species, diona and slimes will now have 400% healing instead of 200%.

Either organs need to remove chems properly after/before this PR or chems(and there is whole list of them to be made) need to either be assigned to medicine/poison/narcotic OR drinks/food to prevent buggy metabolism interactions. Cores separately metabolizing stomach and heart reagent groups could also help if multi-metabolism is desired by maintainters.

#29393

@BramvanZijp
Copy link
Contributor Author

The core issue is how organ metabolism works, while it will fix it compared to original for non-core species, diona and slimes will now have 400% healing instead of 200%.

Either organs need to remove chems properly after/before this PR or chems(and there is whole list of them to be made) need to either be assigned to medicine/poison/narcotic OR drinks/food to prevent buggy metabolism interactions. Cores separately metabolizing stomach and heart reagent groups could also help if multi-metabolism is desired by maintainters.

#29393

What if we make the drink aspect use 0 and the medicine aspect use 0.5?

@thebadman4662
Copy link

thebadman4662 commented Sep 19, 2024

As in 0 reagents used but still requiring stomach/core organ for food/drink tag effects? Then I believe it would use metabolism slot but give otherwise "free" effects. Other than that it should fix some species getting way less and others way more healing though.

Is only TDD in scope of this PR? Wondering if I should make list of ALL reagents that are affected with multi-organ tags. If its only TDD I guess ill just put it sometime later when I remake the original issue and rename.

@HandsomeTheEgg
Copy link

HandsomeTheEgg commented Sep 20, 2024

What if we make the drink aspect use 0 and the medicine aspect use 0.5?

if i may throw something in , would it be bad to just make it food/drink aspect and no medicine tag ?(so basicly just adding the healing effects to the drink aspect, and removing medical) There arent many drinks that actuall provide healing or special effects so having on in that group doesnt sound bad

@thebadman4662
Copy link

thebadman4662 commented Sep 20, 2024

Making drinks provide healing effects just like Saline together with normal hydration would be my choice when making my own PR, yeah. Theres plenty of heart reagents and using stomach metabolism would make them more worth making.

@HandsomeTheEgg
Copy link

yea i mean overall i am glad to see someone else also cares about food/drink related things =) if it only has a medical tag , or drink tag , if ifts metabolize is adjusted (but that one seems trickier than others to get it working) , sounds all good to me and i am excited to see when it gets merged.

@BramvanZijp
Copy link
Contributor Author

What if we make the drink aspect use 0 and the medicine aspect use 0.5?

if i may throw something in , would it be bad to just make it food/drink aspect and no medicine tag ?(so basicly just adding the healing effects to the drink aspect, and removing medical) There arent many drinks that actuall provide healing or special effects so having on in that group doesnt sound bad

Thats probably the better move here, Ill update it.

@slarticodefast slarticodefast added the Undergoing Maintainer Discussion This PR is currently going through the 72-hour discussion window as per maintainer policy label Sep 21, 2024
- !type:HealthChange
damage:
groups:
Burn: -1
Brute: -1
Airloss: -1
Toxin: -1

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

remove extra new line

@@ -867,6 +867,7 @@
metamorphicChangeColor: false
metabolisms:
Drink:
metabolismRate: 0.5
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
metabolismRate: 0.5

Isn't this the default rate? So it can be removed I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No C# For things that don't need code. Undergoing Maintainer Discussion This PR is currently going through the 72-hour discussion window as per maintainer policy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants