First-person Model (Forge/Fabric)

First-person Model (Forge/Fabric)

4M Downloads

Head turns invisible when Essential Mod is enabled

SGWasTooShort opened this issue ยท 13 comments

commented

I noticed a bug where my head would turn invisible, and it seems to only happen when this mod and Essential are both enabled. Disabling Essential makes the head render normally.

I have no idea why this happens, as i don't even use Essential cosmetics or anything of the sort.

2023-02-04_13 54 21_edited

I apologize for any lack of information or if this has already been reported before

commented

Yea this is an essentials issue under some circumstances as far as I know. Since it's closed source, and the bug is probably on their end, not much I can do about it.

commented

Hi, one of the Essential devs here. We have looked at this internally and came to the conclusion that this is best fixed on FPM's end, so I'm here to explain what, how and why we do for this (and probably actually also #315 if I'm not mistaken) to happen and what can be done about it.

I happen to be the person that implemented the relevant feature in Essential, and I usually give much thought to third-party mod compatibility in anything I implement, so if any of my decisions in how exactly I have implemented something are unclear, feel free to ask for clarification, there's a high chance I've considered alternatives and can give an explanation for why I chose the particular approach.

The code relevant for these two bugs is a mixin in Essential that aims to fix a vanilla deficiency: Not all variables in ModelPart are reset before each frame, only those which vanilla changes itself will also be reset by vanilla. This is of importance to Essential because it implements emotes which may very well change some of those variables and so it also needs to make sure they go back to their original values when the emote ends / another player is rendered.

The way we solve this problem is with a relatively simple mixin into HumanoidModel at HEAD of setupAnim that stores the current values for all the variables on first invoke, and then restores those default values on every subsequent invoke. Looks approximately like this (PlayerPose is a POJO storing all the variables for all the model parts and PlayerPoseKt copies to/from that pojo from/onto the MC types):

    @Unique
    private PlayerPose resetPose;

    @Inject(method = "setupAnim", at = @At("HEAD"))
    private void resetPose(CallbackInfo ci) {
        HumanoidModel model = (HumanoidModel) (Object) this;

        if (resetPose == null) {
            resetPose = PlayerPoseKt.toPose(model);
        } else {
            PlayerPoseKt.applyTo(resetPose, model);
        }
    }

The setupAnim method is of special significance to this issue because that's where vanilla sets up all the variables it uses. As such, any mod that wishes to modify one of the variables which vanilla uses must necessarily do it at some point towards the end of that method or after that method was called but never at the start of the method nor before the method is called.

And this leads me to the main assumption I'm making (and if you wish I can further elaborate on why I think this must be the case for independent mods to be compatible) which FPM violates: The same should be true for variables that vanilla doesn't touch. Mods should not make any assumption about the value of these variables until somewhere in the middle of setupAnim and any mod may overwrite their values back to the defaults before that point (in fact, every mod must do so, otherwise if they're the only mod it's never getting reset; FPM does that right at the end of the render method; Essential does this right at the start of setupAnim, basically where vanilla does it as well).

So fixing both issues should be as simple as moving FPM's modification of the z value to the same place where vanilla modifies the other variables: In HumanoidModel.setupAnim, ideally right below the lines where it sets this.head.y.
Thereby Essential won't overwrite it because it already reset the value at the start of the method (hopefully fixing #315) and Essential's reset-values won't be incorrect (fixing this issue).

The latter could also be worked around on Essential's end by reading the reset-values earlier (but who's to say when would be the most appropriate time to do that? we might then do it too early for some other mod; I'd argue the place where it currently does so is already the most reasonable place to do so) or by hard-coding the values but that has the same issue of wiping legitimate modifications of another third-party mod and more importantly, it cannot fix the first issue.
We also can't really move our resetting code to run before FPM's setting code because that's somewhat of a moving target: If another third-party mod comes along and modifies the variables at yet another non-compliant location, we might have to move again, or worse, there might not be any location where we could reset at the right time for both FPM and the third-party mod (such considerations are what lead me to the assumption/assertion I stated above).
Hence why I think this should be fixed on FPM's end (by not violating above assumption/assertion).

commented

Thanks for the insight. So basically it boils down to 2 mods caching the value but at different times, causing one mod to cache some temporary value as truth. I don't "really" change the z value, and instead, keep the real value around, but that doesn't help when Essential picks up the wrong one. Please give it a try with the version I just pushed, where I moved it to setupAnim for "HumanoidModels". For anything else, it will still set the hidden flag during render(no shared setupAnim location).

commented

Hi, another Essential dev here. This fix works, fixing this issue as well as #315. Thanks.

commented

OK nice. Then I'll role this update out later together with some code cleanup and 1.19.4 changes.

commented

1.18.2 too plz?

commented

Ehhh, sometime later when 1.18 gets updated. But 1.18 is not a vital version anymore so it will get updates less often.

commented

Noticed this happening on 1.19.2 or maybe it's not the Essentials mod? could it be Cosmetica?

commented

The fix is currently only in the 1.19.3 and 1.19.4 version.

commented

Is there a possibility that you can make an option to straight up turn off your player head when in first person?
I currently have to use / stick with the unpatched version of both essential and first person to get my desired result.

commented

Is there a possibility that you can make an option to straight up turn off your player head when in first person?

Thaaats the entire point of this mod? If the head is visible while in first person, then you have some incompatible mod installed. And this issue is the reverse, that the head was staying invisible when it shouldn't(fixed in 1.19.3/1.19.4).

commented

My apologies. It would be ' Kelvin's Better Player Animations '. Had that from the start using the First Person mod and figured the head visible was normal. Great work and sorry to bother!

commented

Backported the fix now to 1.19.2 and 1.18.2. Closing the issue.