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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds the Cat and Dog Person traits #36963

Closed
wants to merge 1 commit into from
Closed

Adds the Cat and Dog Person traits #36963

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 4, 2018

馃啈 Xhuis
add: You can now be a cat person or a dog person in Space Station 13!
/:cl:

Thaw PR: #36663

You know how you ask someone their favorite kind of pets, and they might say something like "oh, I'm a cat person?" Now you can be one. These two new neutral traits give you preference to dogs or cats, meaning you gain a slightly higher mood increase from petting your preferred one and slightly lower mood increase from petting the inferior other one. So, if you're a dog person, you get +5 to mood from petting a corgi and +1 from petting a cat, instead of +3 from either like normal.

Having both traits will cancel each other out. Also changed the pet_corgi association for moodlets gained by petting animals to wuv since there's now a mood increase from petting cats, too.

@tgstation-server tgstation-server added the Feature Exposes new bugs in interesting ways label Apr 4, 2018
@ghost ghost changed the title Adds the Cat Person and Dog Person traits Adds the Cat Person and Dog Person traits (temporary thaw PR link inside) Apr 4, 2018
@JJRcop
Copy link
Contributor

JJRcop commented Apr 4, 2018

This should have some interaction with cat mutant parts.

@imtakingabreakdontatme
Copy link
Contributor

You need to fix moodlet traits being available when moodlets are not before you add more of them

@Anonmare
Copy link
Contributor

Anonmare commented Apr 4, 2018

Lucky for you I'm a dog lover

@ghost
Copy link
Author

ghost commented Apr 4, 2018

@KorPhaeron I've been intending to make an "is_available" system for traits, but I'm not sure how to implement them since in its current state the objects aren't created, just read from their initial vars. Would it be cleaner to use those initial vars like incompatible_with and moodlet_trait, or to have a separate datum for the purchase entry that has an is_available() proc that can be overriden?

@nicbn
Copy link
Contributor

nicbn commented Apr 4, 2018

In other words free mood by petting mobs

@ghost
Copy link
Author

ghost commented Apr 4, 2018

Mood boosts from petting corgis has been around since moodlets were first added.

@ma44
Copy link
Contributor

ma44 commented Apr 4, 2018

What if you were a dog lover but a cat person.

-50 mood from identity crisis

@ghost
Copy link
Author

ghost commented Apr 4, 2018

Having both traits cancels each other out and makes no change to either mood bonus from its normal value.

@ExcessiveUseOfCobblestone
Copy link
Contributor

@Xhuis replace mood with braindamage when mood is disabled, problem solved :^)

@ghost ghost changed the title Adds the Cat Person and Dog Person traits (temporary thaw PR link inside) Adds the Cat Person and Dog Person traits, disables mood traits if their config is disabled, and further tweaks Social Anxiety Apr 5, 2018
@ghost
Copy link
Author

ghost commented Apr 5, 2018

I made mood traits be unavailable for purchase if moodlets are disabled, plus a failsafe that removes them from your setup if they're disabled but you added them before they were turned off.

Also some more tweaks to Social Anxiety, the eternal conundrum. I'd like to atomize this PR a bit, but since the freeze is currently going on and I can only make one thaw PR, I've bundled them up here.

@ghost ghost changed the title Adds the Cat Person and Dog Person traits, disables mood traits if their config is disabled, and further tweaks Social Anxiety Disables mood traits moodlets are disabled, tweaks Social Anxiety, adds the Cat and Dog Person traits Apr 5, 2018
@ExcessiveUseOfCobblestone
Copy link
Contributor

why would you tie the fix to a feature pr you hecker

@ghost
Copy link
Author

ghost commented Apr 5, 2018

I can only make one thaw PR during the freeze and the bundle of features here is what I'd wanted to add once it ended. Unless any maintainers have a notable problem with it and want me to slice off the Social Anxiety portion, the other two are fine in the same PR, I think.

@ExcessiveUseOfCobblestone
Copy link
Contributor

ExcessiveUseOfCobblestone commented Apr 5, 2018

sorry my last post was meant to be lighthearted banter but text is hard.

@ghost
Copy link
Author

ghost commented Apr 5, 2018

I'll stab my light knife through your heart

@ExcessiveUseOfCobblestone
Copy link
Contributor

watch it buddy i'll take your heirloom.

@ghost
Copy link

ghost commented Apr 6, 2018

image
i read this and thought moodlets were disabled for a second because there isn't an if between "traits" and "moodlets"

@ghost ghost changed the title Disables mood traits moodlets are disabled, tweaks Social Anxiety, adds the Cat and Dog Person traits Disables mood traits if moodlets are disabled, tweaks Social Anxiety, adds the Cat and Dog Person traits Apr 6, 2018
@ghost
Copy link
Author

ghost commented Apr 6, 2018

Oops.

Copy link
Member

@ninjanomnom ninjanomnom left a comment

Choose a reason for hiding this comment

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

Please atomize this pr

@ghost
Copy link
Author

ghost commented Apr 8, 2018

As I already mentioned, the feature freeze prevents me from atomizing all of it. If I wanted to add the Cat and Dog Person traits in another PR, I'd have to add the framework separately in a different PR since the thaw only covers one PR.

I can remove the social anxiety tweaks if that's your main concern, though. Otherwise, I'm not sure the best way to handle it.

@ninjanomnom
Copy link
Member

I primarily meant splitting up the code disabling traits, the anxiety changes, and the [animal] person traits. You can include the framework for whatever changes you need in those.

@ghost
Copy link
Author

ghost commented Apr 9, 2018

It seems like the best idea would be to set this PR to the original scope (adding the two traits), and then once that's in, I can add the restriction as it wouldn't conflict with the freeze. The social anxiety changes can wait.

@ghost ghost changed the title Disables mood traits if moodlets are disabled, tweaks Social Anxiety, adds the Cat and Dog Person traits Adds the Cat and Dog Person traits Apr 9, 2018
@ghost
Copy link
Author

ghost commented Apr 9, 2018

There we go, quick and clean.

@ExcessiveUseOfCobblestone
Copy link
Contributor

ExcessiveUseOfCobblestone commented Apr 9, 2018

You've already secured the spot in the thaw, make the fix then fix conflicts in here post.

@ninjanomnom
Copy link
Member

Kor requested the traits get their fix before more features are merged so this'll have to wait until that gets merged.

Or at least approved

@ghost
Copy link
Author

ghost commented Apr 9, 2018

I'll make a second PR that includes the framework/fix standalone, and once that gets in I can update this one.

if(change)
if(change > 0)
if(M && stat != DEAD)
new /obj/effect/temp_visual/heart(loc)
if(M.has_trait(TRAIT_CAT_PERSON))
Copy link
Member

Choose a reason for hiding this comment

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

uhm this is all backwards.

Send only a signal that a cat has been petted.

then let the thing that processes the signal figure out what trait the person has.

You're leaking the systems abstractions

Copy link
Author

Choose a reason for hiding this comment

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

Unless I misunderstand moodlet code, wouldn't that require me to check every moodlet event to see if it's a certain type, and if so, if the traits were present?

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently, yes. You will have to edit moodlet code to allow for some moodlets to receive nonstandard signals.

Copy link
Member

Choose a reason for hiding this comment

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

@Qustinnus didn't I make you do this for something else, how did you implement it?

Copy link

Choose a reason for hiding this comment

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

I think my case was unique because everyone gets the same moodlet (table) except I modified it after if you are a catperson. Ill look into a different way of approaching this

@ghost
Copy link
Author

ghost commented Apr 19, 2018

I don't have any more desire to work on this or wait for more days hoping Qustinnus will respond so I'm gonna close this

@ghost ghost closed this Apr 19, 2018
@ghost
Copy link

ghost commented Apr 19, 2018

Oh im sorry I havnt been on github for a while due to personal issues and uni keeping me busy. Ill look into it now

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Exposes new bugs in interesting ways
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants