search

Aether-Tools / CustomizePlus

Dalamud plugin designed to give you better control over your Final Fantasy XIV character appearance.
Apache License 2.0
21 stars 10 forks source link

Local Space Transforms #38

Open d87 opened 1 month ago

d87 commented 1 month ago

Solves https://github.com/Aether-Tools/CustomizePlus/issues/36 Mostly taken from Ktisis. The existing GetDescendants method was broken it seems.

https://github.com/user-attachments/assets/0059aeb5-ff3d-4383-a3a0-0c8ea96a284f

RisaDev commented 2 weeks ago

Hello. Thank you for your contribution. Unfortunately this cannot be merged as is. Contributing section of repository readme was written specifically to avoid such issues.

One of the biggest problems with your code is that it is not backwards compatible. Merging this will break majority of user templates. The solution for it would be to include a checkbox for every bone in the bone editor UI which will control if entered values should be propagated or not. It should probably be on by default, but it should be confirmed that older templates load with this option being off.

I would also like to ask you to research possible options of extending this functionality to scale option of the bones if possible.

There is also an issue of Ktisis and Customize+ licenses not being compatible. It should not be a huge issue, but we will have to ask Chirp for permission to re-use Ktisis code if this is to be merged. This is something I can deal with myself, but this is important to mention.

I also have several issues with various things in the code but those can be wait until this PR is implemented in a way that doesn't break existing functionality.

d87 commented 2 weeks ago

Hi, I added checkboxes for all 3 types. And you can even scale the head now. Scale definitely shouldn't be propagated by default or mashed with translation & rotation.

RisaDev commented 2 weeks ago

Hey, that looks really good. I assume there is no way of making scale compatible with the other two options? People will definitely be asking about that feature so I need to know if this is just something that is too much hassle to implement properly, technical limitation or something else.

Also there seems to be some issues when using both position and rotation, at least for the head bone. I assume nothing can be done about that as well?

image Values to recreate that issue (head bone): Rotation Yaw: -23.500 Position Y: -0.136

Also there seems to be some really weird thing with head bone not being affected by parent bones? You have any idea why that might be the case?

d87 commented 2 weeks ago

No, scale IS compatible now. I just mean I split them into 3 settings because scale shouldn't be mixed with the rest into a single checkbox, as it is not propagated in Anamnesis/Ktisis/Blender, and it's just perfectly fine as it is mostly, it's easier to think about it when it's not.
Scale propagation is just for fun
image

Also there seems to be some issues when using both position and rotation, at least for the head bone. I assume nothing can be done about that as well?

Yep, some kind of face rigging magic is going on there I assume

Also there seems to be some really weird thing with head bone not being affected by parent bones? You have any idea why that might be the case?

I think it's something about these eyelid/lip bones not always getting correctly registered as children, no idea why For me this only happens on Vieras, and even then far from always apparently.
When I start the game with Viera character in the login screen, then it's always fine. And once it works, it works for the rest of the session.

On the screenshot above everything was fine no matter what I did, then I restarted the client and... image

RisaDev commented 2 weeks ago

No, scale IS compatible now. I just mean I split them into 3 settings because scale shouldn't be mixed with the rest into a single checkbox, as it is not propagated in Anamnesis/Ktisis/Blender, and it's just perfectly fine as it is mostly, it's easier to think about it when it's not.

Ah, I see. I thought you meant a different thing because I've noticed that it seems like scale doesn't work super well sometimes when used alongside with rotation and position, but I assume it's just what we'll have to accept as this seems like the same thing as with rotation and position when they are used simultaneously.

I don't really have time to review this properly at this moment, so I will leave this open for now. It is likely I will just merge it and do necessary changes/refactoring myself unless there there are serious issues.