From 9dfd2d172446d8fb1805313d1687a0dbf53c76ac Mon Sep 17 00:00:00 2001 From: NotEvil Date: Fri, 17 Apr 2026 02:12:47 +0200 Subject: [PATCH] fix(validation): reduce log spam + add OOM guard + check correct mesh for WEIGHTS_0 - DataDrivenItemParser: downgrade animation_bones absence log from INFO to DEBUG (was spamming for every item without the optional field) - GlbValidator: read 12-byte GLB header first and reject files declaring totalLength > 50 MB before allocating (prevents OOM on malformed GLBs) - GlbValidator: WEIGHTS_0 check now uses the same mesh selection logic as GlbParser (prefer "Item", fallback to last non-Player) instead of blindly checking the first mesh --- docs/ARTIST_GUIDE.md | 25 +++-- .../client/gltf/diagnostic/GlbValidator.java | 97 ++++++++++++++----- .../datadriven/DataDrivenItemParser.java | 2 +- 3 files changed, 88 insertions(+), 36 deletions(-) diff --git a/docs/ARTIST_GUIDE.md b/docs/ARTIST_GUIDE.md index aa1eb79..cb6f14b 100644 --- a/docs/ARTIST_GUIDE.md +++ b/docs/ARTIST_GUIDE.md @@ -183,7 +183,8 @@ Weight paint your mesh to the skeleton bones it should follow. ### Rules -- **Only paint to the 11 standard bones.** Any other bones in your Blender file will be ignored by the mod. +- **Paint to the 11 standard bones** for parts that should follow the player's body. +- **You can also use custom bones** for decorative elements like chains, ribbons, pendants, or twist bones. Custom bones follow their parent in the bone hierarchy (rest pose) — they won't animate independently, but they move with the body part they're attached to. This is useful for better weight painting and mesh deformation. - **Paint to the bones of your regions.** Handcuffs (ARMS region) should be weighted to `rightUpperArm`, `rightLowerArm`, `leftUpperArm`, `leftLowerArm`. - **You can paint to bones outside your regions** for smooth deformation. For example, handcuffs might have small weights on `body` near the shoulder area for smoother bending. This is fine — the weight painting is about mesh deformation, not animation control. - **Normalize your weights.** Each vertex's total weights across all bones must sum to 1.0. Blender does this by default. @@ -193,6 +194,7 @@ Weight paint your mesh to the skeleton bones it should follow. - For rigid items (metal cuffs), use hard weights — each vertex fully assigned to one bone. - For flexible items (rope, leather), blend weights between adjacent bones for smooth bending. - The chain between handcuffs? Weight it 50/50 to both arms, or use a separate mesh element weighted to `body`. +- Custom bones are great for chains, dangling locks, or decorative straps — add a bone parented to a standard bone, weight your mesh to it, and it'll follow the parent's movement automatically. --- @@ -630,7 +632,8 @@ In your JSON definition, separate the mesh from the animations: - [ ] Actions are named `PlayerArmature|Idle`, `PlayerArmature|Struggle`, etc. - [ ] Mesh is weight-painted to skeleton bones only - [ ] Weights are normalized -- [ ] No orphan bones (extra bones not in the standard 11 are ignored but add file size) +- [ ] Custom bones (if any) are parented to a standard bone in the hierarchy +- [ ] Your item mesh is named `Item` in Blender (recommended — ensures the mod picks the correct mesh if your file has multiple objects) - [ ] Materials/textures are applied (the GLB bakes them in) - [ ] Scale is correct (1 Blender unit = 1 Minecraft block = 16 pixels) @@ -763,14 +766,16 @@ The `movement_style` changes how the player physically moves — slower speed, d | `movement_style` | string | No | Movement restriction: `"waddle"`, `"shuffle"`, `"hop"`, or `"crawl"` | | `movement_modifier` | object | No | Override speed/jump for the movement style (requires `movement_style`) | | `creator` | string | No | Author/creator name, shown in the item tooltip | -| `animation_bones` | object | Yes | Per-animation bone whitelist (see below) | +| `animation_bones` | object | No | Per-animation bone whitelist (see below). If omitted, all owned bones are enabled for all animations. | | `components` | object | No | Gameplay behavior components (see [Components](#components-gameplay-behaviors) below) | -### animation_bones (required) +### animation_bones (optional) -Declares which bones each named animation is allowed to control for this item. This enables fine-grained per-animation bone filtering: an item might own `body` via its regions but only want the "idle" animation to affect the arms. +Fine-grained control over which bones each animation is allowed to affect. Most items don't need this — if omitted, all owned bones (from your `regions`) are enabled for all animations automatically. -**Format:** A JSON object where each key is an animation name (matching the GLB animation names) and each value is an array of bone names. +**When to use it:** When your item owns bones via its regions but you only want specific animations to affect specific bones. For example, an item owning ARMS + TORSO might only want the "idle" pose to affect the arms, and only the "struggle" animation to also move the body. + +**Format:** A JSON object where each key is an animation name and each value is an array of bone names. **Valid bone names:** `head`, `body`, `rightArm`, `leftArm`, `rightLeg`, `leftLeg` @@ -784,7 +789,7 @@ Declares which bones each named animation is allowed to control for this item. T At runtime, the effective bones for a given animation clip are computed as the **intersection** of `animation_bones[clipName]` and the item's owned parts (from region conflict resolution). If the clip name is not listed in `animation_bones`, the item falls back to using all its owned parts. -This field is **required**. Items without `animation_bones` will be rejected by the parser. +**If omitted entirely:** All owned bones are enabled for all animations. This is the correct default for most items — you only need `animation_bones` for advanced per-animation filtering. ### Components (Gameplay Behaviors) @@ -1020,8 +1025,8 @@ Use your own namespace (e.g., `mycreator`) to avoid conflicts with the base mod | Mistake | Symptom | Fix | |---------|---------|-----| -| Bone name typo (`RightUpperArm` instead of `rightUpperArm`) | Mesh doesn't follow that bone | Names are **camelCase**, not PascalCase. Check exact spelling. | -| Extra bones in the armature | No visible issue (ignored), larger file | Delete non-standard bones before export | +| Bone name typo (`RightUpperArm` instead of `rightUpperArm`) | Warning in log: "did you mean 'rightUpperArm'?" — bone treated as custom | Names are **camelCase**, not PascalCase. Check exact spelling. Run `/tiedup validate` to see warnings. | +| Extra bones in the armature | Custom bones follow their parent in rest pose | Intentional custom bones are fine (chains, decorations). Unintentional ones add file size — delete them. | | Missing `PlayerArmature` root | Mesh renders at wrong position | Rename your armature root to `PlayerArmature` | | Animating `body` bone without TORSO region | Body keyframes used only if `body` is free (no other item owns it) | Declare TORSO/WAIST region if you always want to control body, or use `Full` animations for free-bone effects | @@ -1041,7 +1046,7 @@ Use your own namespace (e.g., `mycreator`) to avoid conflicts with the base mod |---------|---------|-----| | Vertices not weighted to any bone | Part of mesh stays frozen in space | Weight paint everything to at least one bone | | Weights not normalized | Mesh stretches or compresses oddly | Blender > Weights > Normalize All | -| Weighted to a non-standard bone | That part of mesh stays frozen | Only weight to the 11 standard bones | +| Weighted to a non-standard bone | Mesh follows parent bone in rest pose | This is OK if intentional (custom bones). If not, re-weight to a standard bone. | ### JSON Issues diff --git a/src/main/java/com/tiedup/remake/client/gltf/diagnostic/GlbValidator.java b/src/main/java/com/tiedup/remake/client/gltf/diagnostic/GlbValidator.java index 1df9c45..6b649aa 100644 --- a/src/main/java/com/tiedup/remake/client/gltf/diagnostic/GlbValidator.java +++ b/src/main/java/com/tiedup/remake/client/gltf/diagnostic/GlbValidator.java @@ -92,6 +92,9 @@ public final class GlbValidator { // Header + JSON chunk extraction // // ------------------------------------------------------------------ // + /** Maximum GLB file size the validator will accept (50 MB). */ + private static final long MAX_GLB_SIZE = 50L * 1024 * 1024; + /** * Parse the GLB header and extract the JSON chunk root object. * Returns null and adds ERROR diagnostics on failure. @@ -101,20 +104,21 @@ public final class GlbValidator { ResourceLocation source, List diagnostics ) throws Exception { - byte[] allBytes = input.readAllBytes(); - - if (allBytes.length < 12) { + // Read the 12-byte header first to check totalLength before + // committing to readAllBytes (OOM guard for malformed GLBs). + byte[] header = input.readNBytes(12); + if (header.length < 12) { diagnostics.add(new GlbDiagnostic( source, null, Severity.ERROR, "INVALID_GLB_HEADER", - "File too small to contain a GLB header (" + allBytes.length + " bytes)" + "File too small to contain a GLB header (" + header.length + " bytes)" )); return null; } - ByteBuffer buf = ByteBuffer.wrap(allBytes).order(ByteOrder.LITTLE_ENDIAN); + ByteBuffer headerBuf = ByteBuffer.wrap(header).order(ByteOrder.LITTLE_ENDIAN); // -- Header (12 bytes) -- - int magic = buf.getInt(); + int magic = headerBuf.getInt(); if (magic != GLB_MAGIC) { diagnostics.add(new GlbDiagnostic( source, null, Severity.ERROR, "INVALID_GLB_HEADER", @@ -123,7 +127,7 @@ public final class GlbValidator { return null; } - int version = buf.getInt(); + int version = headerBuf.getInt(); if (version != GLB_VERSION) { diagnostics.add(new GlbDiagnostic( source, null, Severity.ERROR, "INVALID_GLB_HEADER", @@ -132,8 +136,31 @@ public final class GlbValidator { return null; } - @SuppressWarnings("unused") - int totalLength = buf.getInt(); + int totalLength = headerBuf.getInt(); + + // OOM guard: reject files that declare a size exceeding the cap. + // totalLength is a signed int, so treat negative values as > 2 GB. + if (totalLength < 0 || Integer.toUnsignedLong(totalLength) > MAX_GLB_SIZE) { + diagnostics.add(new GlbDiagnostic( + source, null, Severity.ERROR, "GLB_TOO_LARGE", + "GLB declares totalLength=" + Integer.toUnsignedLong(totalLength) + + " bytes which exceeds the " + (MAX_GLB_SIZE / (1024 * 1024)) + + " MB safety cap — aborting validation" + )); + return null; + } + + // Now read the remainder (totalLength includes the 12-byte header) + int remaining = totalLength - 12; + if (remaining < 0) { + diagnostics.add(new GlbDiagnostic( + source, null, Severity.ERROR, "INVALID_GLB_HEADER", + "GLB totalLength (" + totalLength + ") is smaller than the header itself" + )); + return null; + } + byte[] restBytes = input.readNBytes(remaining); + ByteBuffer buf = ByteBuffer.wrap(restBytes).order(ByteOrder.LITTLE_ENDIAN); // -- First chunk must be JSON -- if (buf.remaining() < 8) { @@ -285,22 +312,42 @@ public final class GlbValidator { )); } - // Check WEIGHTS_0 on the first mesh's first primitive - if (meshes.size() > 0) { - JsonObject firstMesh = meshes.get(0).getAsJsonObject(); - if (firstMesh.has("primitives")) { - JsonArray primitives = firstMesh.getAsJsonArray("primitives"); - if (primitives.size() > 0) { - JsonObject firstPrimitive = primitives.get(0).getAsJsonObject(); - if (firstPrimitive.has("attributes")) { - JsonObject attributes = firstPrimitive.getAsJsonObject("attributes"); - if (!attributes.has("WEIGHTS_0")) { - diagnostics.add(new GlbDiagnostic( - source, null, Severity.WARNING, "NO_WEIGHTS", - "First mesh's first primitive has no WEIGHTS_0 attribute " - + "— skinning will not work correctly" - )); - } + // Check WEIGHTS_0 on the mesh that GlbParser would actually select: + // 1) mesh named "Item", 2) last non-Player mesh. + JsonObject targetMesh = null; + String targetMeshName = null; + for (int mi = 0; mi < meshes.size(); mi++) { + JsonObject mesh = meshes.get(mi).getAsJsonObject(); + String meshName = mesh.has("name") + ? mesh.get("name").getAsString() + : ""; + if ("Item".equals(meshName)) { + targetMesh = mesh; + targetMeshName = meshName; + break; // Convention match — same as GlbParser + } + if (!"Player".equals(meshName)) { + targetMesh = mesh; + targetMeshName = meshName; + } + } + + if (targetMesh != null && targetMesh.has("primitives")) { + JsonArray primitives = targetMesh.getAsJsonArray("primitives"); + if (primitives.size() > 0) { + JsonObject firstPrimitive = primitives.get(0).getAsJsonObject(); + if (firstPrimitive.has("attributes")) { + JsonObject attributes = firstPrimitive.getAsJsonObject("attributes"); + if (!attributes.has("WEIGHTS_0")) { + String meshLabel = targetMeshName != null + ? "'" + targetMeshName + "'" + : "(unnamed)"; + diagnostics.add(new GlbDiagnostic( + source, null, Severity.WARNING, "NO_WEIGHTS", + "Selected mesh " + meshLabel + + " first primitive has no WEIGHTS_0 attribute " + + "— skinning will not work correctly" + )); } } } diff --git a/src/main/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParser.java b/src/main/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParser.java index a8b9a03..2415cac 100644 --- a/src/main/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParser.java +++ b/src/main/java/com/tiedup/remake/v2/bondage/datadriven/DataDrivenItemParser.java @@ -267,7 +267,7 @@ public final class DataDrivenItemParser { if (!root.has("animation_bones")) { // Not an error — absent means "permissive" (all owned bones, all animations) animationBones = null; - LOGGER.info( + LOGGER.debug( "[DataDrivenItems] {}: animation_bones not declared — all owned bones enabled for all animations", fileId );