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

Add flowers to loadout #32097

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

Conversation

JustArt1m
Copy link
Contributor

@JustArt1m JustArt1m commented Sep 12, 2024

About the PR

Added poppy and lily head flowers.
Added flower wreath, poppy flower and lily flower to loadout.

Why / Balance

Just for more customization of loadout.

Media

Screenshot 2024-09-20 171457
Screenshot 2024-09-15 232353

Requirements

Changelog
🆑 Just_Art

  • add: Added poppy and lily head flowers!
  • add: Added flower wreath, poppy flower and lily flower to loadout!

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

See: #30079 and #29302 for more discussion.

@JustArt1m
Copy link
Contributor Author

See: #30079 and #29302 for more discussion.

So I will still need to make second versions of these flowers in order to exclude the possibility of using them outside the cosmetic context?

@beck-thompson
Copy link
Contributor

So I will still need to make second versions of these flowers in order to exclude the possibility of using them outside the cosmetic context?

Unknown. The maintainers switched back and fourth between which version they wanted 🤷
I love the idea so I just hope it gets added in some version!

@slarticodefast
Copy link
Member

slarticodefast commented Sep 12, 2024

Unknown. The maintainers switched back and fourth between which version they wanted 🤷

That maintainer discussion was way too long last time. Medical flowers might cause metagaming issues as they have a significant amount how healing reagents inside them and loadouts should be mostly for visual customization. The suggestion we finally came up with was that another "hairflower" item with a new item sprite should be made (in contrast to a "fake poppy"), similar to the one that had been removed when flowers were changed to be wearable. The worn sprite could be the same as the poppy's. The flower wreath is probably fine.

@slarticodefast slarticodefast added the Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. label Sep 12, 2024
@SlamBamActionman
Copy link
Member

What Slart said. In addition, since Lillies are a mutation botanists have to work towards, I personally don't think they should be a roundstart cosmetic item. Makes botany still have a reason to make flowers for cosmetic purposes.

@Boaz1111
Copy link
Contributor

What Slart said. In addition, since Lillies are a mutation botanists have to work towards, I personally don't think they should be a roundstart cosmetic item. Makes botany still have a reason to make flowers for cosmetic purposes.

Agree with Slam on this one, I love making hairflower lilies for the crew, and I think people being able to just spawn with them would be weird.

Sprites, meta, prototype
@github-actions github-actions bot added the Changes: Sprites Should be reviewed or fixed by people who are knowledgeable with spriting or visual design. label Sep 12, 2024
Copy link
Contributor

github-actions bot commented Sep 12, 2024

RSI Diff Bot; head commit 30b4c29 merging into 2a58fa1
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/Clothing/Head/Misc/hairflower.rsi

State Old New Status
equipped-HELMET Added
icon Added

Edit: diff updated after 30b4c29

@lzk228
Copy link
Contributor

lzk228 commented Sep 12, 2024

and we are back to hairflowers... that were removed in #25475

@slarticodefast
Copy link
Member

slarticodefast commented Sep 12, 2024

and we are back to hairflowers... that were removed in #25475

We did not have loadouts back then and people want their drip

@slarticodefast slarticodefast added the Undergoing Maintainer Discussion This PR is currently going through the 72-hour discussion window as per maintainer policy label Sep 12, 2024
@bruhmogus
Copy link

why are these entire flowers as opposed to just using the normal preexisting poppy and lily?

@JustArt1m
Copy link
Contributor Author

why are these entire flowers as opposed to just using the normal preexisting poppy and lily?

#32097 (comment)

@slarticodefast
Copy link
Member

slarticodefast commented Sep 15, 2024

Ok, you might have misunderstood my comment, sorry if I was unclear. The loadout hairflower should not be a poppy at all, since this causes confusion with the botany plant. The new sprite makes this even worse: The hair flower is a whole plant while the botany produce is only a flower (you would not put a whole plant stem into your hair). Also I agree with Slam that the lily should be only available through botany.

So to reiterate, could you:

  • remove the lily
  • Resprite the item icon for the hairflower, so that it does not resemble the poppy, but some other, unspecific red flower
  • rename it to "hairflower"

The worn sprite for the hairflower and the flower wreath can stay the way they are.

@JustArt1m
Copy link
Contributor Author

@slarticodefast

Copy link
Member

@UbaserB UbaserB left a comment

Choose a reason for hiding this comment

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

can you make the middle of the flower sprite a bunch darker please

@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. and removed Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. labels Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: Sprites Should be reviewed or fixed by people who are knowledgeable with spriting or visual design. No C# For things that don't need code. Status: Needs Review This PR requires new reviews before it can be merged. 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.

8 participants