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

New chemical based artifact nodes #30873

Merged
merged 18 commits into from
Sep 3, 2024

Conversation

Lyroth001
Copy link
Contributor

@Lyroth001 Lyroth001 commented Aug 10, 2024

About the PR

Added 6 new artifact triggers, with 4 new hints, for artifacts.
Adds pH to the guidebook entries for chems used by the pH nodes, since there is no implementation of pH.

The nodes are

  • Medication (Min depth 2. Activated by Dylovene, Diphenhydramine, Arithrazine, Bicaridine, Dermaline, Dexalin or DexalinPlus.)

  • Acidic (Min depth 2. Activated by Vinegar, Polytrinic Acid, Ferrochromic Acid, Fluorosulfuric Acid, Sulfuric Acid, Artifexium or Norepinephric Acid)

  • Basic (Min depth 2. Activated by Sodium Carbonate, Hydroxide or Sodium Hydroxide)

  • Psychedelics (Min depth 3. Activated by Impedrezene, Space Mirage, Bananadine, Happiness, Heartbreaker Toxin, Mindbreaker Toxin or Pax )

  • Stimulants (Min depth 3. Activated by Desoxyephedrine, Ephedrine or Stimulants)

  • Pyrotechnics (Min depth 3. Activated by Thermite, Napalm, Phlogiston or Chlorine Trifluoride )

The hints are

  • ph - Extreme pH Level (Used for acidic and basic)
  • medical - Therapeutic chemicals (used for Medication)
  • drugs - Psychoactive chemicals (used for Psychedelics and Stimulants)
  • energetic - Highly energetic chemicals (Used for pyrotechnics)

Why / Balance

This adds new gameplay for scientists due to needing to obtain various chems to continue artifact research. This also means that chemistry will have more variety in the chems they are asked to make, especially early in rounds before sci can setup their own lab. No longer do they only get requests for medical chems, mutagen, and whatever the clown wants! Some of the chemicals are deliberately dangerous and/or illegal, this adds gameplay for sec (whats that scientist carrying? Do we need to test it? Is that tider really bringing LSD 'for sci'?) with minor, non-antag crimes to deal with. This then adds more to differentiate RD from just a scientist with their own office, since they can write documents to allow people to carry these chems, and prove its actually for science.

Technical details

6 trigger added to artifact_triggers.yml, same as other chemical nodes

Media

Screenshot 2024-08-10 181418

(Not sure the best way to create specific artifacts to demo, but they're all implemented the same way, and the same as other reaction nodes)

Requirements

  • I have read and I am following the Pull Request Guidelines. I understand that not doing so may get my pr closed at maintainer’s discretion
  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

Changelog

🆑
-add: added a new artifact node for medical chems

@github-actions github-actions bot added the Changes: No C# Changes: Requires no C# knowledge to review or fix this item. label Aug 10, 2024
@Lyroth001
Copy link
Contributor Author

going to try and get some more screenshots, but just wanted to get the pr up for now

Comment on lines 124 to 132
components:
- type: reactive
groups:
Acidic: [ Touch ]
reactions:
- reagents: [ Dylovene, Diphenhydramine, Arithrazine, Bicaridine, Dermaline, Dexalin, DexalinPlus ] # adding all medical chems seems. Excessive, and also long. Ive just added the main ones for now
methods: [ Touch ]
effects:
- !type:ActivateArtifact
Copy link
Contributor

@Aeshus Aeshus Aug 10, 2024

Choose a reason for hiding this comment

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

I think this has the wrong indentation level?

Also, you keep using "reactive" instead of "Reactive" (this is case sensitive).

Suggested change
components:
- type: reactive
groups:
Acidic: [ Touch ]
reactions:
- reagents: [ Dylovene, Diphenhydramine, Arithrazine, Bicaridine, Dermaline, Dexalin, DexalinPlus ] # adding all medical chems seems. Excessive, and also long. Ive just added the main ones for now
methods: [ Touch ]
effects:
- !type:ActivateArtifact
components:
- type: Reactive
groups:
Acidic: [ Touch ]
reactions:
- reagents: [ Dylovene, Diphenhydramine, Arithrazine, Bicaridine, Dermaline, Dexalin, DexalinPlus ] # adding all medical chems seems. Excessive, and also long. Ive just added the main ones for now
methods: [ Touch ]
effects:
- !type:ActivateArtifact

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review! Pretty sure i've fixed all the indentation and capitalisation issues now hopefully, appreciate it!

@Lyroth001
Copy link
Contributor Author

Yaml linter was successful so i think we good

@LankLTE
Copy link
Contributor

LankLTE commented Aug 10, 2024

The only one I'm concerned about is the "basic" one, since I don't expect players to really know what that means at all. The rest are self-explanatory enough (or explained well enough in the guidebook) to be fine

@Lyroth001
Copy link
Contributor Author

The only one I'm concerned about is the "basic" one, since I don't expect players to really know what that means at all. The rest are self-explanatory enough (or explained well enough in the guidebook) to be fine

The word basic never gets shown to players, just extreme pH level. All valid chems have pHs in the guidebooks, so you'd just try one low and one high imo

@LankLTE
Copy link
Contributor

LankLTE commented Aug 10, 2024

All valid chems have pHs in the guidebooks, so you'd just try one low and one high imo

From the looks of it, this just isn't true? I took a quick look and didn't see pH mentioned anywhere.

also I realized there's different hints, imho just saying the name of the node is gonna be enough, since that already means you need to splash it with some specific chems that meet that criteria which is about on-par with stuff like radiation or especially reaction with hematological fluid. Having one hint that can refer to 3 completely different chemicals needing to be splashed seems a little unnecessary compared to just telling them which group of chems to use.

@Lyroth001
Copy link
Contributor Author

Lyroth001 commented Aug 11, 2024

All valid chems have pHs in the guidebooks, so you'd just try one low and one high imo

From the looks of it, this just isn't true? I took a quick look and didn't see pH mentioned anywhere.

also I realized there's different hints, imho just saying the name of the node is gonna be enough, since that already means you need to splash it with some specific chems that meet that criteria which is about on-par with stuff like radiation or especially reaction with hematological fluid. Having one hint that can refer to 3 completely different chemicals needing to be splashed seems a little unnecessary compared to just telling them which group of chems to use.

By valid i mean the ones that are accepted for the nodes (i.e. defined in the reaction node as valid to trigger it). Those ones have pHs. I havent given every single chem a pH. If you see the ss i attached thats for a basic (as in high pH) node, and the hint says Extreme pH level

And the reason I've used the same hints for multiple nodes is to follow the advice at the top of the arti hints fluent, and also so that you have to try a few different chems when you get one of those nodes. It encourages good notekeeping of what a given arti wants as well, which is just good sci habits

@Lyroth001
Copy link
Contributor Author

pH has been implemented as text in guidebook and list of chems for now as i don't want to touch actual chemical stuff before #30254 is merged.

would be nice for solutions to have an actual pH, but that can be done later

Copy link
Member

@slarticodefast slarticodefast left a comment

Choose a reason for hiding this comment

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

Looks good from a coding perspective. I tested it in-game and it works as intended. Gameplay-wise I am not sure if this would make science too dependent on chemistry, but I can bring it up for discussion with the other maintainers.

@@ -5,19 +5,19 @@ reagent-name-phenol = phenol
reagent-desc-phenol = An aromatic ring of carbon with a hydroxyl group. A useful precursor to some medicines, but has no healing properties on its own.

reagent-name-sodium-carbonate = sodium carbonate
reagent-desc-sodium-carbonate = A white, odorless, water-soluble salt that yields an alkaline solution in water. Also known as soda ash.
reagent-desc-sodium-carbonate = A white, odorless, water-soluble salt that yields an alkaline solution in water. Also known as soda ash. pH = 11.6
Copy link
Member

@slarticodefast slarticodefast Aug 13, 2024

Choose a reason for hiding this comment

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

I'm not sure if I like the description addition this way since this might cause confusion for chemists wondering if this has an impact on gameplay and it makes them inconsistent with other reagents without an pH value. It might be fine to remove it considering there is an "alkaline" or "acid" in all names/descriptions. Or keep only the acidic trigger variant.

groups:
Acidic: [ Touch ]
reactions:
- reagents: [ Dylovene, Diphenhydramine, Arithrazine, Bicaridine, Dermaline, Dexalin, DexalinPlus ] # adding all medical chems seems. Excessive, and also long. Ive just added the main ones for now
Copy link
Member

Choose a reason for hiding this comment

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

It would be a long list, but a few more of the main ones I can think of are
Tricordrazine, Leporazine, Bruizine, Lacerinol, Puncturase, Pyrazine, Insuzine, Kelotane, Hyronalin, Inaprovaline and Epinephrine
Adding those should be enough

groups:
Acidic: [ Touch ]
reactions:
- reagents: [ Vinegar, PolytrinicAcid, FerrochromicAcid, FluorosulfuricAcid, SulfuricAcid, Artifexium, NorepinephricAcid ]
Copy link
Member

Choose a reason for hiding this comment

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

I think you can remove artifexium since it already triggers all artifacts anyways.
Should TranexamicAcid be added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it says acid i think that makes sense, would probably be expected

groups:
Acidic: [ Touch ]
reactions:
- reagents: [ Impedrezene, SpaceDrugs, Bananadine, Happiness, HeartbreakerToxin, MindbreakerToxin, Pax ]
Copy link
Member

Choose a reason for hiding this comment

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

Add THC, remove HeartbreakerToxin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

main reason for heartbreaker was it says its a hallucinogen in the guidebook desc

@github-actions github-actions bot added the S: Needs Review Status: Requires additional reviews before being fully accepted label Aug 15, 2024
@deathride58 deathride58 added the S: Undergoing Maintainer Discussion Status: Currently going through an extended discussion amongst maintainers, as per procedure. label Aug 19, 2024
@slarticodefast
Copy link
Member

Hey, I brought this up for discussion and the maintainers came to the agreement it would be best to only keep only the new medication node. The main reason for this is that science should not be too dependent on the chemistry department. And with 6 different nodes one of them would show up on most artifacts with a high probability. If you could make that change this should be good to merge.

@slarticodefast slarticodefast added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted labels Aug 23, 2024
@Aeshus
Copy link
Contributor

Aeshus commented Aug 23, 2024

What about a trigger based on the botany chemicals (Robust Harvest, EZ Nutrient, maybe Mutagen?, etc)?

Botany would start with a supply to share and it would incentivize chemistry into making chemicals useful for botany as they're useful for xeno arch too.

@slarticodefast
Copy link
Member

What about a trigger based on the botany chemicals (Robust Harvest, EZ Nutrient, maybe Mutagen?, etc)?
Botany would start with a supply to share and it would incentivize chemistry into making chemicals useful for botany as they're useful for xeno arch too.

Hard to say. But if you look at the current ones we only have chemical triggers with reagents that are widely available, like water and blood. Medicine fits that condition as well since you can always find it somewhere, for example as pills. Later science will surely have access to more reagents, for example when the Mutant Lab proposal gets implemented.

@UbaserB
Copy link
Member

UbaserB commented Aug 25, 2024

Hard to say. But if you look at the current ones we only have chemical triggers with reagents that are widely available, like water and blood. Medicine fits that condition as well since you can always find it somewhere, for example as pills. Later science will surely have access to more reagents, for example when the Mutant Lab proposal gets implemented.

I would argue no, since it's a bit more difficult/specific (+ time consuming) and one node is already enough to foster inter-departmental relationships

@UbaserB
Copy link
Member

UbaserB commented Sep 3, 2024

You coming back to this?

@Lyroth001
Copy link
Contributor Author

You coming back to this?

Yea, just been needing to find the time to actually do it lmao

@Lyroth001
Copy link
Contributor Author

What about a trigger based on the botany chemicals (Robust Harvest, EZ Nutrient, maybe Mutagen?, etc)?
Botany would start with a supply to share and it would incentivize chemistry into making chemicals useful for botany as they're useful for xeno arch too.

Hard to say. But if you look at the current ones we only have chemical triggers with reagents that are widely available, like water and blood. Medicine fits that condition as well since you can always find it somewhere, for example as pills. Later science will surely have access to more reagents, for example when the Mutant Lab proposal gets implemented.

The main idea with a lot of these nodes was to give chem an excuse to make the more uncommon regents, as opposed to just, im bored lets make meth/oh youre making explosives - antag, but i do see how 6 is probably too many lols

@slarticodefast slarticodefast removed the S: Awaiting Changes Status: Changes are required before another review can happen label Sep 3, 2024
@slarticodefast
Copy link
Member

Thank you for your contribution!

@slarticodefast slarticodefast merged commit cb88750 into space-wizards:master Sep 3, 2024
11 checks passed
@UbaserB UbaserB removed the S: Undergoing Maintainer Discussion Status: Currently going through an extended discussion amongst maintainers, as per procedure. label Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: No C# Changes: Requires no C# knowledge to review or fix this item.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants