cmooref17 / ReservedItemSlotMods

https://thunderstore.io/c/lethal-company/p/FlipMods/ReservedItemSlotCore/
MIT License
1 stars 1 forks source link

Several holster points are broken #53

Closed JillCrungus closed 1 month ago

JillCrungus commented 1 month ago

There are several PlayerBone enum values which return null when passed into BoneMap.GetBone by default. From what I've been able to determine, the following values do not return a valid bone:

Head
LeftArmUpper
LeftArmLower
RightArmUpper
RightArmLower
LeftCalf
RightCalf

I first noticed this when trying to troubleshoot what I originally thought was some kind of networking issue. As it turns out, it was a result of my ReservedItemData registering with PlayerBone.Head as the holster point. In OnPocketReservedItem playerData.boneMap.GetBone was failing for PlayerBone.Head. Further investigation led to me find that the above values all return null when you attempt to GetBone them.

As a result, any items set to attach to these points will start acting very strangely when ShowReservedItemsHolstered is enabled due to SetHolsteredPositionRotation early-exiting whenever the item is holstered. The item's mesh will be made visible, but with no parent and nothing to update its position it will simply stay in place wherever it was last.

JillCrungus commented 1 month ago

I've looked into it, and the cause here is twofold.

LeftArmUpper
LeftArmLower
RightArmUpper
RightArmLower
LeftCalf
RightCalf

For these, their bone names in the default bone name array are simply wrong.

However, for Head, the problem is a logical oversight in MapBoneRecursive. Head shares a bone with Neck (spine.004). However, since MapBoneRecursive uses IndexOf to determine the index to map a bone to, it could only ever find the index for Neck. It would reach spine.004 in the player hierarchy, use IndexOf and find one index - the index corresponding to Neck - assign it, and then proceed to the next bone in the hierarchy, leaving Head forever unassigned.

The solution here would be either to just update Head to use a different node (maybe spine.004_end?) or update the function to correctly handle cases where multiple bone mappings share the same bone.

I've created a pull request (#54) to fix this issue. In the interest of preserving the original intent of the current bone name array, I've opted for the latter option when it comes to fixing Head.

cmooref17 commented 1 month ago

Wow, I'm honestly wondering what bone names I might have been referencing when I got those names wrong lol. It was likely because I was working with another skeleton for my other mod, TooManyEmotes, and probably mixed up some of the bone names. Thanks for pointing this out!

As for the head, I now see the oversight I had, but I did quickly test this, and it's now showing all bones as not null. Thanks so much for letting me know about these issues, and thanks for the changes. I'll get this merged soon and get the update pushed out!

Feel free to close this once you confirm that this works on 2.0.32 when I push the update out. On my side, it does look like it's resolved now.

JillCrungus commented 1 month ago

Seems to work fine after a quick check.