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

Muffled Gas Masks #197

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Conversation

sarcoph
Copy link

@sarcoph sarcoph commented Sep 21, 2024

Description

This PR adds "muffled" variants to three different gas mask types; the regular gas mask, explorer gas masks, and security gas masks. These muffled gas masks combine the best of both muzzles and gas masks, allowing you to breathe from an internals tank but not allowing you to speak like a normal person.

Why? It's practical. It has sex appeal. It's giving me character ideas. We are one step closer to gear fetish roleplay. It's fucking genius

As another note, this was originally meant to be for a larger PR that involved adding a lot more BDSM/bondage gear (more muzzle and handcuff variants), but this specific feature involved minorly refactoring upstream YAML in a kind of questionable way, so I am PRing this separately for the sake of reviews.

Technical details

The gas masks and muzzle all get a new tag that's like, "[part of the item ID]Ingredient", and that is used in the crafting recipe because recipes are tag-based and not prototype-based. However, I did not want the final result (the muffled gas masks themselves) to have the ingredient tags, so I had to do some nasty-ass YAML refactoring to manipulate the tags in this way.

The fact tags do not allow for inheritance (i.e. inheriting the tag list from parents and being able to tack new entries on) kind of encourages repetitive YAML behavior, unfortunately, but on the other hand it CAN be taken advantage of like I did in this case to remove the ingredient tags in the final result.

As for why I did this weird multiple inheritance take instead of just making a new prototype inheriting from MaskBase: For consistency in case any changes to the regular gas masks are made (besides, uh, their tags)


TODO

  • Add muffled gas masks.
  • Add crafting recipe for muffled gas masks.
  • Look into where else they could be added besides crafting menu (e.g. loadouts).

Media

A passenger wearing the muffled gas mask and attempting to speak; it only comes out as "Mmfph!"

Crafting menu showing that you can craft a muffled gas mask, muffled explorer gas mask, or muffled security gas mask. The regular one just takes a gas mask and a muzzle.


Changelog

🆑

  • add: New "muffled" variants for the regular, explorer, and security gas masks, for people who love breathing almost as much as they love speaking unintelligibly. They can be obtained via the crafting tab, or the regular one can be found in loadouts.

The refactors here are a little clunky, sorry. I wanted these three specific gas masks to have a unique Ingredient tag for the sake of crafting.
This is why the GasMaskTags prototype exists: to "reset" the child gas masks to the original tag list. Not pretty but otherwise you would be able to use muffled gas masks in the crafting recipes and waste resources, which would trip up anyone who wanted to make multiple muffled gas masks at a time.
Just the regular one, because the other two gas masks aren't equippable in loadout themselves.
@github-actions github-actions bot added Status: Needs Review Someone please review this Changes: YML Changes any yml files labels Sep 21, 2024
@Floof-Station-Bot Floof-Station-Bot changed the title Muffled gas masks Muffled Gas Masks Sep 21, 2024
@Memeji
Copy link
Collaborator

Memeji commented Sep 22, 2024

I will review this PR in detail.

@ShatteredSwords
Copy link
Contributor

I will playtest this when I get home

Copy link
Contributor

@ShatteredSwords ShatteredSwords left a comment

Choose a reason for hiding this comment

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

Can confirm that it works after playtesting. There may be a few merge conflict errors that pop up from this.

You should always comment out lines instead of remove them if you're messing with things from upstream.

@sarcoph
Copy link
Author

sarcoph commented Sep 25, 2024

You should always comment out lines instead of remove them if you're messing with things from upstream.

I thought I did this but apparently not. Thanks for reminding me

I actually don't know if this is necessary because they were moved rather than removed.
@FoxxoTrystan FoxxoTrystan added the Priority: 2-Medium Needs to be resolved at some point label Sep 25, 2024
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the Status: Merge Conflict FIX YOUR PR AAAGH label Oct 21, 2024
@FoxxoTrystan
Copy link
Collaborator

Is this PR planned to be worked on?

@sarcoph
Copy link
Author

sarcoph commented Oct 29, 2024

Is this PR planned to be worked on?

Currently I am unable to work due to real life circumstances, but I should be back in action in about two weeks. I might try to find time this week to squeeze in a merge conflict resolution, however. I apologize for the inconvenience.

@Fansana Fansana marked this pull request as draft October 30, 2024 21:13
@FoxxoTrystan FoxxoTrystan removed the Status: Needs Review Someone please review this label Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: YML Changes any yml files Priority: 2-Medium Needs to be resolved at some point Status: Merge Conflict FIX YOUR PR AAAGH
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants