Fix broken layer hiding on clothes with multiple equipment slots

[paige404]

About the PR

Adjusted ClothingSystem logic to confirm that a slot to be hidden by a HideLayerComponent matches the slot that an item is equipped in. Added more precise layer selection for the HideLayerClothingComponent, so that different layers may be hidden depending on which slot the item is equipped in. Adjusted the HumanAppearanceComponent to track which slot(s) are hiding each hidden layer (permanently hidden layers are unaffected by this change).

Before PR: ClothingSystem compared the sum combination of slots an item can be equipped in to the slot the HideLayerComponent's slot. This would always be false for clothing with more than one equip slot.

After PR: ClothingSystem specifically checks for a match between the item's current equipped slot and the HideLayerComponent's slot.

Why / Balance

Addresses issue #33965

Technical details

This should be slightly more performant in ToggleVisibleLayers--at the very least it should shift off some CPU cycles in exchange for a small amount of added memory. Specifically, nested loops over the character's inventory slots have been removed in favor of maintaining what had been small HashSets as small Dictionaries instead.

Care has been taken to ensure that no existing items break on this change. Items can use the new functionality added to HideLayerClothingComponent, but it isn't mandatory unless that item will be equippable in multiple clothing slots.

Media

Requirements

• I have read and am following the Pull Request and Changelog Guidelines.
• I have added media to this PR or it does not require an ingame showcase.

Breaking changes

Changelog

[paige404]

It required some bigger refactors than I'd intended, but this solves the problems and removes some excessive looping over the full inventory in ToggleVisibleLayers as a bonus.

I have noticed one strange behavior, though, which is somewhat inconsistent: occasionally, when equipping and then removing multiple layer-hiding pieces, the layer will stay hidden for an extra moment. Just long enough to be noticeable, but not long enough to be a serious issue.

For instance, equipping a bandana or a gas mask, then equipping a welding mask on top, then removing both. Sometimes, there will be a brief delay--less than a second--before the snout appears again. I think it has something to do with client and server mismatch, but I'm not sure where that's occurring. It's also possible that it's just a bottleneck issue on the part of my 10 year old machine running both client and server simultaneously.

Nevermind I fixed it.

[paige404]

Also, I don't mind going through and converting all of the items using HideLayerClothingComponent over to the new setup. Compatibility is there so it isn't an issue either way, but getting them onto the new one is a good way to encourage future items to also use the new one.

[ElectroJr]

Other than ClothingComponent.EquippedInSlot gteting cleared too early, it mostly seems fine. Though there are a few other changes I want to make to just clean up the system a bit, do you mind if I push some changes to your branch? Otherwise I can also just open a separate PR later

[paige404]

Other than ClothingComponent.EquippedInSlot gteting cleared too early, it mostly seems fine. Though there are a few other changes I want to make to just clean up the system a bit, do you mind if I push some changes to your branch? Otherwise I can also just open a separate PR later

I don't mind at all! Better to have it clean the first time and fewer PRs IMO

[ElectroJr]

I've pushed some changes, including:

• Splitting the layer hiding logic out of ClothingSystem and into a separate HideLayerClothingSystem.
• Allowing an item to hide the same single layer from more than one slot.
• Changing HumanoidAppearanceSystem.SetLayerVisibility to make the permanent option mutually exclusive with specifying a SlotFlag.
  • Specifically, I just removed the argument and made the slot flag argument nullable.
  • If its null it implies its not due to any clothing and thus "permanent".
  • Before it was possible to accidentally use permanent = false with flags = SlotFlags.None, which wouldn't have worked.
• Reworking parts of MaskSystem, mainly to fix some networking & prediction bugs
  • It wasn't actually marking the component as dirty when changing IsToggled, leading to prediction issues.
  • There's parts here that I still don't really understand, and there weren't any comments. I've just tried to keep the functionality mostly unchanged, though I have renamed some fields like MaskComponent.IsEnabled to IsToggleable, because I think that's what it was meant to do??

[paige404]

Forgive me the liberty of reviewing your code in your review of my code, but there is one bug to fix here. I don't mind fixing it though, if you want.

[ElectroJr]

Forgive me the liberty of reviewing your code in your review of my code, but there is one bug to fix here. I don't mind fixing it though, if you want.

No worries, thanks for reviewing my changes. Assuming its the one you pointed out I just fixed it, but obviously feel free to push any changes you want to make or revert any I made, it's still your PR

[paige404]

... I found another bug, but I have no idea what's causing it. Folding a bandana to wear on the head, the bandana doesn't render when worn.

If you spawn a folded bandana and put it on your head, it renders appropriately. If you unfold that bandana and wear it on your face, that renders too. If you then refold the bandana and put it on your head, the bandana no longer renders on the sprite.

I'll dig into this some more, but this is not ready to ship.

Edit: I am completely stumped.

[ElectroJr]

If you spawn a folded bandana and put it on your head, it renders appropriately. If you unfold that bandana and wear it on your face, that renders too. If you then refold the bandana and put it on your head, the bandana no longer renders on the sprite.

The cause was that toggling a mask sets a prefix for the clothing RSI state. Previously foldable clothing was being given special treatment in the mask system without any comments or explanation, and was seemingly not setting its rsi state specifically if the mask was being toggled due to folding, which I accidentally removed in an earlier commit.

I've fixed it now by making bandanas use the existing FoldableClothingComponent.FoldedEquippedPrefix instead of having some unintuitive hidden interaction with mask system. I've also fixed another bug in foldable clothing where it could just modify hidden layers while equipped. Every time I look at a new clothing system I find more spaghetti & weird interactions.

[paige404]

I can find no bugs. LGTM.

[paige404]

Oh hell. Merge conflicts. @ElectroJr it's in the code you changed. Could I ask you to fix it?

[github-actions]

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