Bug: Duplicated cape rendering code
Johni0702 opened this issue ยท 3 comments
What Feature Types Apply to This Bug?
Item
Other Type
No response
What Type of Bug Is This?
Compatibility
Forge Version
N/A
The Aether Version
1.19.4-1.0.0-beta.2-forge
Is This Bug a Conflict With Another Mod?
Essential 1.2.0.14
Client Log
No response
Crash Report (if applicable)
No response
Steps to Reproduce
- Install The Aether and Essential (also available via CF/Modrinth)
- Enter world and equip any of Aether's capes
- Open the Wardrobe from the Pause Menu
- In the Emotes>Movement category, click the Running emote twice to unlock and equip it
- Go back in-game
- Press and hold R to open emote wheel
- Click the running emote
What You Expect To Happen
Cape should stay relatively attached to body and move around with it.
What Actually Happened
Cape stays fixed in space (relate to entity position) completely independent of body movement.
(this is with Sleep emote; with Running it's not quite as obvious in a still image)
Additional Details
This underlying issue here is that Aether seems to copy the vanilla cape rendering code more or less verbatim and as such any third party modifications do not apply to it. This isn't really specific to Essential; I'd assume mods like Wavey Capes (if there's a forge equivalent) will also not work properly.
There's many more issues with simply coping large swaths of vanilla code. From it being difficult to tell what's different compared to vanilla, the code getting out of sync on updates, to the fact that copying non-trivial amounts of vanilla code is technically illegal.
So, unless there's something Aether does with its capes that I don't see (I'm not actually familiar with how they function ingame, I am just forwarding a report and am making educated guesses based on the code), I'd highly recommend simply replacing all that with a fairly simple mixin into AbstractClientPlayerEntity.getCloakTextureLocation
(as done by this mod which does something fairly similar).
Please Read and Confirm The Following
- I have confirmed this bug can be replicated without the use of Optifine.
- I have confirmed this bug is on the most recently supported version of Minecraft.
- I have confirmed the details provided in this report are concise as possible and does not contained vague information (ie. Versions are properly recorded, answers to questions are clear).
- I have confirmed this issue is unique and has not been reported already.
- If playing on a modpack, I have reported this issue to their issue tracker already.
Have you tested this issue with any of our other accessories? If other mods modify the player model's positions and rotations, I doubt capes are the sole issue here and thus resorting to that mixin is probably not gonna be enough to fix the issue, I would have to see if Essential offers API for their emotes system to account for this. I also don't think I understand what issues you're mentioning with it being hard to distinguish the vanilla code from our code when updating.
Also to comment on the legality thing, the Minecraft EULA states "By "Mods," we mean something original that you or someone else created that doesn't contain a substantial part of our copyrightable code or content." While this is up for interpretation, I would not consider one method in a class to be a substantial part of Minecraft's large copyrighted codebase, nor to it being used for a substantial part of our codebase either.
The code has been needing further documentation for instances of vanilla-copied code as tracked by #463, so I agree there that it is an issue but an issue I am trying to account for. With knowing that its capes specifically though that have issues with other mods while other accessories don't, I will look into the mixin solution then, since that sounds like it would likely be the best solution for compatibility in this specific instance.
Have you tested this issue with any of our other accessories? If other mods modify the player model's positions and rotations, I doubt capes are the sole issue here and thus resorting to that mixin is probably not gonna be enough to fix the issue, I would have to see if Essential offers API for their emotes system to account for this.
I have not but I did not expect them to cause any issues because they copy the positions and rotations from the vanilla model.
The thing with capes is that, despite using the regular model classes to draw them, vanilla doesn't actually use those model classes to position/rotate them. For some stupid reason it uses raw GL (these days PoseStack) instead (probably just convenience so they don't have to convert the order of rotations). And that's why Aether's cape renderer can't just clone the vanilla model position/rotation like it does for the other accessories and instead copies the code which computes them (but then ofc doesn't see any third-party modifications to that code, hence the issue).
Tried it now and indeed the gloves seem to work perfectly fine with emotes:
I would have to see if Essential offers API for their emotes system to account for this.
It does not. But as per above it should be clear why it doesn't need one (except maybe for capes; but aside from WaveyCapes, any mods I've come across so far were always better off just using the vanilla renderer instead of adding specific support for one specific mod).
I also don't think I understand what issues you're mentioning with it being hard to distinguish the vanilla code from our code when updating
These were more general reasons for why I think one should never copy any non-trivial amount of vanilla code. Not a concrete issue but just more reason to generally prefer mixin over copying vanilla code. I'll elaborate a bit but don't worry about it if I don't make sense:
The first of the two issues with copying vanilla code verbatim is that becomes quite difficult to neigh impossible for anyone but the author, and after some more time even for them, to tell which parts of the code were simply copied from vanilla vs which parts have intentionally been changed. Git may help but as the code undergoes further changes and/or Minecraft updates, it'll be as difficult to interpret the history as it is to simply compare to the vanilla code.
Case in point, I looked at the vanilla code and at Aether's renderer, and I can see that there clearly are some differences but I (with no experience of how exactly the capes are supposed to behave) have no clue which of these are intentional, semantically identical refactors or bugs.
The second of the two issues is that such copies can easily drift away from the original over time. The obvious example being that a Minecraft update changes some small detail; unless you look at diffs for Minecraft changes of all your copied source code, you're likely miss those and will then likely at a much later point be in for a debugging session once someone does notice.
But this isn't only the case long term between multiple Minecraft versions, the Essential-Aether-cape bug is an example of this too, the original code was changed by a third-party mod at class load time but the copied code obviously doesn't include those changes, leading once again to bugs, and those can sometimes be quite nasty to debug (this one luckily wasn't at all).
Also to comment on the legality thing [...]
Yes, certainly more of a theoretical issue that entirely depends on your jurisdiction. And I doubt Mojang has any interest in taking down a huge mod over 20 lines of code.
Though since you brought up the EULA, I do have to mention that it doesn't matter how the EULA defines "Mods", what matters is whether it (or local laws) explicitly allow you to use any of their code (and at least the EULA does not; local laws though likely will within certain limits).