Issue #306 Split Non-color Variants#307
Conversation
✅ Deploy Preview for liberatedpixelcup ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Splits several non-color-related variants (belt “other”, shields, and wounds) into separate, dedicated assets/definitions to improve credit lookups and reduce errors during upcoming credits-generation refactors.
Changes:
- Introduces new standalone sheet definitions for round shield and multiple belt subtypes; removes aggregated “shield” and “other belts” definitions.
- Refactors wound definitions to use per-wound directories and a concrete
"red"variant, updating metadata and credit entries accordingly. - Updates z-position mapping and
item-metadata.jscategory/tree entries to reflect the new item split.
Reviewed changes
Copilot reviewed 20 out of 360 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sheet_definitions/weapons/shields/shield_spartan.json | Normalizes Spartan shield naming/paths and adds teen layer support. |
| sheet_definitions/weapons/shields/shield_round.json | Adds a dedicated “Round Shield” definition with its own credits/path. |
| sheet_definitions/weapons/shields/shield.json | Removes the old combined shield definition that bundled multiple shield types/variants. |
| sheet_definitions/torso/waist/belt_robe.json | Adds a dedicated robe belt definition split from “other belts”. |
| sheet_definitions/torso/waist/belt_other_male.json | Removes old aggregated male “other belts” definition. |
| sheet_definitions/torso/waist/belt_other_female.json | Removes old aggregated female “other belts” definition. |
| sheet_definitions/torso/waist/belt_mage.json | Adds dedicated mage belt definition with animations and credits. |
| sheet_definitions/torso/waist/belt_loose.json | Fixes teen path mapping for loose belt. |
| sheet_definitions/torso/waist/belt_leather2.json | Adds dedicated alternate leather belt definition. |
| sheet_definitions/torso/waist/belt_leather.json | Fixes teen path mapping for leather belt. |
| sheet_definitions/torso/waist/belt_formal.json | Adds dedicated formal belt definition. |
| sheet_definitions/body/wounds/wound_ribs.json | Moves ribs wound to its own directory + adds priority and “red” variant. |
| sheet_definitions/body/wounds/wound_mouth.json | Moves mouth wound to its own directory + adds priority and “red” variant. |
| sheet_definitions/body/wounds/wound_eye_right.json | Moves right-eye wound to its own directory + adds priority and “red” variant. |
| sheet_definitions/body/wounds/wound_eye_left.json | Moves left-eye wound to its own directory + adds priority and “red” variant. |
| sheet_definitions/body/wounds/wound_brain.json | Moves brain wound to its own directory + adds priority and “red” variant. |
| sheet_definitions/body/wounds/wound_arm.json | Moves arm wound to its own directory + adds priority and “red” variant. |
| scripts/zPositioning/z_positions.csv | Updates/extends z-position mappings for newly split items and new wound directories. |
| item-metadata.js | Updates generated metadata/category entries to match the newly split definitions. |
| CREDITS.csv | Re-keys credit rows to match new asset paths and newly split item files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ], | ||
| "path": [ | ||
| "weapons", | ||
| "shield", |
There was a problem hiding this comment.
The path segment uses "shield" (singular) here, but the rest of this PR places shields under "shields" (plural) (e.g. item-metadata.js uses ["weapons","shields","shield_round"], and the file lives under sheet_definitions/weapons/shields/). This mismatch is likely to break lookups/grouping for the round shield. Update the path array to use "shields" for consistency with the directory/category structure.
| "shield", | |
| "shields", |
|
Do you think we should do any mitigations for old imports or old urls that will no longer work? |
I'm not really sure what we can do here. If we want paint in place to work we cannot have "variants" that are completely different assets... That said, the shields and wounds shouldn't be a problem anyway. The only actual change is Shield > Round Shield. Crusader Shield and Spartan Shield were already separate assets and Wound was already separate anyway. Belt Other I doubt is something that a lot of people have bookmarked. |
|
I'm going to move the question to discussions and approve this. |
|
Since this is approved, I'll merge it, but I'll also be whipping up a theorycraft update to support forwarding. |
Resolves #306
Split off several variants that aren't related to colors:
These are separate assets to make credit searching easier. I'm about to refactor generating the credits to not rely on variants and enable the ability to split by animation, as currently the paths credits are checking do not work. These assets were throwing errors when I did a trial run of that fix.
This is a pre-requisite for #284.