Improvements #24

Oryxel · 2024-09-04T14:55:57Z

Just a new pr, #23

This is the improvements I made for BedrockSkinUtility, that is already got mention here #22

Refactor a lot of code since and replace many with my own version of the implement, since some model/skin/cape translate fine but won't show up and also models that have really unique model that doesn't look like the player won't works. So I just deleted a lot part of the code and reimplement it, works perfect fine after that.

Parsing model is switched to a library I write that allowed support for per-face uv map, and more.
Most of the changes can be listed in #22.

This haven't been tested on GeyserSkinManager but I test it using ViaBedrock instead.

Here is some images after the changes I made:

Camotoy · 2024-09-06T16:44:42Z

src/main/java/net/camotoy/bedrockskinutility/client/MixinConfigPlugin.java

@@ -23,7 +23,8 @@ public String getRefMapperConfig() {

    @Override
    public boolean shouldApplyMixin(String targetClassName, String mixinClassName) {
-        if (mixinClassName.equals("net.camotoy.bedrockskinutility.client.mixin.CapeFeatureRendererMixin")) {
+        if (mixinClassName.equals("net.camotoy.bedrockskinutility.client.mixin.CapeFeatureRendererMixin") ||
+                mixinClassName.equalsIgnoreCase("net.camotoy.bedrockskinutility.client.mixin.AbstractClientPlayer")) {


This will never be called as the class name is AbstractClientPlayerMixin

Camotoy · 2024-09-06T16:45:21Z

src/main/java/net/camotoy/bedrockskinutility/client/data/CustomCapeData.java

+
+import net.minecraft.resources.ResourceLocation;
+
+public record CustomCapeData(ResourceLocation cape) {


For one parameter, having a whole class seems unnecessary.

This class can be deleted.

This class can be deleted.

done

Camotoy · 2024-09-06T16:46:46Z

src/main/java/net/camotoy/bedrockskinutility/client/mixin/CapeFeatureRendererMixin.java

+    @Redirect(method = "render(Lcom/mojang/blaze3d/vertex/PoseStack;Lnet/minecraft/client/renderer/MultiBufferSource;ILnet/minecraft/client/player/AbstractClientPlayer;FFFFFF)V",
+                at = @At(value = "INVOKE", target = "Lnet/minecraft/client/player/AbstractClientPlayer;isModelPartShown(Lnet/minecraft/world/entity/player/PlayerModelPart;)Z"))
+    public boolean isModelPartShown(AbstractClientPlayer instance, PlayerModelPart playerModelPart) {
+        return SkinManager.getInstance().getCapeData().get(instance.getUUID()) != null;


Can just be a containsKey call.

Camotoy · 2024-09-06T16:47:26Z

src/main/java/net/camotoy/bedrockskinutility/client/mixin/EntityMixin.java

+import java.util.UUID;
+
+@Mixin(Entity.class)
+public abstract class EntityMixin {


This seems to be an unused class.

Oryxel · 2024-09-06T17:39:06Z

These are the changes that you requested, EntityMixin is there so I can get UUID from AbstractClientPlayerMixin.

Camotoy · 2024-09-07T04:57:25Z

Instead, can you extend AbstractClientPlayerMixin from Entity or another class? That should also do the job and keep things to one file.

Oryxel · 2024-09-07T09:08:40Z

Instead, can you extend AbstractClientPlayerMixin from Entity or another class? That should also do the job and keep things to one file.

That should do it, is there anything else?

Camotoy · 2024-09-07T17:29:47Z

src/main/java/net/camotoy/bedrockskinutility/client/GeometryUtil.java

-
-        // Construct a list of all bones we need to translate
-        List<JsonObject> bones = new ArrayList<>();
+        if (info.getGeometryRaw() == null || info.getGeometryRaw().isEmpty())


Camotoy · 2024-09-07T17:29:59Z

src/main/java/net/camotoy/bedrockskinutility/client/GeometryUtil.java

-                        }
-                    }
-                }
+        if (geometry == null)


Camotoy · 2024-09-07T17:30:06Z

src/main/java/net/camotoy/bedrockskinutility/client/GeometryUtil.java

+            final List<ModelPart.Cube> cuboids = new ArrayList<>();
+            float pivotX = (float) bone.pivot()[0], pivotY = (float) bone.pivot()[1], pivotZ = (float) bone.pivot()[2];
+            Bone parentBone = null;
+            if (!bone.parent().isEmpty())


Camotoy · 2024-09-07T17:30:21Z

src/main/java/net/camotoy/bedrockskinutility/client/GeometryUtil.java

+            if (parentBone != null) {
+                // This appears to be a difference between Bedrock and Java - pivots are carried over for us
+                part.setPos((float) (pivotX - parentBone.pivot()[0]), (float) (pivotY - parentBone.pivot()[1]), (float) (pivotZ - parentBone.pivot()[2]));
+            } else part.setPos(pivotX, pivotY, pivotZ);


Camotoy · 2024-09-07T17:30:29Z

src/main/java/net/camotoy/bedrockskinutility/client/GeometryUtil.java

+        for (Map.Entry<String, PartInfo> entry : stringToPart.entrySet()) {
+            if (entry.getValue().parent != null) {
+                PartInfo parentPart = stringToPart.get(entry.getValue().parent);
+                if (parentPart != null)


Camotoy · 2024-09-07T17:30:35Z

src/main/java/net/camotoy/bedrockskinutility/client/GeometryUtil.java

+                if (parentPart != null)
+                    parentPart.children.put(entry.getKey(), entry.getValue().part);
+                else {
+                    if (root != null && entry.getValue().part != root.part) // put to root if you can't find the parent.


Camotoy · 2024-09-07T17:30:41Z

src/main/java/net/camotoy/bedrockskinutility/client/GeometryUtil.java

-            return new BedrockPlayerEntityModel<>(root.part);
-        } catch (Exception e) {
-            LOGGER.error("Error while parsing geometry into model!", e);
+        if (root == null)


Camotoy · 2024-09-07T17:30:55Z

src/main/java/net/camotoy/bedrockskinutility/client/GeometryUtil.java

-            case "leftLeg" -> "left_leg";
-            case "rightLeg" -> "right_leg";
+        // it can be lowercase, use lowercase...
+        return switch (name.toLowerCase()) {


toLowerCase(Locale.ROOT)

Camotoy · 2024-09-07T17:31:16Z

src/main/java/net/camotoy/bedrockskinutility/client/message/BedrockMessageHandler.java

+    }
+
+    public void handle(BaseSkinInfo payload) {
+        if (payload == null)


Camotoy · 2024-09-07T17:31:20Z

src/main/java/net/camotoy/bedrockskinutility/client/message/BedrockMessageHandler.java

+    }
+
+    public void handle(SkinData payload, ClientPlayNetworking.Context context) {
+        if (payload == null)


Camotoy · 2024-09-07T17:31:29Z

src/main/java/net/camotoy/bedrockskinutility/client/message/BedrockMessageHandler.java

+
+        // Convert Bedrock JSON geometry into a class format that Java understands
+        BedrockPlayerEntityModel<AbstractClientPlayer> model = GeometryUtil.bedrockGeoToJava(info);
+        if (model == null)


Camotoy · 2024-09-07T17:35:18Z

src/main/java/net/camotoy/bedrockskinutility/client/GeometryUtil.java


-                stringToPart.put(name, new PartInfo(adjustFormatting(parent), part, children));
+            // Please lowercase this, it can be lowercase...
+            switch (name.toLowerCase()) { // Also do this with the overlays? Those are final, though.


toLowerCase(Locale.ROOT)

Camotoy · 2024-09-07T17:37:27Z

src/main/java/net/camotoy/bedrockskinutility/client/mixin/CapeFeatureRendererMixin.java

+
+    @Redirect(method = "render(Lcom/mojang/blaze3d/vertex/PoseStack;Lnet/minecraft/client/renderer/MultiBufferSource;ILnet/minecraft/client/player/AbstractClientPlayer;FFFFFF)V",
+                at = @At(value = "INVOKE", target = "Lnet/minecraft/client/player/AbstractClientPlayer;isModelPartShown(Lnet/minecraft/world/entity/player/PlayerModelPart;)Z"))
+    public boolean isModelPartShown(AbstractClientPlayer instance, PlayerModelPart playerModelPart) {


Why is this needed?

so the cape will show up? Look at the code and after double check it seems like it is needed for the cape to show up.

Redirecting this means that Java capes won't show because our cape map won't include official Java player capes. Also, this isModelPartShown redirects to player entity data and not anything in the rendering system. Is there an instance where this was actually needed?

Redirecting this means that Java capes won't show because our cape map won't include official Java player capes. Also, this isModelPartShown redirects to player entity data and not anything in the rendering system. Is there an instance where this was actually needed?

Added so now it also check for isModelPartShown, it used to check if the client should render a certain player cape, which in isModelPartShown it get entity data which is normally sent my by the server as my knowledge. Which is not going to get send on ViaBedrock, and maybe GeyserMC which I don't know.

I double check and do some debugging and the cape seems to not showing up without this part of code, if you have any doubt just debug it yourself.

Please remove this part of the code. I understand its use, but I don't want to erase the functionality of hiding a cape if Geyser ever decides to utilize it (technically, a server can already hide the cape of a player if it so desires). I have ran this by RK.

Please remove this part of the code. I understand its use, but I don't want to erase the functionality of hiding a cape if Geyser ever decides to utilize it (technically, a server can already hide the cape of a player if it so desires). I have ran this by RK.

Done.

I removed it as you requested.

Camotoy · 2024-09-15T02:04:45Z

Getting around to testing this. I have an HD skin from the Bloom marketplace pack (should be free), and there's a portion of the overalls on the chest that aren't showing.
However... can you elaborate on why you changed the format of how skins are stored? It seems that the Java skin the server sends is now liable to leaking out. (And I can't send a screenshot of what I mean)

Oryxel · 2024-09-15T03:26:36Z

Getting around to testing this. I have an HD skin from the Bloom marketplace pack (should be free), and there's a portion of the overalls on the chest that aren't showing. However... can you elaborate on why you changed the format of how skins are stored? It seems that the Java skin the server sends is now liable to leaking out. (And I can't send a screenshot of what I mean)

I'm not sure what happening about HD skins that you talking about, but the reason I changed how skins are stored because a lot of models and skins refused to show up even tho server still send them

These one for example which I already show up there.

Oryxel · 2024-09-15T03:27:46Z

I will test it with the bloom skins to see what wrong

Oryxel · 2024-09-15T03:54:27Z

Getting around to testing this. I have an HD skin from the Bloom marketplace pack (should be free), and there's a portion of the overalls on the chest that aren't showing. However... can you elaborate on why you changed the format of how skins are stored? It seems that the Java skin the server sends is now liable to leaking out. (And I can't send a screenshot of what I mean)

Found the issues, the HD skin Bloom needed custom geometry, but when the client received it it's just "null", so it applied skin without geometry, results the skin missing some parts.

Oryxel added 2 commits September 3, 2024 22:13

Improvements

870a089

Possibly fix compatibility by updating the library

0801bac

Camotoy reviewed Sep 6, 2024

View reviewed changes

Changes.

e287f86

Changes.

910645b

Camotoy reviewed Sep 7, 2024

View reviewed changes

Oryxel added 2 commits September 8, 2024 03:46

Changes.

5896323

Allow java cape if there is one.

d519db9

Oryxel requested a review from Camotoy September 11, 2024 15:39

Oryxel added 2 commits September 15, 2024 00:41

Changes.

8529ae5

Deleted unused class.

1554421

Don't do this outside if.

72270fa

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements #24

Improvements #24

Oryxel commented Sep 4, 2024

Camotoy Sep 6, 2024

Camotoy Sep 6, 2024

Camotoy Sep 14, 2024

Oryxel Sep 15, 2024

Camotoy Sep 6, 2024

Camotoy Sep 6, 2024

Oryxel commented Sep 6, 2024

Camotoy commented Sep 7, 2024

Oryxel commented Sep 7, 2024 •

edited

Loading

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Camotoy Sep 7, 2024

Oryxel Sep 7, 2024

Camotoy Sep 8, 2024

Oryxel Sep 9, 2024 •

edited

Loading

Camotoy Sep 14, 2024

Oryxel Sep 14, 2024

Oryxel Sep 14, 2024

Camotoy commented Sep 15, 2024

Oryxel commented Sep 15, 2024

Oryxel commented Sep 15, 2024

Oryxel commented Sep 15, 2024


		import net.minecraft.resources.ResourceLocation;

		public record CustomCapeData(ResourceLocation cape) {

Improvements #24

Are you sure you want to change the base?

Improvements #24

Conversation

Oryxel commented Sep 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Oryxel commented Sep 6, 2024

Camotoy commented Sep 7, 2024

Oryxel commented Sep 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Oryxel Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Camotoy commented Sep 15, 2024

Oryxel commented Sep 15, 2024

Oryxel commented Sep 15, 2024

Oryxel commented Sep 15, 2024

Oryxel commented Sep 7, 2024 •

edited

Loading

Oryxel Sep 9, 2024 •

edited

Loading