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

Felinid changes. #666

Merged
merged 28 commits into from
Feb 12, 2024
Merged

Felinid changes. #666

merged 28 commits into from
Feb 12, 2024

Conversation

Adrian16199
Copy link
Contributor

@Adrian16199 Adrian16199 commented Jan 12, 2024

A plan that is barely functional

I just want to thank Finket, a dear friend of mine who helped me with this PR, if it wasnt for them, it probably would never work.

About the PR

Felinid's thieving gloves being gone and adding a new mechanic, soft paws.
Other tweak is making them get less speed penalty when damaged.

Why / Balance

Felinid's thieving gloves are only really one sided, there really isnt an excuse to steal from someone if you arent an antag.
Im basicly tryin to change that, whether or not someone helps.

Media

  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase
Delta-v.2024-01-16.14-57-04.mp4

Breaking changes

Changelog

🆑

  • tweak: Felinid's thieving gloves have been removed for Soft paws mechanic.

@github-actions github-actions bot added Changes: C# Changes any cs files Changes: YML Changes any yml files labels Jan 12, 2024
@UnicornOnLSD
Copy link
Contributor

image
you're the best Adri :)

- type: Thieving
stealthy: true
stripTimeReduction: 1
#- type: Thieving # FUCKIN KILL.
Copy link
Member

Choose a reason for hiding this comment

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

blessed pr

@AitorLogedo
Copy link
Contributor

I don't like this at all, stealing has been the thing from felinids since forever and changing it up to not making sounds when they have no shoes is totally removing the goblin concept of them for a change barely nobody will ever notice.

@Adrian16199
Copy link
Contributor Author

The point of removing thieving gloves is the fact that its only for antags.
Using this trait as called: Goblin-like as an non-antag, sounds like LRP and self antagging by being a nuisance.
If you are an antag, congrats, you saved up tc and your big plus balances every single minus.
Felinids in their current state are basicly only for antags because if you are not an antag, you are basicly just self antagging most of the time with THIS trait. People dont trust felinids and yeah, maybe a mechanic that nobody will notice would be there but I could just simply not add one in replace of it and just simply make felinids be worse overall and most people wont complain.
I want felinids to feel good whether or not you are an antag.

@Adrian16199
Copy link
Contributor Author

And besides, in the end this PR could be closed at any time if the change would be unwanted and nothing will change, felinids will still be hated for being a goblin even if they are existing, being a hardcore species that has one bonus that only applies when you are an antag.

@DangerRevolution
Copy link
Contributor

for a change barely nobody will ever notice.

https://discord.com/channels/968983104247185448/968983104662409244/1195765861756190890
relevant discussion; an hour of ppl voicing their dislike of felinid gloves + how it promotes LRP specism

@AitorLogedo
Copy link
Contributor

The point of removing thieving gloves is the fact that its only for antags. Using this trait as called: Goblin-like as an non-antag, sounds like LRP and self antagging by being a nuisance. If you are an antag, congrats, you saved up tc and your big plus balances every single minus. Felinids in their current state are basicly only for antags because if you are not an antag, you are basicly just self antagging most of the time with THIS trait. People dont trust felinids and yeah, maybe a mechanic that nobody will notice would be there but I could just simply not add one in replace of it and just simply make felinids be worse overall and most people wont complain. I want felinids to feel good whether or not you are an antag.

Felinids are the worst race overall and "current state" they had gloves since nyano, people don't play felinids for the stealing and removing the unique thing they have seems like a wizden change. I just don't see why you will feel bad for stealing, and definitely is not a thing that you can't do as an antag, a lot of situations can happen were the stealing thing is useful

@DebugOk
Copy link
Contributor

DebugOk commented Jan 13, 2024

Felinids are the worst race overall and "current state" they had gloves since nyano, people don't play felinids for the stealing and removing the unique thing they have seems like a wizden change. I just don't see why you will feel bad for stealing, and definitely is not a thing that you can't do as an antag, a lot of situations can happen were the stealing thing is useful

There are several species that are arguably worse then felinids in their current state. This is a change that gets my vouch and I'd love to see it

@DebugOk
Copy link
Contributor

DebugOk commented Jan 13, 2024

Also pull request number 666 real

@deltanedas
Copy link
Member

felenids having thieving promotes shittery i hate it

@Carolyn3114
Copy link
Contributor

I don't like this at all, stealing has been the thing from felinids since forever and changing it up to not making sounds when they have no shoes is totally removing the goblin concept of them for a change barely nobody will ever notice.

maybe they should have more interesting upsides other than "free antag item"

@ps3moira
Copy link
Contributor

666, truly the evil PR. :trollface:

for a friend to see and help
@github-actions github-actions bot added the Status: Merge Conflict Fix your PR! label Jan 14, 2024
Copy link
Contributor

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

Adrian16199 and others added 2 commits January 15, 2024 11:43
Cos I could be at yours on friday
Tonight's at the place we started
It's a long way back from sorry
But here I go ....
How does it feel, how does it feel
Just tell me something

I don't know what to tell you
I don't know what to say
I got into my head, i'm sorry i threw it away
But i'm so glad you came back, even through all the pain
Let me try and mend it, i can promise that i have changed
Oh, let me give you all of me
Can we just sort it out for real
@github-actions github-actions bot removed the Status: Merge Conflict Fix your PR! label Jan 15, 2024
@Adrian16199 Adrian16199 marked this pull request as ready for review January 17, 2024 14:36
@Adrian16199
Copy link
Contributor Author

Applied the funny changes.

@@ -443,6 +445,14 @@ private bool IsAroundCollider(SharedPhysicsSystem broadPhaseSystem, TransformCom
sound = moverModifier.FootstepSoundCollection;
return true;
}

// If soft paws and no shoes, no sound. Delta V
if (_entities.TryGetComponent(uid, out SoftPawsComponent? _) &

Choose a reason for hiding this comment

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

I would rename this component just because it could be reused for any number of other things

@Adrian16199
Copy link
Contributor Author

I mean, got a cool name that you could propose?

@Elijahrane
Copy link

I mean, got a cool name that you could propose?

NoShoesSilentFootsteps

@Adrian16199
Copy link
Contributor Author

Fair enough.
Dont got the cool factor but probably got the "straight up the point" factor.

@Elijahrane
Copy link

Stuff not player facing should be utilitarian and easily understood rather than cool

@Adrian16199
Copy link
Contributor Author

so for some reason, the code just dies and doesnt want to work with that name.
How funny.

@Adrian16199
Copy link
Contributor Author

Somehow got it to work, yey.

{
return false;
}
// Delta V NoShoesSilentFootsteps till here.
Copy link
Contributor

Choose a reason for hiding this comment

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

image
I'm actually going to request that this be made using a pre-existing component. You can simply make a footstep collection for Felinids that contains only a completely silent .ogg file, and set Felinids to use this collection for their footsteps. Since it gets overridden by shoes, this would make them walk silently while barefoot without needing to touch any upstream code at all.

Copy link
Member

Choose a reason for hiding this comment

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

could it not be changed to be optional too, a silent ogg file is 1. probably some performance landmine 2. hacky as fuck

Copy link
Contributor

@VMSolidus VMSolidus Feb 12, 2024

Choose a reason for hiding this comment

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

performance impact

Literally zero. It already plays a .ogg clientside whenever a felinid walks. It is a default component on all humanoids.

We don't need to touch upstream code at all if this can be done with a single line of yaml in a non-upstream file. Even if you consider it "Hacky", bear in mind everything we do is similarly, if not more "hacky". I consider a single-line change in a non c# file to be the most elegant solution of all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Literally zero. It already plays a .ogg clientside whenever a felinid walks. It is a default component on all humanoids.

It is still doing stuff every step it doesn't have to

Copy link
Contributor

Choose a reason for hiding this comment

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

I've also considered that it could be done with a felinidcomponent that remcomps footstepmodifiercomponent on init.

Another potential option is tagsystem.removetag footstep , but both options I havent investigated deep enough into what would happen if either were outright removed. I happen to know that every shoe in the game temporarily modifies the species footstepmodifiercomponent, so removing it might not be safe. Whereas giving them a silent footstep sound file has a net zero performance change, and is guaranteed to work for this purpose.

@Adrian16199
Copy link
Contributor Author

Im just gonna copy what i wrote in the morning 10 hours into a comment because the "pending" thing is just annoying and I dont want to bother with it.

The comment:

I'm actually going to request that this be made using a pre-existing component. You can simply make a footstep collection for Felinids that contains only a completely silent .ogg file, and set Felinids to use this collection for their footsteps. Since it gets overridden by shoes, this would make them walk silently while barefoot without needing to touch any upstream code at all.

Erm, I tried this and just No.
-THAT is pretty much:
This entity is going to make this sound effect no matter what and will not care what the floor sound makes nor if you have any sort of shoes on you or not, it will make that sound effect no matter what. Its the most noticeable on slimes that do bwoin bwoin bwoin and will not care for the maintenance's cat walk existance.
And that would make the feet itself and with shoes make that sound unless I would do some C# that it would check if the shoes are there or not but even then.
That means that it would make only one sound type, it would ignore the floor so you wouldnt suddenly hear alot clearer someone that is steppin on a catwalk but someone who is just stepping on normal floor and it would be a repetable ogg of one floor type.

-This being an example of how it would be for a felinid to have your thing:

The code:
image

The video of demonstrating it:

Delta-v.2024-02-12.12-27-22.mov

-My code is basicly what stops the floor itself to stop making noise when you dont have shoes with 5 lines of code without havin to rework an entire system.

-Even if, i were to make this somehow look how you suggested, at a file with a big collection of sound effects,

what is going to determine the sound effect that its goin to make?
Why should it care for what effect would it make without touchin C# that could be extremely touchin wizden code more than this one.
Even if it were to only make it sound WHILE you were barefoot only, that would still require to touch wizden code or to get rid of the footstepsounds tag just for it to not check for that original one that would be simply fuckin pain because that would mean that you would need to give every single other tag to the felinid yml that was in its previous parent yml and then keep addin more as soon as somesort of doorbumper 2.0 would be added.

@VMSolidus
Copy link
Contributor

#666 (comment)

Ah, I actually really do appreciate that you went and made this demonstration video, which proves that I was incorrect, and that you went and did more research into the subject to figure out that the other options weren't viable. I apologize for any perceived offense, and will accept that I was incorrect. It's a good learning experience on both of our parts. I don't have perfect knowledge of these systems either, and I don't think anyone does. In light of that, and that you have done everything correctly when it comes to fully documenting changes to upstream code, I will sign this PR with my approval.

@DebugOk DebugOk dismissed their stale review February 12, 2024 20:53

No longer relevant

@deltanedas
Copy link
Member

yeeeeeeeeeeeeeeees no more shitters stealing

@Adrian16199
Copy link
Contributor Author

Well, I wouldnt say that it was offensive, im sorry if i made it seem that way but today is not a day for me.

I could make the code even shorter since I doubt that anything else or any other species would be usin this which would mean that I could just simply make it look for felinid component instead of "NoshoesSilent" component which would just make it better technicly.
I do appreciate that you wanted to find an alternative but tag system is a no due to how it destroys previous tags which would require maintance of even puttin new tags in.
And the method that you showed, simple yet its a big "it will make this no matter what" sound.
I appreciate your understanding. I really want this rework to be in its best shape compared to makin it be "shit code"

Copy link
Contributor

@DebugOk DebugOk left a comment

Choose a reason for hiding this comment

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

Codeowner review required

@DebugOk DebugOk merged commit 49d3c45 into DeltaV-Station:master Feb 12, 2024
10 checks passed
DeltaV-Bot pushed a commit that referenced this pull request Feb 12, 2024
@Adrian16199 Adrian16199 deleted the Felinid_rework branch February 13, 2024 12:27
@WarMechanic WarMechanic mentioned this pull request Apr 17, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: C# Changes any cs files Changes: YML Changes any yml files Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants