Mewtwo Update#667
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request adds a comprehensive cloning machine feature system comprising five animation controllers, six animation sequences, eight entity definitions, six geometry models, and supporting particle and texture assets. The changes are primarily data-driven JSON configuration files that define state machines, bone animations, and client-side entity rendering with minimal modifications to existing files. ChangesCloning Machine Feature System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Review focus areas:
Poem
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
textures/entity/containment_chamber.pngis excluded by!**/*.pngtextures/items/battery_holder.pngis excluded by!**/*.pngtextures/items/broken_mew_eyelash.pngis excluded by!**/*.pngtextures/items/containment_chamber.pngis excluded by!**/*.pngtextures/items/containment_fluid.pngis excluded by!**/*.pngtextures/items/cooling_station.pngis excluded by!**/*.pngtextures/items/damaged_mew_eyelash.pngis excluded by!**/*.pngtextures/items/dna_scanner.pngis excluded by!**/*.pngtextures/items/mew_eyelash.pngis excluded by!**/*.pngtextures/items/wrench.pngis excluded by!**/*.pngtextures/particle/magic.pngis excluded by!**/*.png
📒 Files selected for processing (30)
animation_controllers/cloningMachine/battery_holder.json(1 hunks)animation_controllers/cloningMachine/battery_holder_pipe.json(1 hunks)animation_controllers/cloningMachine/containment_chamber.json(1 hunks)animation_controllers/cloningMachine/containment_chamber_liquid.json(1 hunks)animation_controllers/cloningMachine/dna_scanner.json(1 hunks)animations/cloningMachine/battery_holder.animation.json(1 hunks)animations/cloningMachine/battery_holder_pipe.animation.json(1 hunks)animations/cloningMachine/containment_chamber.animation.json(1 hunks)animations/cloningMachine/containment_chamber_liquid.animation.json(1 hunks)animations/cloningMachine/cooling_station.animation.json(1 hunks)animations/item_holder.animation.json(1 hunks)entity/cloningMachine/battery_holder.json(1 hunks)entity/cloningMachine/containment_chamber.json(1 hunks)entity/cloningMachine/containment_chamber_liquid.json(1 hunks)entity/cloningMachine/cooling_station.json(1 hunks)entity/cloningMachine/dna_scanner.json(1 hunks)entity/cloningMachine/pipes/battery_holder_pipe.json(1 hunks)entity/cloningMachine/pipes/cooling_station_pipe.json(1 hunks)entity/cloningMachine/pipes/dna_scanner_pipe.json(1 hunks)entity/item_holder.json(1 hunks)entity/trainer.json(1 hunks)models/entity/cloningMachine/battery_holder.geo.json(1 hunks)models/entity/cloningMachine/battery_holder_pipe.geo.json(1 hunks)models/entity/cloningMachine/containment_chamber_liquid.geo.json(1 hunks)models/entity/cloningMachine/cooling_station.geo.json(1 hunks)models/entity/cloningMachine/cooling_station_pipe.geo.json(1 hunks)models/entity/cloningMachine/dna_scanner_pipe.geo.json(1 hunks)models/entity/item_holder.geo.json(1 hunks)particles/disinfect.particle.json(1 hunks)textures/item_texture.json(1 hunks)
🔇 Additional comments (5)
animations/cloningMachine/containment_chamber.animation.json (1)
19-118: Great staggered keyframe sequencing for bubble drift.The alternating pre/post easing you scripted on the bubble bones gives a lively fluid wobble without extra controllers—nicely done.
animations/cloningMachine/cooling_station.animation.json (1)
33-42: Confirm liquid/potion amount variables are populated.Both potion animations override prior state and immediately sample
variable.liquid_amount/variable.potion_amountto drive the fluid scale. If the controller doesn’t set those variables every tick before the override kicks in, the scale will collapse to 0 and the bottle appears empty. Please double-check the controller logic populates them (with sane defaults) during idle and each potion state.entity/cloningMachine/dna_scanner.json (1)
21-32: Clean animation/controller wiring.The controller slot and state animations are mapped consistently—looks solid.
textures/item_texture.json (1)
505-531: All new texture assets are present
Confirmed that eachtextures/items/*.pngfile referenced initem_texture.jsonexists.entity/item_holder.json (1)
15-28: Ensurepokeb:scalehas a valid default
q.property('pokeb:scale')returns 0 if unset, causing invisible holders. Confirm your server-side registration initializespokeb:scale(e.g. default 1) for every entity before relying on it.
| "controller": "controller.animation.battery_holder", | ||
| "empty": "animation.battery_holder.empty", | ||
| "one": "animation.battery_holder.one", | ||
| "two": "animation.battery_holder.two", | ||
| "three": "animation.battery_holder.three" | ||
| }, |
There was a problem hiding this comment.
Expose all animations to scripts
The entity stores extra animations (empty/one/two/three), yet the scripts.animate array only triggers the controller. Without calling animate for the child clips, Bedrock won’t actually play them. Please either move those clips into the controller or add them to scripts.animate.
| "animations": { | ||
| "default": "controller.animation.containment_chamber_liquid", | ||
| "water_fill0": "animation.containment_chamber_liquid.water_fill0", | ||
| "water_fill1": "animation.containment_chamber_liquid.water_fill1", | ||
| "water_fill2": "animation.containment_chamber_liquid.water_fill2", | ||
| "water_fill3": "animation.containment_chamber_liquid.water_fill3", | ||
| "water_fill4": "animation.containment_chamber_liquid.water_fill4", | ||
| "water_fill5": "animation.containment_chamber_liquid.water_fill5", | ||
| "water_fill6": "animation.containment_chamber_liquid.water_fill6", | ||
| "water_fill7": "animation.containment_chamber_liquid.water_fill7", | ||
| "water_fill8": "animation.containment_chamber_liquid.water_fill8", | ||
| "water_fill9": "animation.containment_chamber_liquid.water_fill9", | ||
| "water_fill10": "animation.containment_chamber_liquid.water_fill10", | ||
| "water_fill11": "animation.containment_chamber_liquid.water_fill11", | ||
| "water_fill12": "animation.containment_chamber_liquid.water_fill12", | ||
| "water_fill13": "animation.containment_chamber_liquid.water_fill13", | ||
| "water_fill14": "animation.containment_chamber_liquid.water_fill14", | ||
| "water_fill15": "animation.containment_chamber_liquid.water_fill15" |
There was a problem hiding this comment.
Controller misses liquid states
You declare 16 water_fill* animations but only reference the controller via scripts.animate. Unless the controller explicitly blends in these clips, they’ll never run. Consider mapping the controller output to the same names (or add them to scripts.animate) so each fill tier renders.
| "default": "textures/entity/containment_chamber" | ||
| }, | ||
| "geometry": { | ||
| "default": "geometry.dna_scanner_pipe" | ||
| }, | ||
| "scripts": { | ||
| "scale": "0.65" | ||
| }, | ||
| "render_controllers": [ | ||
| "controller.render.default" | ||
| ] |
There was a problem hiding this comment.
Texture path likely incorrect
This entity points textures/default to textures/entity/containment_chamber, but the pipeline geometry/animations for the DNA scanner pipe expect their own atlas entry (textures/entity/dna_scanner_pipe). With the current config the pipe will render with the containment chamber sheet, producing wrong UVs. Please switch to the dedicated DNA scanner pipe texture before shipping.
🤖 Prompt for AI Agents
In entity/cloningMachine/pipes/dna_scanner_pipe.json around lines 10–20, the
"textures" default is set to "textures/entity/containment_chamber" but the
geometry expects "textures/entity/dna_scanner_pipe"; update the "textures": {
"default": "textures/entity/dna_scanner_pipe" } entry to point to the dedicated
DNA scanner pipe texture and verify that the corresponding texture atlas file
and atlas key ("textures/entity/dna_scanner_pipe") exist in the assets so the
geometry UVs map correctly.
| 554 | ||
| ], | ||
| "uv_size": [ | ||
| 8, | ||
| 3 | ||
| ] | ||
| }, | ||
| "east": { | ||
| "uv": [ | ||
| 462, | ||
| 557 | ||
| ], | ||
| "uv_size": [ | ||
| 3, | ||
| 3 | ||
| ] | ||
| }, | ||
| "south": { | ||
| "uv": [ | ||
| 83, | ||
| 554 | ||
| ], | ||
| "uv_size": [ | ||
| 8, | ||
| 3 | ||
| ] | ||
| }, | ||
| "west": { | ||
| "uv": [ | ||
| 465, | ||
| 557 | ||
| ], | ||
| "uv_size": [ | ||
| 3, | ||
| 3 | ||
| ] | ||
| }, | ||
| "up": { | ||
| "uv": [ | ||
| 554, | ||
| 234 | ||
| ], | ||
| "uv_size": [ | ||
| 8, | ||
| 3 | ||
| ] | ||
| }, | ||
| "down": { | ||
| "uv": [ | ||
| 554, | ||
| 424 | ||
| ], | ||
| "uv_size": [ | ||
| 8, | ||
| -3 | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
UV down face has negative height
Cube on Line 344 sets "uv_size": [8, -3] for the down face. Bedrock treats face UV sizes as absolute dimensions; negative height mirrors/warps the texture and regularly produces rendering artifacts. Please make the value positive (swap origin if needed) to keep texels stable.
🤖 Prompt for AI Agents
In models/entity/cloningMachine/cooling_station_pipe.geo.json around lines 344
to 401, the cube's "down" face specifies "uv_size": [8, -3], which uses a
negative height and causes texture mirroring/artifacts; change the height to a
positive value (e.g., [8, 3]) and, if the visual orientation must be preserved,
flip the "uv" origin Y coordinate accordingly so the mapped region covers the
same texels with a positive size.
smell-of-curry
force-pushed
the
feature/mewtwo-update
branch
2 times, most recently
from
cec107e to
376d4eb
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
animations/cloningMachine/containment_chamber_liquid.animation.json (1)
14-26: Missing bubble bones in geometryAnimations still drive
bubbles,bubble2,bubble4, andbubble8, butmodels/entity/cloningMachine/containment_chamber_liquid.geo.jsondoes not define bones with those names. Every keyframe on these tracks is ignored at runtime, so the intended bubble motion never renders. Please either add the corresponding bones (with neutral transforms) to the geometry file or retarget these timelines to existing bones such aswaterparticle*/drop*.Also applies to: 84-96, 151-163
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
textures/entity/containment_chamber.pngis excluded by!**/*.pngtextures/items/battery_holder.pngis excluded by!**/*.pngtextures/items/broken_mew_eyelash.pngis excluded by!**/*.pngtextures/items/containment_chamber.pngis excluded by!**/*.pngtextures/items/containment_fluid.pngis excluded by!**/*.pngtextures/items/cooling_station.pngis excluded by!**/*.pngtextures/items/damaged_mew_eyelash.pngis excluded by!**/*.pngtextures/items/dna_scanner.pngis excluded by!**/*.pngtextures/items/mew_eyelash.pngis excluded by!**/*.pngtextures/items/wrench.pngis excluded by!**/*.pngtextures/particle/magic.pngis excluded by!**/*.png
📒 Files selected for processing (30)
animation_controllers/cloningMachine/battery_holder.json(1 hunks)animation_controllers/cloningMachine/battery_holder_pipe.json(1 hunks)animation_controllers/cloningMachine/containment_chamber.json(1 hunks)animation_controllers/cloningMachine/containment_chamber_liquid.json(1 hunks)animation_controllers/cloningMachine/dna_scanner.json(1 hunks)animations/cloningMachine/battery_holder.animation.json(1 hunks)animations/cloningMachine/battery_holder_pipe.animation.json(1 hunks)animations/cloningMachine/containment_chamber.animation.json(1 hunks)animations/cloningMachine/containment_chamber_liquid.animation.json(1 hunks)animations/cloningMachine/cooling_station.animation.json(1 hunks)animations/item_holder.animation.json(1 hunks)entity/cloningMachine/battery_holder.json(1 hunks)entity/cloningMachine/containment_chamber.json(1 hunks)entity/cloningMachine/containment_chamber_liquid.json(1 hunks)entity/cloningMachine/cooling_station.json(1 hunks)entity/cloningMachine/dna_scanner.json(1 hunks)entity/cloningMachine/pipes/battery_holder_pipe.json(1 hunks)entity/cloningMachine/pipes/cooling_station_pipe.json(1 hunks)entity/cloningMachine/pipes/dna_scanner_pipe.json(1 hunks)entity/item_holder.json(1 hunks)entity/trainer.json(1 hunks)models/entity/cloningMachine/battery_holder.geo.json(1 hunks)models/entity/cloningMachine/battery_holder_pipe.geo.json(1 hunks)models/entity/cloningMachine/containment_chamber_liquid.geo.json(1 hunks)models/entity/cloningMachine/cooling_station.geo.json(1 hunks)models/entity/cloningMachine/cooling_station_pipe.geo.json(1 hunks)models/entity/cloningMachine/dna_scanner_pipe.geo.json(1 hunks)models/entity/item_holder.geo.json(1 hunks)particles/disinfect.particle.json(1 hunks)textures/item_texture.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- animations/cloningMachine/battery_holder_pipe.animation.json
🚧 Files skipped from review as they are similar to previous changes (18)
- models/entity/cloningMachine/dna_scanner_pipe.geo.json
- entity/cloningMachine/pipes/battery_holder_pipe.json
- entity/cloningMachine/containment_chamber.json
- animations/cloningMachine/cooling_station.animation.json
- models/entity/cloningMachine/battery_holder.geo.json
- animations/item_holder.animation.json
- entity/cloningMachine/battery_holder.json
- entity/cloningMachine/pipes/cooling_station_pipe.json
- animation_controllers/cloningMachine/battery_holder_pipe.json
- entity/cloningMachine/containment_chamber_liquid.json
- models/entity/cloningMachine/cooling_station_pipe.geo.json
- animation_controllers/cloningMachine/containment_chamber_liquid.json
- entity/cloningMachine/pipes/dna_scanner_pipe.json
- animations/cloningMachine/battery_holder.animation.json
- models/entity/item_holder.geo.json
- particles/disinfect.particle.json
- textures/item_texture.json
- entity/cloningMachine/dna_scanner.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (1)
entity/item_holder.json (1)
15-20: Confirmpokeb:scaleproperty availability.The new scale script queries
q.property('pokeb:scale'). If that property isn’t defined (or defaults to 0) in the corresponding behavior entity, the holder will render at scale 0 and disappear. Please double-check thatpokeb:scaleexists with a default of 1 (or another sensible value) in the behavior pack before we ship this change.
| "textures": { | ||
| "default": "textures/entity/containment_chamber" | ||
| }, | ||
| "geometry": { | ||
| "default": "geometry.cooling_station" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Bind the cooling station to its own texture.
The client entity still points at textures/entity/containment_chamber, so the new geometry will render with the wrong atlas and the dedicated cooling_station texture never gets used. Swap this reference to the cooling station texture asset.
Run this to confirm the available texture path:
🏁 Script executed:
#!/bin/bash
# Locate cooling-station textures to wire up the correct resource
fd --ignore-file .gitignore --type f 'cooling_station' texturesLength of output: 34
Update cooling station texture reference
Change the default texture in entity/cloningMachine/cooling_station.json from textures/entity/containment_chamber to textures/items/cooling_station so the correct asset is used.
🤖 Prompt for AI Agents
In entity/cloningMachine/cooling_station.json around lines 9 to 14, the
"textures.default" value references the wrong asset
("textures/entity/containment_chamber"); update that field to
"textures/items/cooling_station" so the JSON points to the correct texture
resource for the cooling station.
pyris-dev
force-pushed
the
feature/mewtwo-update
branch
from
376d4eb to
e9266f7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
animation_controllers/cloningMachine/containment_chamber_liquid.json (1)
7-886: Add the missing fill_16 state.Line 8: The controller tops out at
fill_15, so whenvariable.fluid_amount == 16the machine never leaves the previous state andwater_fill16can’t play. Please add afill_16state (wired towater_fill16) and extend every state’s transitions with a"fill_16": "variable.fluid_amount == 16"branch so the last fill tier is reachable.{ "fill_15": "variable.fluid_amount == 15" + }, + { + "fill_16": "variable.fluid_amount == 16" } ] }, + "fill_16": { + "animations": [ + "water_fill16" + ], + "transitions": [ + { + "fill_0": "variable.fluid_amount == 0" + }, + ... + { + "fill_16": "variable.fluid_amount == 16" + } + ] + }
🧹 Nitpick comments (3)
animations/cloningMachine/containment_chamber.animation.json (3)
1735-1743: Add explicit animation_length to ‘empty’.Not required but avoids engine defaults ambiguity. Recommend 0s static.
"animation.containment_chamber.empty": { - "loop": "hold_on_last_frame", + "loop": "hold_on_last_frame", + "animation_length": 0, "bones": {
624-626: Unused ‘sparks’ bone?It’s permanently hidden here and not driven elsewhere. Remove to reduce payload, or wire it in if intended.
- "sparks": { - "scale": [0, 0, 1] - },
220-621: Reduce duplication for green/green1–5 tracks.These are near‑identical oscillations with phase/sign flips. Consider generating these tracks via a small script (source-of-truth) to avoid manual drift and ease updates.
I can provide a tiny generator (e.g., Node/Python) and an ADR note.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
textures/entity/containment_chamber.pngis excluded by!**/*.pngtextures/items/battery_holder.pngis excluded by!**/*.pngtextures/items/broken_mew_eyelash.pngis excluded by!**/*.pngtextures/items/containment_chamber.pngis excluded by!**/*.pngtextures/items/containment_fluid.pngis excluded by!**/*.pngtextures/items/cooling_station.pngis excluded by!**/*.pngtextures/items/damaged_mew_eyelash.pngis excluded by!**/*.pngtextures/items/dna_scanner.pngis excluded by!**/*.pngtextures/items/mew_eyelash.pngis excluded by!**/*.pngtextures/items/wrench.pngis excluded by!**/*.pngtextures/particle/magic.pngis excluded by!**/*.png
📒 Files selected for processing (30)
animation_controllers/cloningMachine/battery_holder.json(1 hunks)animation_controllers/cloningMachine/battery_holder_pipe.json(1 hunks)animation_controllers/cloningMachine/containment_chamber.json(1 hunks)animation_controllers/cloningMachine/containment_chamber_liquid.json(1 hunks)animation_controllers/cloningMachine/dna_scanner.json(1 hunks)animations/cloningMachine/battery_holder.animation.json(1 hunks)animations/cloningMachine/battery_holder_pipe.animation.json(1 hunks)animations/cloningMachine/containment_chamber.animation.json(1 hunks)animations/cloningMachine/containment_chamber_liquid.animation.json(1 hunks)animations/cloningMachine/cooling_station.animation.json(1 hunks)animations/item_holder.animation.json(1 hunks)entity/cloningMachine/battery_holder.json(1 hunks)entity/cloningMachine/containment_chamber.json(1 hunks)entity/cloningMachine/containment_chamber_liquid.json(1 hunks)entity/cloningMachine/cooling_station.json(1 hunks)entity/cloningMachine/dna_scanner.json(1 hunks)entity/cloningMachine/pipes/battery_holder_pipe.json(1 hunks)entity/cloningMachine/pipes/cooling_station_pipe.json(1 hunks)entity/cloningMachine/pipes/dna_scanner_pipe.json(1 hunks)entity/item_holder.json(1 hunks)entity/trainer.json(1 hunks)models/entity/cloningMachine/battery_holder.geo.json(1 hunks)models/entity/cloningMachine/battery_holder_pipe.geo.json(1 hunks)models/entity/cloningMachine/containment_chamber_liquid.geo.json(1 hunks)models/entity/cloningMachine/cooling_station.geo.json(1 hunks)models/entity/cloningMachine/cooling_station_pipe.geo.json(1 hunks)models/entity/cloningMachine/dna_scanner_pipe.geo.json(1 hunks)models/entity/item_holder.geo.json(1 hunks)particles/disinfect.particle.json(1 hunks)textures/item_texture.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (20)
- entity/cloningMachine/battery_holder.json
- models/entity/cloningMachine/containment_chamber_liquid.geo.json
- entity/cloningMachine/pipes/battery_holder_pipe.json
- animation_controllers/cloningMachine/containment_chamber.json
- entity/item_holder.json
- animations/cloningMachine/battery_holder.animation.json
- animations/cloningMachine/containment_chamber_liquid.animation.json
- entity/cloningMachine/dna_scanner.json
- entity/cloningMachine/containment_chamber_liquid.json
- animations/cloningMachine/battery_holder_pipe.animation.json
- models/entity/cloningMachine/cooling_station.geo.json
- models/entity/cloningMachine/dna_scanner_pipe.geo.json
- entity/cloningMachine/pipes/cooling_station_pipe.json
- particles/disinfect.particle.json
- entity/cloningMachine/containment_chamber.json
- entity/cloningMachine/pipes/dna_scanner_pipe.json
- models/entity/item_holder.geo.json
- models/entity/cloningMachine/battery_holder.geo.json
- entity/trainer.json
- animations/item_holder.animation.json
🔇 Additional comments (5)
animation_controllers/cloningMachine/dna_scanner.json (1)
1-105: Controller looks solid.State definitions and transitions line up correctly with the expected
pokeb:statevalues. Nice work keeping the transitions comprehensive.animations/cloningMachine/containment_chamber.animation.json (4)
1120-1131: Intent check: water5 scale collapses X to 0 at 9.0417.Filling ends by setting scale.x from 0.6 → 0, which may visually pop/hide this layer. Confirm this is intentional vs. [1,1,1].
1486-1550: MoLang rotation usage LGTM.Dynamic spin on glassparticle1/2 with timed show/hide is correct and idiomatic.
2-2: Confirm format_version matches target engine.1.8.0 is commonly fine for animations; verify against your pack’s minimum supported Bedrock version.
4-4: Animation references are correctly defined and used.Verification confirms all six animations (
filling,breaking,opening,closing,opening_ditto,empty) are properly defined in the animation file and correctly referenced in the entity file with exact name matching. No action needed.
| "scale": "0.75", | ||
| "pre_animation": [ | ||
| "variable.potion_amount = query.property('pokeb:potion_amount');", | ||
| "variable.has_fluid = query.property('pokeb:has_fluid');" | ||
| ], | ||
| "animate": [ | ||
| "idle" | ||
| ] |
There was a problem hiding this comment.
Initialize variable.liquid_amount before the animation runs.
Lines 18‑22: animation.cooling_station.potion* scales water6 with variable.liquid_amount, but the pre-animation script never sets that variable, so it stays 0 and the fluid column never appears during potion cycles. Add the property lookup alongside the existing assignments.
"pre_animation": [
"variable.potion_amount = query.property('pokeb:potion_amount');",
- "variable.has_fluid = query.property('pokeb:has_fluid');"
+ "variable.has_fluid = query.property('pokeb:has_fluid');",
+ "variable.liquid_amount = query.property('pokeb:liquid_amount');"
],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "scale": "0.75", | |
| "pre_animation": [ | |
| "variable.potion_amount = query.property('pokeb:potion_amount');", | |
| "variable.has_fluid = query.property('pokeb:has_fluid');" | |
| ], | |
| "animate": [ | |
| "idle" | |
| ] | |
| "scale": "0.75", | |
| "pre_animation": [ | |
| "variable.potion_amount = query.property('pokeb:potion_amount');", | |
| "variable.has_fluid = query.property('pokeb:has_fluid');", | |
| "variable.liquid_amount = query.property('pokeb:liquid_amount');" | |
| ], | |
| "animate": [ | |
| "idle" | |
| ] |
🤖 Prompt for AI Agents
In entity/cloningMachine/cooling_station.json around lines 16 to 23, the
pre_animation block initializes variable.potion_amount and variable.has_fluid
but never sets variable.liquid_amount, so animation steps that scale water6 with
variable.liquid_amount remain zero; add a line in the pre_animation array to
assign variable.liquid_amount = query.property('pokeb:liquid_amount'); (placed
alongside the existing property lookups) so the fluid column is initialized
before animations run.
a5f4e0f to
2a0c5a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
animation_controllers/cloningMachine/containment_chamber_liquid.json (1)
6-886:⚠️ Critical issue persists: Missingfill_16state and transitions.This is the same issue flagged in the previous review. The animation file (
animations/cloningMachine/containment_chamber_liquid.animation.json) defines awater_fill16animation, but the controller still lacks a correspondingfill_16state and transitions forvariable.fluid_amount == 16. When fluid level reaches 16, the controller cannot transition to the final fill state and the animation remains stuck onfill_15.Add a
fill_16state (lines 887–932, before the closing brace on line 887) that referenceswater_fill16, and update every existing state's transitions array to include afill_16entry for the conditionvariable.fluid_amount == 16.Example addition for
fill_16state:"fill_16": { "animations": [ "water_fill16" ], "transitions": [ { "fill_0": "variable.fluid_amount == 0" }, { "fill_1": "variable.fluid_amount == 1" }, ... { "fill_16": "variable.fluid_amount == 16" } ] }Then add
{ "fill_16": "variable.fluid_amount == 16" }as the final transition entry in every existing state (e.g., lines 11–60 forfill_0, lines 66–115 forfill_1, etc.).entity/cloningMachine/pipes/dna_scanner_pipe.json (1)
10-10: Texture path mismatch—same critical issue from previous review.Line 10 incorrectly references
textures/entity/containment_chamber, but the geometrygeometry.dna_scanner_piperequires its own dedicated texture atlastextures/entity/dna_scanner_pipe. The current mismatch will cause incorrect UV mapping and visual corruption at runtime.Apply this fix:
"textures": { - "default": "textures/entity/containment_chamber" + "default": "textures/entity/dna_scanner_pipe" }Ensure the texture atlas file
textures/entity/dna_scanner_pipe.pngand corresponding entry in the item/block texture registry exist in the repository.
🧹 Nitpick comments (3)
animations/cloningMachine/containment_chamber.animation.json (1)
18-621: Extreme code duplication across green particle bones.The
green,green1,green2,green3,green4, andgreen5bones (lines 18–621) follow nearly identical keyframe patterns with the same 25 timestamps, creating a ~600-line sparkling effect. The only variation is z-position alternating between+1/-1(opposite pairs), which suggests this was copy-pasted or generated. While this is valid and functional, it significantly impacts maintainability.Consider one or more of the following for future iterations:
- Document the pattern in a code comment explaining that these bones create a synchronized flickering effect.
- Use an animation generator (external tool/build step) to reduce manual duplication.
- Group bones by behavior with a naming convention (e.g.,
green_flicker_pos,green_flicker_neg) to make intent clearer.Example comment to add near line 18:
"green": { "position": { "0.0": [0, 0, 0], // Synchronized flicker effect: z oscillates 0 ↔ 1 every ~0.5 frames for 11 seconds "0.5": { "pre": [0, 0, 0], "post": [0, 0, 1] }, ...models/entity/cloningMachine/cooling_station.geo.json (1)
14-419: Consider renaming auto-generated bone identifiers for maintainability. Bones such asbone,bone2,bone3,bone49,bone50,bone51,bone52,bone53are Blockbench-generated defaults. More descriptive names (e.g.,cooler_frame,potion_hatch_top,potion_hatch_left,water_fill_layer, etc.) would improve readability and make animations and related configurations easier to maintain.particles/disinfect.particle.json (1)
1-73: Consider adding a trailing newline to the JSON file.The file lacks a trailing newline at the end, which is a common formatting convention and may trigger linter warnings in some CI/CD pipelines.
Apply this diff to add a trailing newline:
} } } +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
textures/entity/containment_chamber.pngis excluded by!**/*.pngtextures/items/battery_holder.pngis excluded by!**/*.pngtextures/items/broken_mew_eyelash.pngis excluded by!**/*.pngtextures/items/containment_chamber.pngis excluded by!**/*.pngtextures/items/containment_fluid.pngis excluded by!**/*.pngtextures/items/cooling_station.pngis excluded by!**/*.pngtextures/items/damaged_mew_eyelash.pngis excluded by!**/*.pngtextures/items/dna_scanner.pngis excluded by!**/*.pngtextures/items/mew_eyelash.pngis excluded by!**/*.pngtextures/items/wrench.pngis excluded by!**/*.pngtextures/particle/magic.pngis excluded by!**/*.png
📒 Files selected for processing (30)
animation_controllers/cloningMachine/battery_holder.json(1 hunks)animation_controllers/cloningMachine/battery_holder_pipe.json(1 hunks)animation_controllers/cloningMachine/containment_chamber.json(1 hunks)animation_controllers/cloningMachine/containment_chamber_liquid.json(1 hunks)animation_controllers/cloningMachine/dna_scanner.json(1 hunks)animations/cloningMachine/battery_holder.animation.json(1 hunks)animations/cloningMachine/battery_holder_pipe.animation.json(1 hunks)animations/cloningMachine/containment_chamber.animation.json(1 hunks)animations/cloningMachine/containment_chamber_liquid.animation.json(1 hunks)animations/cloningMachine/cooling_station.animation.json(1 hunks)animations/item_holder.animation.json(1 hunks)entity/cloningMachine/battery_holder.json(1 hunks)entity/cloningMachine/containment_chamber.json(1 hunks)entity/cloningMachine/containment_chamber_liquid.json(1 hunks)entity/cloningMachine/cooling_station.json(1 hunks)entity/cloningMachine/dna_scanner.json(1 hunks)entity/cloningMachine/pipes/battery_holder_pipe.json(1 hunks)entity/cloningMachine/pipes/cooling_station_pipe.json(1 hunks)entity/cloningMachine/pipes/dna_scanner_pipe.json(1 hunks)entity/item_holder.json(1 hunks)entity/trainer.json(1 hunks)models/entity/cloningMachine/battery_holder.geo.json(1 hunks)models/entity/cloningMachine/battery_holder_pipe.geo.json(1 hunks)models/entity/cloningMachine/containment_chamber_liquid.geo.json(1 hunks)models/entity/cloningMachine/cooling_station.geo.json(1 hunks)models/entity/cloningMachine/cooling_station_pipe.geo.json(1 hunks)models/entity/cloningMachine/dna_scanner_pipe.geo.json(1 hunks)models/entity/item_holder.geo.json(1 hunks)particles/disinfect.particle.json(1 hunks)textures/item_texture.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (18)
- animation_controllers/cloningMachine/battery_holder_pipe.json
- entity/trainer.json
- entity/cloningMachine/cooling_station.json
- models/entity/cloningMachine/battery_holder.geo.json
- animations/item_holder.animation.json
- entity/cloningMachine/battery_holder.json
- entity/cloningMachine/containment_chamber_liquid.json
- entity/cloningMachine/containment_chamber.json
- animations/cloningMachine/cooling_station.animation.json
- models/entity/cloningMachine/containment_chamber_liquid.geo.json
- entity/cloningMachine/dna_scanner.json
- entity/cloningMachine/pipes/battery_holder_pipe.json
- animations/cloningMachine/containment_chamber_liquid.animation.json
- animation_controllers/cloningMachine/dna_scanner.json
- animations/cloningMachine/battery_holder.animation.json
- entity/item_holder.json
- animation_controllers/cloningMachine/containment_chamber.json
- textures/item_texture.json
🔇 Additional comments (11)
models/entity/item_holder.geo.json (1)
9-11: Verify visible bounds accommodate new animations and scaling.The visible bounds adjustments (width: 1→2, height: 1→1.5, offset +0.25 on Y) are reasonable given the PR adds idle animations and scaling to this entity. However, confirm these bounds are sufficient to contain the full animation range and any scaling transformations, especially during animated rotations or scale changes referenced in
entity/item_holder.json.To verify the bounds are correct:
- Check the actual bone geometry dimensions (cubes in this file, not shown in the diff)
- Simulate the idle animation to ensure no part of the scaled/animated item extends beyond the new visible bounds
- Confirm the Y-offset of 0.25 properly centers the bounds accounting for animation motion
models/entity/cloningMachine/dna_scanner_pipe.geo.json (1)
1-267: Geometry asset is well-formed.The geometry definition is structurally sound with proper bone hierarchy, pivot points, rotations, and complete UV mappings across all cube faces. However, note that this asset's visual correctness depends on the texture path issue in
entity/cloningMachine/pipes/dna_scanner_pipe.jsonbeing resolved first.animations/cloningMachine/battery_holder_pipe.animation.json (1)
1-266: Animation structure is well-formed with proper bone targeting and timing.The file correctly defines four animations with consistent bone references (bone56, bone54, bone57, bone55) that align with the geometry model. Keyframe timing is precise, with final keyframes properly bounded at or near each animation's total length. The use of pre/post transitions for easing is a good practice.
Ensure the animation controller at
animation_controllers/cloningMachine/battery_holder_pipe.jsonreferences these four animations and maps them to the correct state machine transitions (e.g., viapokeb:batteriesproperty as mentioned in the enriched summary).models/entity/cloningMachine/battery_holder_pipe.geo.json (1)
1-996: Geometry structure properly supports animation and follows Bedrock model conventions.The bone hierarchy is well-designed with all animated bones correctly present. The hierarchical tree (root → bone37/35/34 → animated children) enables the animation system to apply transforms appropriately. UV mappings are consistent with per-face coordinates properly bounded within the 1024×1024 texture, and the use of negative uv_size for "down" faces correctly flips UVs per Bedrock conventions. The selective use of
inflate: 0.01on certain cubes is appropriate for preventing z-fighting in overlapping geometry.Verify that the client entity at
entity/cloningMachine/pipes/battery_holder_pipe.jsoncorrectly referencesgeometry.battery_holder_pipeand that all geometry bones are bound to the animation controller defined inanimation_controllers/cloningMachine/battery_holder_pipe.json.entity/cloningMachine/pipes/cooling_station_pipe.json (3)
9-10: Verify texture path is intentional for cooling_station_pipe.The entity references
"textures/entity/containment_chamber"rather than a cooling-station-specific texture. Confirm this texture asset reuse is intentional.
6-7: Confirm "zombie" material is the intended choice.The material is set to
"zombie"rather than a default or custom material. Verify this is intentional for the desired visual rendering.
15-20: No animation controller defined—confirm static entity is intended.The entity definition includes no
animation_controllerin the description, meaning the cooling station pipe will not animate. The PR summary mentions cooling station animations exist. Verify whether this entity should include an animation controller or remain static.models/entity/cloningMachine/cooling_station.geo.json (2)
1-12: Geometry structure is well-formed. JSON syntax is valid, format version is appropriate, and texture dimensions (1024×1024) are standard for complex models. Visible bounds and pivot positioning look reasonable for a cooling station structure.
232-250: No animation files exist for cooling_station—bone53 is static geometry. Verification confirms bone53 in cooling_station.geo.json:233 is defined only as static geometry. Unlike other entities (e.g., archeops), the cloningMachine cooling_station entity has no associated animation files. The zero-height plane[17, 0, 8]is intentional static geometry, likely for visual layering in the model structure.particles/disinfect.particle.json (2)
12-42: Particle physics and lifecycle configuration looks solid.The emitter configuration (spawn_rate 37, max_particles 100) and particle lifecycle (1-second lifetime with 1-second active emitter time) create a steady, looping disinfect effect. The disc emission shape and downward direction/acceleration (-6 linear_acceleration) are well-suited for a disinfecting particle that spreads and falls.
49-69: No issues found—configuration is optimal.The texture dimensions (16×128) match the configuration exactly. With 8×8 frames and step_UV [0, 16] (vertical stepping), the 2×2 base_UV randomization creates 4 distinct animation sequences, each using 8 frames across the full 16×128 texture. All 32 frame slots are utilized efficiently, and the animation will render without errors or visual glitches. The particle effect is ready as-is.
| { | ||
| "format_version": "1.18.3", | ||
| "animation_controllers": { | ||
| "controller.animation.battery_holder": { | ||
| "initial_state": "empty", | ||
| "states": { | ||
| "empty": { | ||
| "animations": [ | ||
| "empty" | ||
| ], | ||
| "transitions": [ | ||
| { | ||
| "one": "q.property('pokeb:batteries') == 1" | ||
| }, | ||
| { | ||
| "two": "q.property('pokeb:batteries') == 2" | ||
| }, | ||
| { | ||
| "three": "q.property('pokeb:batteries') == 3" | ||
| } | ||
| ] | ||
| }, | ||
| "one": { | ||
| "animations": [ | ||
| "one" | ||
| ], | ||
| "transitions": [ | ||
| { | ||
| "empty": "q.property('pokeb:batteries') == 0" | ||
| }, | ||
| { | ||
| "two": "q.property('pokeb:batteries') == 2" | ||
| }, | ||
| { | ||
| "three": "q.property('pokeb:batteries') == 3" | ||
| } | ||
| ] | ||
| }, | ||
| "two": { | ||
| "animations": [ | ||
| "two" | ||
| ], | ||
| "transitions": [ | ||
| { | ||
| "empty": "q.property('pokeb:batteries') == 0" | ||
| }, | ||
| { | ||
| "one": "q.property('pokeb:batteries') == 1" | ||
| }, | ||
| { | ||
| "three": "q.property('pokeb:batteries') == 3" | ||
| } | ||
| ] | ||
| }, | ||
| "three": { | ||
| "animations": [ | ||
| "three" | ||
| ], | ||
| "transitions": [ | ||
| { | ||
| "empty": "q.property('pokeb:batteries') == 0" | ||
| }, | ||
| { | ||
| "one": "q.property('pokeb:batteries') == 1" | ||
| }, | ||
| { | ||
| "two": "q.property('pokeb:batteries') == 2" | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
Well-structured animation state machine with complete transition coverage.
The animation controller defines a four-state machine (empty, one, two, three) with full bidirectional transitions driven by the pokeb:batteries property. The structure is consistent and logically sound—each state transitions to the other three when the property matches the corresponding value.
However, verify the following dependencies:
- Animation names (lines 9, 25, 41, 57): Confirm that animations named "empty", "one", "two", and "three" exist and are defined in
animations/cloningMachine/battery_holder.animation.json. - Property setup (lines 13, 16, 19, etc.): Verify that the entity referencing this controller (
entity/cloningMachine/battery_holder.json) properly initializes and updates thepokeb:batteriesproperty. - Edge case handling: If
pokeb:batteriescan take values outside the range [0, 3], the controller will remain in its current state indefinitely. Confirm this is the intended behavior or add a fallback transition if needed.
Run this script to verify the animation references:
🏁 Script executed:
#!/bin/bash
# Verify animation definitions exist for battery_holder animations
# Check if the animations file exists and contains the required animation names
if [ -f "animations/cloningMachine/battery_holder.animation.json" ]; then
echo "✓ Animation file found"
# Extract animation names
rg -o '"(empty|one|two|three)":\s*\{' animations/cloningMachine/battery_holder.animation.json | wc -l
else
echo "✗ Animation file not found"
fi
# Check entity file references this controller
rg -n 'battery_holder' entity/cloningMachine/battery_holder.json | head -10Length of output: 443
I need to inspect the actual content of the animation file to understand why the animation names weren't found.
🏁 Script executed:
#!/bin/bash
# Inspect the animation file content to verify animation definitions
echo "=== Animation file content (first 100 lines) ==="
head -100 animations/cloningMachine/battery_holder.animation.json
echo -e "\n=== Animation file length ==="
wc -l animations/cloningMachine/battery_holder.animation.json
echo -e "\n=== Search for animation object keys ==="
rg -n '"animations"' animations/cloningMachine/battery_holder.animation.jsonLength of output: 419
The animation file doesn't exist at the assumed path. Let me search for where animation files are actually located.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for animation files related to battery_holder
echo "=== Searching for battery_holder animation files ==="
fd -t f 'battery_holder.*animation' --case-sensitive
echo -e "\n=== Searching for any .animation.json files ==="
fd -t f '\.animation\.json$' | head -20
echo -e "\n=== Searching for 'animations' directories ==="
fd -t d 'animations' --case-sensitive | head -10
Length of output: 1152
Now that the file location is confirmed, let me inspect its content to verify the animation definitions exist.
---
🏁 Script executed:
```shell
#!/bin/bash
# Read animation file to verify animation definitions
echo "=== Animation file content ==="
cat animations/cloningMachine/battery_holder.animation.json
echo -e "\n=== Verify animation names in controller ==="
rg -n '"(empty|one|two|three)"' animations/cloningMachine/battery_holder.animation.json
Length of output: 5423
Now let me verify that the entity file has complete animation mappings for all four animation names.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check entity file for complete animation mappings
echo "=== Entity file animation mappings section ==="
rg -A 10 '"animations"' entity/cloningMachine/battery_holder.json | head -20
echo -e "\n=== Verify all 4 animation mappings exist ==="
rg -c '"empty"|"one"|"two"|"three"' entity/cloningMachine/battery_holder.json
echo -e "\n=== Check property initialization ==="
rg -n 'pokeb:batteries' entity/cloningMachine/battery_holder.json
Length of output: 577
The property pokeb:batteries is not appearing in the output. Let me check if it's initialized through entity components or elsewhere in the entity file.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check full entity file structure for property initialization
echo "=== Full entity file content ==="
cat entity/cloningMachine/battery_holder.json
echo -e "\n=== Search for any property/component initialization ==="
rg -i 'component|property' entity/cloningMachine/battery_holder.json
Length of output: 1113
The file shown is a client-side entity. Properties are typically defined in behavior entity files. Let me search for a behavior entity file.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for behavior entity files for battery_holder
echo "=== Search for behavior entity files ==="
fd -t f 'battery_holder' | grep -i entity
echo -e "\n=== Search for any behavior pack directories ==="
fd -t d 'behavior' | head -10
echo -e "\n=== Search for all entity files in behavior directories ==="
find . -path '*/entities/*battery_holder*' 2>/dev/null
Length of output: 381
Let me search the entire codebase for where the pokeb:batteries property is being set or initialized.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for all references to pokeb:batteries property
echo "=== All references to pokeb:batteries ==="
rg -r 'pokeb:batteries' --type-list | head -5
rg -n 'pokeb:batteries'
echo -e "\n=== Search for property setting mechanisms ==="
rg -n 'property.*batteries|batteries.*property'
echo -e "\n=== Check behavior scripts directory ==="
ls -la scripts/data/behaviors/ 2>/dev/null || echo "Directory empty or missing"
Length of output: 6974
Critical issue: pokeb:batteries property is never initialized, causing animation controller to malfunction.
The animation controller structure is correct and all animation references exist, but the implementation is incomplete:
- Animation definitions ✓: All four animations (empty, one, two, three) are properly defined in
animations/cloningMachine/battery_holder.animation.json - Animation mappings ✓: Entity file correctly maps animation names to full animation definitions
- Property initialization ✗: The
pokeb:batteriesproperty referenced in all transition conditions is never initialized anywhere in the codebase
Without initialization, the property will remain undefined and the animation controller will remain stuck in the "empty" state indefinitely. All transition conditions will fail to match.
Add property initialization to the entity's component section or create a behavior entity file that sets pokeb:batteries to values 0-3.
🤖 Prompt for AI Agents
In animation_controllers/cloningMachine/battery_holder.json lines 1-74: the
controller references q.property('pokeb:batteries') but that property is never
initialized; add initialization so the controller can transition. Fix by adding
a pokeb:batteries component/property with a default value (0) to the entity's
components section or create a behavior/behavior_pack entity that defines
pokeb:batteries (integer) and sets its initial value to 0 and bounds/valid
values 0–3; ensure the property is synced/available to the animation controller
and any code that updates it so transitions can evaluate correctly.
| "glassparticle1": { | ||
| "rotation": { | ||
| "6.2917": [0, "(query.anim_time*240)", 0], | ||
| "6.5417": [0, "(query.anim_time*240)", 0], | ||
| "6.625": [0, 0, 0] | ||
| }, | ||
| "position": { | ||
| "6.25": [237, -25, 104], | ||
| "6.3333": [247, -35, 66], | ||
| "6.4167": [255, -51, 59], | ||
| "6.5": [261, -80, 59], | ||
| "6.5833": [261, -98, 53] | ||
| }, | ||
| "scale": { | ||
| "0.0": [0, 0, 0], | ||
| "6.2083": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "6.25": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0.5, 0.5, 0.5] | ||
| }, | ||
| "6.2917": { | ||
| "pre": [0.5, 0.5, 0.5], | ||
| "post": [1.2, 1.2, 1.2] | ||
| }, | ||
| "6.625": { | ||
| "pre": [1.2, 1.2, 1.2], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "glassparticle2": { | ||
| "rotation": { | ||
| "6.2917": [0, "(query.anim_time*360)", 0], | ||
| "6.5417": [0, "(query.anim_time*360)", 0], | ||
| "6.625": [0, 0, 0] |
There was a problem hiding this comment.
Query expressions in rotation fields are not valid animation format.
Lines 1487–1488 and 1520–1521 use string expressions like "(query.anim_time*240)" in rotation arrays, but the Bedrock animation format requires numeric [x, y, z] values. Query expressions belong in scripting contexts, not animation keyframes. This will fail to parse or animate.
Replace the query expressions with static rotation values:
"glassparticle1": {
"rotation": {
- "6.2917": [0, "(query.anim_time*240)", 0],
- "6.5417": [0, "(query.anim_time*240)", 0],
+ "6.2917": [0, 0, 0],
+ "6.5417": [0, 0, 0],
"6.625": [0, 0, 0]
},If dynamic rotation is intended, you'll need to use a script or animation controller with state-driven keyframes instead of query expressions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "glassparticle1": { | |
| "rotation": { | |
| "6.2917": [0, "(query.anim_time*240)", 0], | |
| "6.5417": [0, "(query.anim_time*240)", 0], | |
| "6.625": [0, 0, 0] | |
| }, | |
| "position": { | |
| "6.25": [237, -25, 104], | |
| "6.3333": [247, -35, 66], | |
| "6.4167": [255, -51, 59], | |
| "6.5": [261, -80, 59], | |
| "6.5833": [261, -98, 53] | |
| }, | |
| "scale": { | |
| "0.0": [0, 0, 0], | |
| "6.2083": { | |
| "pre": [0, 0, 0], | |
| "post": [0, 0, 0] | |
| }, | |
| "6.25": { | |
| "pre": [0, 0, 0], | |
| "post": [0.5, 0.5, 0.5] | |
| }, | |
| "6.2917": { | |
| "pre": [0.5, 0.5, 0.5], | |
| "post": [1.2, 1.2, 1.2] | |
| }, | |
| "6.625": { | |
| "pre": [1.2, 1.2, 1.2], | |
| "post": [0, 0, 0] | |
| } | |
| } | |
| }, | |
| "glassparticle2": { | |
| "rotation": { | |
| "6.2917": [0, "(query.anim_time*360)", 0], | |
| "6.5417": [0, "(query.anim_time*360)", 0], | |
| "6.625": [0, 0, 0] | |
| "glassparticle1": { | |
| "rotation": { | |
| "6.2917": [0, 0, 0], | |
| "6.5417": [0, 0, 0], | |
| "6.625": [0, 0, 0] | |
| }, | |
| "position": { | |
| "6.25": [237, -25, 104], | |
| "6.3333": [247, -35, 66], | |
| "6.4167": [255, -51, 59], | |
| "6.5": [261, -80, 59], | |
| "6.5833": [261, -98, 53] | |
| }, | |
| "scale": { | |
| "0.0": [0, 0, 0], | |
| "6.2083": { | |
| "pre": [0, 0, 0], | |
| "post": [0, 0, 0] | |
| }, | |
| "6.25": { | |
| "pre": [0, 0, 0], | |
| "post": [0.5, 0.5, 0.5] | |
| }, | |
| "6.2917": { | |
| "pre": [0.5, 0.5, 0.5], | |
| "post": [1.2, 1.2, 1.2] | |
| }, | |
| "6.625": { | |
| "pre": [1.2, 1.2, 1.2], | |
| "post": [0, 0, 0] | |
| } | |
| } | |
| }, | |
| "glassparticle2": { | |
| "rotation": { | |
| "6.2917": [0, 0, 0], | |
| "6.5417": [0, 0, 0], | |
| "6.625": [0, 0, 0] |
🤖 Prompt for AI Agents
In animations/cloningMachine/containment_chamber.animation.json around lines
1485 to 1522 the rotation keyframes for glassparticle1 and glassparticle2
contain string query expressions like "(query.anim_time*240)" and
"(query.anim_time*360)" which are invalid in Bedrock animation keyframes;
replace those string expressions with concrete numeric rotation values (degrees
or radians as used elsewhere in the file) for each keyframe, or remove the
dynamic entries and implement the intended dynamic rotation via a script or
animation controller that updates the entity rotation at runtime instead of
embedding query expressions in the JSON.
| "animation.containment_chamber.empty": { | ||
| "loop": "hold_on_last_frame", | ||
| "bones": { | ||
| "floorwater": { | ||
| "scale": 0 | ||
| }, | ||
| "Door": { | ||
| "rotation": [0, 0, 0] | ||
| }, | ||
| "Door2": { | ||
| "scale": 0 | ||
| }, | ||
| "Door3": { | ||
| "scale": 0 | ||
| }, | ||
| "Door4": { | ||
| "scale": 0 | ||
| }, | ||
| "water": { | ||
| "scale": 0 | ||
| }, | ||
| "water2": { | ||
| "scale": 0 | ||
| }, | ||
| "water3": { | ||
| "scale": 0 | ||
| }, | ||
| "water4": { | ||
| "scale": 0 | ||
| }, | ||
| "water5": { | ||
| "scale": 0 | ||
| }, | ||
| "bubbles": { | ||
| "scale": 0 | ||
| }, | ||
| "ditto": { | ||
| "scale": 0 | ||
| }, | ||
| "drop3": { | ||
| "scale": 0 | ||
| }, | ||
| "waterparticles": { | ||
| "scale": 0 | ||
| }, | ||
| "glassparticles": { | ||
| "scale": 0 | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing animation_length in the "empty" animation.
The animation.containment_chamber.empty animation (line 1735) does not declare animation_length, while all other animations in this file do. This will likely cause the animation to fail validation or behave unpredictably at runtime.
Add animation_length after the "loop" property:
"animation.containment_chamber.empty": {
"loop": "hold_on_last_frame",
+ "animation_length": 1,
"bones": {(Use 0 if this is an instant/no-op state, or 1 for a single-frame hold.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "animation.containment_chamber.empty": { | |
| "loop": "hold_on_last_frame", | |
| "bones": { | |
| "floorwater": { | |
| "scale": 0 | |
| }, | |
| "Door": { | |
| "rotation": [0, 0, 0] | |
| }, | |
| "Door2": { | |
| "scale": 0 | |
| }, | |
| "Door3": { | |
| "scale": 0 | |
| }, | |
| "Door4": { | |
| "scale": 0 | |
| }, | |
| "water": { | |
| "scale": 0 | |
| }, | |
| "water2": { | |
| "scale": 0 | |
| }, | |
| "water3": { | |
| "scale": 0 | |
| }, | |
| "water4": { | |
| "scale": 0 | |
| }, | |
| "water5": { | |
| "scale": 0 | |
| }, | |
| "bubbles": { | |
| "scale": 0 | |
| }, | |
| "ditto": { | |
| "scale": 0 | |
| }, | |
| "drop3": { | |
| "scale": 0 | |
| }, | |
| "waterparticles": { | |
| "scale": 0 | |
| }, | |
| "glassparticles": { | |
| "scale": 0 | |
| } | |
| } | |
| } | |
| "animation.containment_chamber.empty": { | |
| "loop": "hold_on_last_frame", | |
| "animation_length": 1, | |
| "bones": { | |
| "floorwater": { | |
| "scale": 0 | |
| }, | |
| "Door": { | |
| "rotation": [0, 0, 0] | |
| }, | |
| "Door2": { | |
| "scale": 0 | |
| }, | |
| "Door3": { | |
| "scale": 0 | |
| }, | |
| "Door4": { | |
| "scale": 0 | |
| }, | |
| "water": { | |
| "scale": 0 | |
| }, | |
| "water2": { | |
| "scale": 0 | |
| }, | |
| "water3": { | |
| "scale": 0 | |
| }, | |
| "water4": { | |
| "scale": 0 | |
| }, | |
| "water5": { | |
| "scale": 0 | |
| }, | |
| "bubbles": { | |
| "scale": 0 | |
| }, | |
| "ditto": { | |
| "scale": 0 | |
| }, | |
| "drop3": { | |
| "scale": 0 | |
| }, | |
| "waterparticles": { | |
| "scale": 0 | |
| }, | |
| "glassparticles": { | |
| "scale": 0 | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In animations/cloningMachine/containment_chamber.animation.json around lines
1735 to 1784 the "animation.containment_chamber.empty" block is missing the
required "animation_length" property; add "animation_length" immediately after
the "loop" property and set it to 0 for an instant/no-op state (or 1 if you want
a single-frame hold) so the animation validates and behaves consistently with
the other animations.
| "down": { | ||
| "uv": [ | ||
| 56, | ||
| 513 | ||
| ], | ||
| "uv_size": [ | ||
| 3, | ||
| -5 | ||
| ] | ||
| } |
There was a problem hiding this comment.
Systemic issue: All "down" faces use negative UV heights—fix before merge.
This is the issue previously flagged on line 344 but appears systematically across nearly every bone in the file. Bedrock geometry treats negative uv_size dimensions as texture mirror/warp operations, causing rendering artifacts. The pattern affects ~27 cubes:
- bone5 (line 128):
"down": { "uv": [56, 513], "uv_size": [3, -5] } - bone6 (line 218):
"down": { "uv": [238, 550], "uv_size": [5, -3] } - bone8 (line 398):
"down": { "uv": [554, 424], "uv_size": [8, -3] } - …and continues through bone31 (line 2524)
All require correction. Change each down face's uv_size to use positive height and adjust the UV origin Y coordinate if the visual orientation must remain unchanged.
Representative fix for bone5:
"down": {
"uv": [
- 56,
+ 56,
513
],
"uv_size": [
3,
- -5
+ 5
]
}If the texture mapping visually changes, swap the UV origin Y to preserve appearance.
Also applies to: 211-220, 301-310, 391-400, 482-491, 572-581, 663-672, 753-762, 853-862, 953-962, 1043-1052, 1133-1142, 1224-1233, 1314-1323, 1404-1413, 1495-1504, 1585-1594, 1685-1694, 1785-1794, 1875-1884, 1965-1974, 2055-2064, 2146-2155, 2237-2246, 2328-2337, 2418-2426, 2518-2526
🤖 Prompt for AI Agents
In models/entity/cloningMachine/cooling_station_pipe.geo.json around lines
121-130 (and the other listed ranges), the "down" face uses a negative uv_size
height (e.g., "uv_size": [3, -5]); change the second value to its absolute
(positive) value and, to preserve current visual orientation, adjust the uv Y
origin by subtracting that positive height (e.g., new_uv_size = [w, h];
new_uv[1] = old_uv[1] - h). Apply this fix to every "down" face flagged (all
bones listed and the ranges: 211-220, 301-310, 391-400, 482-491, 572-581,
663-672, 753-762, 853-862, 953-962, 1043-1052, 1133-1142, 1224-1233, 1314-1323,
1404-1413, 1495-1504, 1585-1594, 1685-1694, 1785-1794, 1875-1884, 1965-1974,
2055-2064, 2146-2155, 2237-2246, 2328-2337, 2418-2426, 2518-2526).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
animations/cloningMachine/dna_scanner.animation.json (2)
4-465: Substantial duplication across idle, opening, and closing animations.The
idle_open(lines 4–234),idle_closed(lines 235–465),opening(lines 1038–1271), andclosing(lines 1272–1505) animations share nearly identical bone keyframe structures for green, green1–5, green2–4. The only material differences are inbone39rotation values. Consider whether this repetition could be reduced through shared keyframe data or animation layering if the engine supports it.
466-1024: Error animation contains highly repetitive keyframes; verify necessity.The
red2bone (lines 499–698) andred4bone (lines 846–1022) repeat alternating[0, 0, 0]and[0, 0, -1]position keyframes at 0.25-second intervals over ~11 seconds. This produces 44+ nearly identical keyframe entries. Similarly,red3(lines 736–815) follows a 0.5-second pattern. If this is intended to represent a rapid flickering effect, consider whether the animation length and interval precision can be simplified or if this level of granularity is necessary for the intended visual output.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
animations/cloningMachine/cooling_station.animation.json(1 hunks)animations/cloningMachine/dna_scanner.animation.json(1 hunks)models/entity/cloningMachine/cooling_station.geo.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- animations/cloningMachine/cooling_station.animation.json
- models/entity/cloningMachine/cooling_station.geo.json
🔇 Additional comments (2)
animations/cloningMachine/dna_scanner.animation.json (2)
235-465: Verify intent:idle_openandidle_closedhave identical bone keyframes.Both animations have identical position keyframes for green/green1–5 bones but differ only in
bone39rotation (idle_open:[0, -90, 0]at line 231 vs.idle_closed:[0, 0, 0]at line 462). If these are meant to represent meaningfully different states (open vs. closed), consider whether the green bone animations should also differ, or if the naming is misleading.
1025-1037: All referenced bones are properly defined in the model geometry.Verification confirms that all bones referenced in the animation file (
deadlylazer,green,green1–green5,red,red2–red4,red3, andbone39) are defined in./models/entity/cloningMachine/dna_scanner.geo.json. No animation binding errors will occur at runtime.
smell-of-curry
force-pushed
the
feature/mewtwo-update
branch
2 times, most recently
from
4066f06 to
64e1f50
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
entity/cloningMachine/cooling_station.json (2)
9-14: Fix texture reference — still pointing to wrong asset.This unresolved critical issue from the previous review persists: the
textures.defaultstill referencestextures/entity/containment_chamberinstead of the correcttextures/items/cooling_station. The geometry will render with the wrong texture atlas."textures": { - "default": "textures/entity/containment_chamber" + "default": "textures/items/cooling_station" },
16-23: Initializevariable.liquid_amountin pre_animation — still missing.This unresolved major issue from the previous review persists: the pre_animation block sets
potion_amountandhas_fluid, but never initializesvariable.liquid_amount. Animations that scalewater6with this variable will fail, and the fluid column won't appear during potion cycles."pre_animation": [ "variable.potion_amount = query.property('pokeb:potion_amount');", - "variable.has_fluid = query.property('pokeb:has_fluid');" + "variable.has_fluid = query.property('pokeb:has_fluid');", + "variable.liquid_amount = query.property('pokeb:liquid_amount');" ],animation_controllers/cloningMachine/battery_holder.json (1)
1-74: [DUPLICATE] Property initialization still missing—pokeb:batteries never set.As flagged in the previous review, the animation controller references
q.property('pokeb:batteries')in all transition conditions (lines 13, 16, 19, etc.), but this property is never initialized anywhere in the codebase. The entity will remain stuck in the "empty" state indefinitely since all transition conditions will fail to match.Ensure the
pokeb:batteriesproperty is initialized and updated by the behavior entity or script that manages the battery_holder. This is a blocker for the feature to function.animation_controllers/cloningMachine/containment_chamber_liquid.json (1)
6-886: [DUPLICATE] Missing fill_16 state—animation has no matching controller state.The accompanying animation file defines
water_fill16, but this controller only includes statesfill_0throughfill_15. Ifvariable.fluid_amountis set to 16, the controller will not transition to afill_16state, and the final animation will never play. This blocks full utilization of the animation set.Add a
fill_16state with"animations": ["water_fill16"]and update every existing state's transitions to include a branch for"fill_16": "variable.fluid_amount == 16"so the controller covers the full 0–16 range.animations/cloningMachine/containment_chamber.animation.json (2)
1485-1550: [DUPLICATE] Query expressions in rotation keyframes are invalid Bedrock animation format.Lines 1487–1488 and 1520–1521 embed string query expressions like
"(query.anim_time*240)"and"(query.anim_time*360)"directly in rotation arrays, but the Bedrock animation format requires numeric[x, y, z]values. These will fail to parse or animate at runtime.Replace query expressions with static numeric rotation values. If dynamic rotation is required, implement it via script or state-driven animation sequences instead of query expressions in keyframes.
Apply this diff:
"glassparticle1": { "rotation": { - "6.2917": [0, "(query.anim_time*240)", 0], - "6.5417": [0, "(query.anim_time*240)", 0], + "6.2917": [0, 0, 0], + "6.5417": [0, 0, 0], "6.625": [0, 0, 0] },"glassparticle2": { "rotation": { - "6.2917": [0, "(query.anim_time*360)", 0], - "6.5417": [0, "(query.anim_time*360)", 0], + "6.2917": [0, 0, 0], + "6.5417": [0, 0, 0], "6.625": [0, 0, 0] },
1735-1784: [DUPLICATE] Missing required animation_length in empty animation.The
animation.containment_chamber.emptyanimation declaresloop: "hold_on_last_frame"but omits theanimation_lengthproperty. All other animations in this file include it. This will cause validation failure or undefined runtime behavior.Add
animation_lengthimmediately after theloopproperty. Use0for an instant/no-op state or1for a single-frame hold:"animation.containment_chamber.empty": { "loop": "hold_on_last_frame", + "animation_length": 0, "bones": {
🧹 Nitpick comments (1)
animations/cloningMachine/dna_scanner.animation.json (1)
4-234: Significant code duplication across idle and transition animations.The
idle_open,idle_closed,opening, andclosinganimations share nearly identical bone keyframe structures. Each animation contains 5 "green" bones (green, green1–5) with identical temporal keyframe patterns (0.0, 0.5, 1.0, 1.5, 2.0, 2.5, 3.5, 4.0 seconds) and identical interpolation (pre/post) logic, differing only in Z-position values (oscillating between 0, ±1).For instance, the
greenbone position timeline spans lines 9–43 (idle_open), 240–274 (idle_closed), 1043–1077 (opening), and 1276–1310 (closing), with negligible variation.While JSON resource files have limited abstraction mechanisms in Bedrock, this duplication makes maintenance difficult. Any timing or interpolation logic corrections must be replicated across all four animations.
If tooling permits, consider generating these animations programmatically (e.g., via JSON templating or a build script) to reduce manual maintenance burden and ensure consistency. If not feasible, document the intentional mirroring in a
READMEor inline comment to clarify why the repetition exists.Also applies to: 235-465, 1038-1271, 1272-1505
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
animation_controllers/cloningMachine/battery_holder.json(1 hunks)animation_controllers/cloningMachine/battery_holder_pipe.json(1 hunks)animation_controllers/cloningMachine/containment_chamber.json(1 hunks)animation_controllers/cloningMachine/containment_chamber_liquid.json(1 hunks)animation_controllers/cloningMachine/dna_scanner.json(1 hunks)animations/cloningMachine/battery_holder.animation.json(1 hunks)animations/cloningMachine/battery_holder_pipe.animation.json(1 hunks)animations/cloningMachine/containment_chamber.animation.json(1 hunks)animations/cloningMachine/containment_chamber_liquid.animation.json(1 hunks)animations/cloningMachine/cooling_station.animation.json(1 hunks)animations/cloningMachine/dna_scanner.animation.json(1 hunks)animations/item_holder.animation.json(1 hunks)entity/cloningMachine/battery_holder.json(1 hunks)entity/cloningMachine/contained_mewtwo.json(1 hunks)entity/cloningMachine/containment_chamber.json(1 hunks)entity/cloningMachine/containment_chamber_liquid.json(1 hunks)entity/cloningMachine/cooling_station.json(1 hunks)entity/cloningMachine/dna_scanner.json(1 hunks)entity/cloningMachine/pipes/battery_holder_pipe.json(1 hunks)entity/cloningMachine/pipes/cooling_station_pipe.json(1 hunks)entity/cloningMachine/pipes/dna_scanner_pipe.json(1 hunks)entity/contained_mewtwo.json(1 hunks)entity/item_holder.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (14)
- animations/cloningMachine/battery_holder.animation.json
- animation_controllers/cloningMachine/battery_holder_pipe.json
- animation_controllers/cloningMachine/dna_scanner.json
- entity/cloningMachine/pipes/cooling_station_pipe.json
- entity/cloningMachine/battery_holder.json
- entity/cloningMachine/pipes/dna_scanner_pipe.json
- animations/cloningMachine/battery_holder_pipe.animation.json
- entity/cloningMachine/dna_scanner.json
- entity/item_holder.json
- entity/cloningMachine/pipes/battery_holder_pipe.json
- entity/contained_mewtwo.json
- animations/item_holder.animation.json
- entity/cloningMachine/contained_mewtwo.json
- animations/cloningMachine/cooling_station.animation.json
🔇 Additional comments (7)
animations/cloningMachine/dna_scanner.animation.json (3)
1-1520: Animation definitions and state transitions are well-structured.The JSON structure is valid and animations are logically sound:
- Loop and hold semantics are appropriate:
idle_*animations loop;opening,closing, anderrorusehold_on_last_frame.- Bone39 rotations correctly manage state transitions:
idle_open(−90°) ↔idle_closed(0°), withopeningandclosinganimating the transitions smoothly.- The
scanandshakeanimations are succinct and fit their intended purposes.- Timing is reasonable (12s error, 4s idle/transitions, 3s scan, 0.12s shake).
1025-1037: Verify thatscananimation timing aligns with intended gameplay.The
scananimation moves thedeadlylazerbone from[0, 0, 0]to[−18, 0, 0]over 3 seconds (lines 1030–1033). Confirm this timing and magnitude are intended and match gameplay expectations (e.g., duration of a DNA scan operation).
1498-1503: Verifybone39closure rotation transition.The
closinganimation interpolatesbone39rotation from[0, −90, 0](line 1500) to[0, 0, 0](line 1501) over 0.5 seconds. Confirm this brief transition is intentional and visually appropriate for the closure motion.animations/cloningMachine/containment_chamber_liquid.animation.json (1)
1-1438: Animation definitions are well-formed and complete.All 17 water fill animations (water_fill0–water_fill16) and the breaking animation are properly defined with animation_length, loop behavior, and bone keyframes. Each animation transitions smoothly between fill states with proper pre/post keyframe handling.
entity/cloningMachine/containment_chamber_liquid.json (1)
12-39: Entity structure is sound; animation mappings align with controller.The pre_animation script correctly queries
pokeb:fluid_amountand stores it invariable.fluid_amountfor controller evaluation. Animation mappings for water_fill0–water_fill15 are properly defined and wired to the controller states.Once the controller's fill_16 state is added, update this file's animations section to include
"water_fill16": "animation.containment_chamber_liquid.water_fill16"for completeness.Verify that
water_fill16animation mapping will be added when controller is updated to support fill_16 state.entity/cloningMachine/containment_chamber.json (1)
1-35: Entity configuration is well-structured.The containment chamber entity properly wires the animation controller and all state-driven animations (empty, filling, breaking, opening, opening_ditto, closing). Geometry, material, and texture references are all in place.
animation_controllers/cloningMachine/containment_chamber.json (1)
1-142: Controller state machine is well-designed and complete.Six states (empty, filling, breaking, opening, opening_ditto, closing) with full bidirectional transitions enable flexible state-driven animation control. Each transition is keyed to
q.property('pokeb:state')with appropriate string comparisons. All animation references align with the animation definitions.
Update models and animations add contained mewtwo Add contained mewtwo
smell-of-curry
force-pushed
the
feature/mewtwo-update
branch
from
64e1f50 to
fa6d0b7
Compare
Minecraft Model PreviewModel Changes
Animation Previews
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
animation_controllers/cloningMachine/containment_chamber.json (2)
4-140: 💤 Low valueNote: "idle" animation defined in entity but not used by this controller.
The entity file maps an "idle" animation key, but none of the six states in this controller reference it. If the animation is unused and not needed by the secondary
controller.animation.containment_chamber_dirtycontroller, consider removing it to reduce maintenance burden.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@animation_controllers/cloningMachine/containment_chamber.json` around lines 4 - 140, The controller.animation.containment_chamber states (empty, filling, breaking, opening, opening_ditto, closing) do not reference an "idle" animation that is defined in the entity file. Before removing it, verify that the idle animation is not used by the secondary controller.animation.containment_chamber_dirty controller or referenced elsewhere in the entity configuration. Once confirmed as unused, remove the idle animation definition from the entity file to reduce maintenance burden.
11-137: ⚡ Quick winConsider adding default state handling for robustness.
The state machine allows direct transitions between any two states based solely on
pokeb:stateproperty equality checks. If the server-side code setspokeb:stateto an unexpected value (typo, version mismatch, etc.), the entity will remain stuck in its current state with no visual feedback.Consider adding a fallback transition or a default "error" state to handle unexpected property values gracefully.
💡 Example fallback pattern
"empty": { "animations": ["empty"], "transitions": [ { "filling": "q.property('pokeb:state') == 'filling'" }, { "breaking": "q.property('pokeb:state') == 'breaking'" }, { "opening": "q.property('pokeb:state') == 'opening'" }, { "opening_ditto": "q.property('pokeb:state') == 'opening_ditto'" }, { "closing": "q.property('pokeb:state') == 'closing'" }, { "error": "q.property('pokeb:state') != 'empty'" } ] }Then add an "error" state that plays a visual indicator or logs the issue.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@animation_controllers/cloningMachine/containment_chamber.json` around lines 11 - 137, The state machine definition in the transitions array for each state (empty, filling, breaking, opening, opening_ditto, closing) lacks error handling for unexpected pokeb:state values. Add a fallback transition to each state that catches invalid or unexpected property values by adding a condition that evaluates to true when the pokeb:state does not match any of the expected valid values, and route those cases to a new "error" state. Then define the "error" state with appropriate animations or visual feedback to gracefully handle unexpected state values instead of leaving the entity stuck.animations/cloningMachine/dna_scanner.animation.json (1)
466-1024: ⚖️ Poor tradeoffConsider simplifying the error animation keyframes.
The error animation contains 558 lines with highly repetitive keyframe patterns. For example,
red2toggles between[0,0,0]and[0,0,-1]across 92 keyframes (lines 499-698). If the stepped blinking effect is essential, this verbosity is acceptable; however, if smoother interpolation or fewer steps would suffice, consolidating keyframes would improve maintainability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@animations/cloningMachine/dna_scanner.animation.json` around lines 466 - 1024, The animation.dna_scanner.error animation contains excessive repetitive keyframe definitions across its bones (red2, green, red3, red4, green1, green2, green3) with hundreds of nearly identical position toggles between states like [0,0,0] and [0,0,-1]. Review whether the stepped blinking effect requires all these individual keyframes or if the animation can achieve the same visual result with fewer, consolidated keyframes and smoother interpolation. Reduce the keyframe count for each bone by grouping similar transitions and removing redundant entries while preserving the intended animation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@animation_controllers/cloningMachine/containment_chamber.json`:
- Around line 7-28: The containment_chamber animation controller file references
animations named "empty", "filling", and "opening_ditto" in state transitions,
but these animation keys are not defined in the animations section of the
entity/cloningMachine/containment_chamber.json file. To fix this, add the
missing animation mappings to the animations object in the entity file by
including entries for "empty", "filling", and "opening_ditto", each pointing to
their corresponding animation definitions (following the pattern of existing
animation mappings like "animation.containment_chamber.closing"). Verify that
the referenced animation definitions exist in the
animations/cloningMachine/containment_chamber.animation.json file.
In `@animations/cloningMachine/dna_scanner.animation.json`:
- Around line 235-465: The animation.dna_scanner.idle_closed animation contains
duplicated position keyframes for all green bones (green, green1, green2,
green3, green4, green5) that are identical to those in the idle_open animation,
with the only difference being the bone39 rotation value. To reduce this
~230-line duplication, explore extracting the shared position keyframes into a
separate base animation or use animation blending to separate the positional
blinking pattern from the rotation state, allowing both idle_open and
idle_closed to reference the same blinking animation while only differing in
their bone39 rotation configuration.
- Around line 1038-1505: The position keyframes for bones green, green1, green2,
green3, green4, and green5 are identical between animation.dna_scanner.opening
and animation.dna_scanner.closing, and this same pattern is repeated across
idle_open and idle_closed animations, resulting in approximately 690 lines of
duplication. Only the bone39 rotation keyframes differ between the opening and
closing states. Refactor the animation structure to consolidate these shared
position timelines into a reusable pattern or extract them into a separate
shared animation reference if the Bedrock animation system supports it, while
keeping only the unique bone39 rotation keyframes separate for each state. If
duplication is unavoidable due to Bedrock constraints, document the reason
clearly in comments.
---
Nitpick comments:
In `@animation_controllers/cloningMachine/containment_chamber.json`:
- Around line 4-140: The controller.animation.containment_chamber states (empty,
filling, breaking, opening, opening_ditto, closing) do not reference an "idle"
animation that is defined in the entity file. Before removing it, verify that
the idle animation is not used by the secondary
controller.animation.containment_chamber_dirty controller or referenced
elsewhere in the entity configuration. Once confirmed as unused, remove the idle
animation definition from the entity file to reduce maintenance burden.
- Around line 11-137: The state machine definition in the transitions array for
each state (empty, filling, breaking, opening, opening_ditto, closing) lacks
error handling for unexpected pokeb:state values. Add a fallback transition to
each state that catches invalid or unexpected property values by adding a
condition that evaluates to true when the pokeb:state does not match any of the
expected valid values, and route those cases to a new "error" state. Then define
the "error" state with appropriate animations or visual feedback to gracefully
handle unexpected state values instead of leaving the entity stuck.
In `@animations/cloningMachine/dna_scanner.animation.json`:
- Around line 466-1024: The animation.dna_scanner.error animation contains
excessive repetitive keyframe definitions across its bones (red2, green, red3,
red4, green1, green2, green3) with hundreds of nearly identical position toggles
between states like [0,0,0] and [0,0,-1]. Review whether the stepped blinking
effect requires all these individual keyframes or if the animation can achieve
the same visual result with fewer, consolidated keyframes and smoother
interpolation. Reduce the keyframe count for each bone by grouping similar
transitions and removing redundant entries while preserving the intended
animation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8b5814e-0e9b-465a-837e-03b207100335
📒 Files selected for processing (34)
animation_controllers/cloningMachine/battery_holder.jsonanimation_controllers/cloningMachine/battery_holder_pipe.jsonanimation_controllers/cloningMachine/containment_chamber.jsonanimation_controllers/cloningMachine/containment_chamber_liquid.jsonanimation_controllers/cloningMachine/dna_scanner.jsonanimations/cloningMachine/battery_holder.animation.jsonanimations/cloningMachine/battery_holder_pipe.animation.jsonanimations/cloningMachine/containment_chamber.animation.jsonanimations/cloningMachine/containment_chamber_liquid.animation.jsonanimations/cloningMachine/cooling_station.animation.jsonanimations/cloningMachine/dna_scanner.animation.jsonanimations/item_holder.animation.jsonanimations/pokemon/mewtwo.animation.jsonentity/cloningMachine/battery_holder.jsonentity/cloningMachine/contained_mewtwo.jsonentity/cloningMachine/containment_chamber.jsonentity/cloningMachine/containment_chamber_liquid.jsonentity/cloningMachine/cooling_station.jsonentity/cloningMachine/dna_scanner.jsonentity/cloningMachine/pipes/battery_holder_pipe.jsonentity/cloningMachine/pipes/cooling_station_pipe.jsonentity/cloningMachine/pipes/dna_scanner_pipe.jsonentity/contained_mewtwo.jsonentity/item_holder.jsonentity/trainer.jsonmodels/entity/cloningMachine/battery_holder.geo.jsonmodels/entity/cloningMachine/battery_holder_pipe.geo.jsonmodels/entity/cloningMachine/containment_chamber.geo.jsonmodels/entity/cloningMachine/containment_chamber_liquid.geo.jsonmodels/entity/cloningMachine/cooling_station.geo.jsonmodels/entity/cloningMachine/cooling_station_pipe.geo.jsonmodels/entity/cloningMachine/dna_scanner.geo.jsonmodels/entity/cloningMachine/dna_scanner_pipe.geo.jsonmodels/entity/item_holder.geo.json
💤 Files with no reviewable changes (19)
- entity/cloningMachine/battery_holder.json
- entity/cloningMachine/cooling_station.json
- entity/cloningMachine/pipes/cooling_station_pipe.json
- entity/cloningMachine/contained_mewtwo.json
- entity/cloningMachine/containment_chamber_liquid.json
- entity/cloningMachine/pipes/battery_holder_pipe.json
- entity/contained_mewtwo.json
- entity/cloningMachine/pipes/dna_scanner_pipe.json
- entity/cloningMachine/containment_chamber.json
- models/entity/cloningMachine/dna_scanner_pipe.geo.json
- entity/trainer.json
- models/entity/cloningMachine/battery_holder_pipe.geo.json
- entity/cloningMachine/dna_scanner.json
- models/entity/cloningMachine/containment_chamber_liquid.geo.json
- models/entity/item_holder.geo.json
- models/entity/cloningMachine/cooling_station.geo.json
- models/entity/cloningMachine/cooling_station_pipe.geo.json
- models/entity/cloningMachine/battery_holder.geo.json
- entity/item_holder.json
🚧 Files skipped from review as they are similar to previous changes (10)
- animation_controllers/cloningMachine/battery_holder_pipe.json
- animation_controllers/cloningMachine/dna_scanner.json
- animation_controllers/cloningMachine/containment_chamber_liquid.json
- animations/cloningMachine/battery_holder_pipe.animation.json
- animations/cloningMachine/battery_holder.animation.json
- animations/item_holder.animation.json
- animations/cloningMachine/containment_chamber.animation.json
- animation_controllers/cloningMachine/battery_holder.json
- animations/cloningMachine/containment_chamber_liquid.animation.json
- animations/cloningMachine/cooling_station.animation.json
| "empty": { | ||
| "animations": [ | ||
| "empty" | ||
| ], | ||
| "transitions": [ | ||
| { | ||
| "filling": "q.property('pokeb:state') == 'filling'" | ||
| }, | ||
| { | ||
| "breaking": "q.property('pokeb:state') == 'breaking'" | ||
| }, | ||
| { | ||
| "opening": "q.property('pokeb:state') == 'opening'" | ||
| }, | ||
| { | ||
| "opening_ditto": "q.property('pokeb:state') == 'opening_ditto'" | ||
| }, | ||
| { | ||
| "closing": "q.property('pokeb:state') == 'closing'" | ||
| } | ||
| ] | ||
| }, |
There was a problem hiding this comment.
Critical: Animation "empty" referenced but not mapped in entity file.
The "empty" state references an animation named "empty" (line 9), but the entity file (entity/cloningMachine/containment_chamber.json) does not define this animation key in its animations section. The entity only maps: "closing", "opening", "idle", "breaking", and "water_fill*" animations.
The animation system will fail to resolve "empty", likely causing runtime warnings or falling back to default behavior. This same issue affects "filling" (line 31) and "opening_ditto" (line 97).
🔧 Required fix in entity file
Add missing animation mappings to entity/cloningMachine/containment_chamber.json:
"animations": {
"controller": "controller.animation.containment_chamber",
"empty": "animation.containment_chamber.empty",
"filling": "animation.containment_chamber.filling",
"opening_ditto": "animation.containment_chamber.opening_ditto",
"closing": "animation.containment_chamber.closing",
"opening": "animation.containment_chamber.opening",
"breaking": "animation.containment_chamber.breaking"
}Ensure the corresponding animation definitions exist in animations/cloningMachine/containment_chamber.animation.json.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@animation_controllers/cloningMachine/containment_chamber.json` around lines 7
- 28, The containment_chamber animation controller file references animations
named "empty", "filling", and "opening_ditto" in state transitions, but these
animation keys are not defined in the animations section of the
entity/cloningMachine/containment_chamber.json file. To fix this, add the
missing animation mappings to the animations object in the entity file by
including entries for "empty", "filling", and "opening_ditto", each pointing to
their corresponding animation definitions (following the pattern of existing
animation mappings like "animation.containment_chamber.closing"). Verify that
the referenced animation definitions exist in the
animations/cloningMachine/containment_chamber.animation.json file.
| "animation.dna_scanner.idle_closed": { | ||
| "loop": true, | ||
| "animation_length": 4, | ||
| "bones": { | ||
| "green": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green3": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green1": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green2": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green4": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green5": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "bone39": { | ||
| "rotation": [0, 0, 0] | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Consider extracting the shared position keyframes to reduce duplication.
The idle_closed animation duplicates all position keyframes from idle_open (lines 4-234), differing only in bone39 rotation. This ~230-line duplication creates a maintainability burden—any change to the blinking pattern must be synchronized across both animations.
While Bedrock's animation format doesn't directly support inheritance, consider whether the two states could share a base animation or whether animation blending could separate the positional blinking from the rotation state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@animations/cloningMachine/dna_scanner.animation.json` around lines 235 - 465,
The animation.dna_scanner.idle_closed animation contains duplicated position
keyframes for all green bones (green, green1, green2, green3, green4, green5)
that are identical to those in the idle_open animation, with the only difference
being the bone39 rotation value. To reduce this ~230-line duplication, explore
extracting the shared position keyframes into a separate base animation or use
animation blending to separate the positional blinking pattern from the rotation
state, allowing both idle_open and idle_closed to reference the same blinking
animation while only differing in their bone39 rotation configuration.
| "animation.dna_scanner.opening": { | ||
| "loop": "hold_on_last_frame", | ||
| "animation_length": 4, | ||
| "bones": { | ||
| "green": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green3": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green1": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green2": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green4": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green5": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "bone39": { | ||
| "rotation": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": [0, -90, 0] | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "animation.dna_scanner.closing": { | ||
| "loop": "hold_on_last_frame", | ||
| "animation_length": 4, | ||
| "bones": { | ||
| "green": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green3": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green1": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green2": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green4": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, 1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, 1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "green5": { | ||
| "position": { | ||
| "0.0": [0, 0, 0], | ||
| "0.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "1.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "1.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "2.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "2.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "3.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| }, | ||
| "3.5": { | ||
| "pre": [0, 0, 0], | ||
| "post": [0, 0, -1] | ||
| }, | ||
| "4.0": { | ||
| "pre": [0, 0, -1], | ||
| "post": [0, 0, 0] | ||
| } | ||
| } | ||
| }, | ||
| "bone39": { | ||
| "rotation": { | ||
| "0.0": [0, -90, 0], | ||
| "0.5": [0, 0, 0] | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Significant duplication: opening and closing animations repeat shared keyframes.
The opening (lines 1038-1271) and closing (lines 1272-1505) animations duplicate the same ~230 lines of position keyframes already present in idle_open and idle_closed. Across all four animations, these identical position timelines are repeated, totaling ~690 lines of duplication. Only the bone39 rotation keyframes differ between states.
This duplication severely impacts maintainability—updating the LED blinking pattern would require changes in four locations. Consider refactoring to share the position timelines or document why the duplication is necessary given Bedrock's animation system constraints.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@animations/cloningMachine/dna_scanner.animation.json` around lines 1038 -
1505, The position keyframes for bones green, green1, green2, green3, green4,
and green5 are identical between animation.dna_scanner.opening and
animation.dna_scanner.closing, and this same pattern is repeated across
idle_open and idle_closed animations, resulting in approximately 690 lines of
duplication. Only the bone39 rotation keyframes differ between the opening and
closing states. Refactor the animation structure to consolidate these shared
position timelines into a reusable pattern or extract them into a separate
shared animation reference if the Bedrock animation system supports it, while
keeping only the unique bone39 rotation keyframes separate for each state. If
duplication is unavoidable due to Bedrock constraints, document the reason
clearly in comments.
2 participants
Summary by CodeRabbit
Release Notes
New Features
Improvements
Note
Adds cloning machine client entities and geometry (containment chamber/liquid, DNA scanner, cooling station, battery holder, pipes), a contained Mewtwo, item_holder idle/attachables, new disinfect particle, and item texture mappings; updates Mewtwo animations.
pokeb:containment_chamber,pokeb:containment_chamber_liquid,pokeb:dna_scanner,pokeb:cooling_station,pokeb:battery_holder, and related pipe entities.pokeb:contained_mewtwoclient entity (duplicate entry under cloningMachine/ and root).models/entity/cloningMachine/*.geo.jsonfor all components and pipes.animation.item_holder.idleand hook via scripts withpokeb:scale; enable attachables; update visible bounds in geometry.animations/pokemon/mewtwo.animation.json, expanding animations and addinganimation.mewtwo.rest.variable.skin_index = query.property('pokeb:skin_index');to pre_animation.particles/disinfect.particle.jsoneffect.textures/item_texture.jsonwith mappings forcontainment_fluid, cloning machine items,wrench, andmew_eyelashvariants.Written by Cursor Bugbot for commit 4066f06. This will update automatically on new commits. Configure here.