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 alt verb to toggle accents from clothing #2648

Merged

Conversation

Alkheemist
Copy link
Contributor

@Alkheemist Alkheemist commented Jan 2, 2025

About the PR

Added an alt verb to AddAccentClothingSystem that will allow you to toggle the active accent given by a piece of clothing. To do so, you must be wearing the clothing. This isn't stored, so unequipping and re-equipping will reapply the accent, but you can just toggle it off again afterwards.

Why / Balance

By popular request.

How to test

Put on accent clothing (cat ears are the most cringe based)
Say something
Alt click or right click and toggle the accent
Say something else

Media

dotnet_03_10-02-29
dotnet_03_10-03-03

Requirements

Breaking changes

None identified.

Changelog

🆑

  • tweak: Clothing and items that apply accents can be toggled with alt click, so you can dress in style without worrying about intelligibility.

@Alkheemist
Copy link
Contributor Author

Good catch, applied changes

@dvir001 dvir001 self-requested a review January 5, 2025 17:58
@github-actions github-actions bot added the S: Needs Review This PR is awaiting reviews label Jan 5, 2025
@dvir001
Copy link
Contributor

dvir001 commented Jan 8, 2025

@Alkheemist Can you also add this to AddAccentPickupComponent?

Right now its just the caveman club

@Alkheemist
Copy link
Contributor Author

actually digging into it... pickup code and the club is fundamentally broken
image

@Alkheemist
Copy link
Contributor Author

OK, I've fixed it by adding in a new event (old one used for accentpickup was firing when the check for eligibility to pick up was made, not when the item was actually being picked up) and implementing the toggle for the club.

@Alkheemist
Copy link
Contributor Author

anyone have any idea what's up with the 1 failing test?

@GreyMario
Copy link
Contributor

anyone have any idea what's up with the 1 failing test?

public DataNode Write(ISerializationManager serializationManager, EntityUid value,
this is failing because your entity field isn't nullable

@Alkheemist
Copy link
Contributor Author

I fixed the thing that was causing the failing test, ready for another review @dvir001

@dvir001 dvir001 requested a review from whatston3 January 15, 2025 12:34
@whatston3
Copy link
Contributor

Not a fan of this, in all honesty.

@dvir001
Copy link
Contributor

dvir001 commented Jan 17, 2025

Not a fan of this, in all honesty.

To be honest other forks just removed the accents fully from cloth then leaving the toggle (DeltaV, EE)
So this is more fine for general case.

Copy link
Contributor

@whatston3 whatston3 left a comment

Choose a reason for hiding this comment

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

A few suggestions.

Content.Shared/Item/PickupEvent.cs Outdated Show resolved Hide resolved
Content.Shared/Item/PickupEvent.cs Outdated Show resolved Hide resolved
@Alkheemist Alkheemist requested a review from whatston3 January 18, 2025 23:17
Copy link
Contributor

@whatston3 whatston3 left a comment

Choose a reason for hiding this comment

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

Works fine, did a little cleanup:

"GettingPickedUpEvent" changed to "PickedUpEvent" - the item is already picked up. I think the hand can be found through the ContainerSystem.

Made the NF changes use the EntityManager proxy without the EntityManager reference for brevity, and removed "HasComp->RemComp", that check's already done by the function. The Add path I kept, since it's an early return.

Again, works without issue, good stuff.

@whatston3 whatston3 dismissed dvir001’s stale review January 19, 2025 15:51

Changes applied previously.

@Cheackraze Cheackraze merged commit 6a9d357 into new-frontiers-14:master Jan 19, 2025
12 checks passed
FrontierATC added a commit that referenced this pull request Jan 19, 2025
@Alkheemist Alkheemist deleted the togglable-accent-clothing branch January 20, 2025 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# FTL S: Needs Review This PR is awaiting reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants