Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for Prerequisite Assets...? (or Toggle Asset Directory if Another Asset Enabled) #129

Open
jrconway3 opened this issue Jan 27, 2025 · 27 comments
Labels
enhancement New feature or request

Comments

@jrconway3
Copy link
Contributor

I'm not exactly sure how to do this, but there's a functionality I've been interested in for a while now that may highly benefit overlay assets.

This is in relation to multiple issues:

Its especially important for #102 because I'm actively working on this right now.

Here's the problem I'm seeing:

Eyes overlays only work for specific heads. Of course its all the standard human heads, so that covers a LOT of things. However, what happens when we add Expressions in?

Obviously, the eyes just won't work anymore. We need a new overlay for the eyes in the new positions.

This is pretty easy to do. Once I map the expressions to all heads, I can create a duplicate asset and remove the skintone so we just have the eyes left. Then we can have an eye color swap overlay.

However, we would need to manually toggle the swap PER asset... or perhaps we can add the ability to change the directory paths if specific assets are enabled? Another option: have a default required asset, and if that asset is turned off, the asset goes away.

This second option will work well for many overlay assets, such as what I described in #42:
When we toggle between different hats, the trims just don't work anymore. What I would want to do in such cases is, if the root asset is changed, we remove the trim recolor.

I'm bringing this up here because I'm not exactly sure how to do this, and it will likely become important pretty soon. Here's an example of a toggle in the code for something similar:

  "match_body_color": true,

Match body color ensures that whatever asset is added always matches the same variant as the body, so it forcibly changes the body color. This is an example of a prerequisite, but it only does one thing--change the body color.

@Gaurav0
Copy link
Member

Gaurav0 commented Jan 28, 2025

Right now, the body matching algorithm unfortunately uses data-required="male,female" attribute.

I wish I had made it data-required="body=male,female" or something.

Then we could have expanded it to accept data-required="body=male,female&head=male,female"

@jrconway3
Copy link
Contributor Author

Wow so that one dates back to you? Interesting. So this mechanic wasn't updated by Sanderfrenken.

@Gaurav0
Copy link
Member

Gaurav0 commented Jan 28, 2025

However, we would need to manually toggle the swap PER asset... or perhaps we can add the ability to change the directory paths if specific assets are enabled? Another option: have a default required asset, and if that asset is turned off, the asset goes away.

  • I really want the directory paths to be set in the sheet definitions as much as possible. We can use a JSON object map.
  • The second I definitely want to do, regardless of whether we do the first.

I'm thinking now I should:

  1. rename data-required to data-sex-required
  2. add special code for a data-head-required This is because head type isn't its own key right now.
  3. add more general code for data-required that allows specifying any key=<csv list>
  4. generalize the general code to allow multiple keys

@jrconway3
Copy link
Contributor Author

I really want the directory paths to be set in the sheet definitions as much as possible. We can use a JSON object map.

I wasn't advocating for changing this. What I meant is that in the sheet definition itself, we add a toggle that says if another asset is enabled, we swap the directory.

For example, eyes:

{
  "name": "Default Eyes",
  "type_name": "eyes",
  "layer_1": {
    "zPos": 105,
    "male": "eyes/human/adult/default/",
    "muscular": "eyes/human/adult/default/",
    "female": "eyes/human/adult/default/",
    "teen": "eyes/human/adult/default/",
    "pregnant": "eyes/human/adult/default/"
  },

Something like this perhaps:

{
  "name": "Default Eyes",
  "type_name": "eyes",
  "layer_1": {
    "zPos": 105,
    "required": {
         "head": ["XXXX", "XXXX"]
    },
    "male": "eyes/human/adult/default/",
    "muscular": "eyes/human/adult/default/",
    "female": "eyes/human/adult/default/",
    "teen": "eyes/human/adult/default/",
    "pregnant": "eyes/human/adult/default/"
  },

And that layer won't load if the "head" isn't set.

This is just a rough example. Another possibility could be having the ability to add %head% into the path, or perhaps in this case, the expression:

    "male": "eyes/human/adult/%expression%/",

@jrconway3
Copy link
Contributor Author

If the asset type doesn't exist it would throw an error on compile.

@Gaurav0
Copy link
Member

Gaurav0 commented Jan 28, 2025

It's hard to know what you mean by "head". Right now head type is an abstraction, for example, the actual key value is head=Human_female_light

@jrconway3
Copy link
Contributor Author

It would be Human_female. light is the variant.

The main issue is that facial expressions are only designed for human heads. But there's a large variety of non-human heads, too. Those heads probably shouldn't accept the facial expressions we have.

@BenCreating
Copy link

I'd recommend against using a required property. If a new (compatible) head was added, you'd have to update the definition file for every expression. If it's compatible already, you should be able to just drop it in without editing unrelated files.

I implemented this in my generator with the tags and excluded-by properties in the spritesheet definition. With that system a head asset would be given the tag human or animal, and the expression asset would list animal in the exclusions. In most cases, you only need to edit the definition for the asset you're adding.

Admittedly, with my system you would need to update the expression definitions if you added a new incompatible (non-animal) head type, but I'm assuming whoever creates/adds that asset will be more aware that additional changes need to be made to prevent incorrect spritesheets.

@jrconway3
Copy link
Contributor Author

Yeah that's probably a good point. Right now we already are using required in some instances, but its not for much. There's only two things it is actively used for: body type (which are RARELY added) and animations, and even then, its mostly used in generate_source.html.

That being said, there may be limited situations where required makes sense. I'm talking about, say, a "trim" overlay for a single specific asset. In that circumstance, required makes the most sense.

@Gaurav0
Copy link
Member

Gaurav0 commented Jan 29, 2025

I'd recommend against using a required property. If a new (compatible) head was added, you'd have to update the definition file for every expression. If it's compatible already, you should be able to just drop it in without editing unrelated files.

I implemented this in my generator with the tags and excluded-by properties in the spritesheet definition. With that system a head asset would be given the tag human or animal, and the expression asset would list animal in the exclusions. In most cases, you only need to edit the definition for the asset you're adding.

Admittedly, with my system you would need to update the expression definitions if you added a new incompatible (non-animal) head type, but I'm assuming whoever creates/adds that asset will be more aware that additional changes need to be made to prevent incorrect spritesheets.

Let me understand. tags is a set represented by a javascript array. excluded by is as well. If the intersection of tags and excluded by from asset A and asset B is not the empty set, A and B are not compatible? And if the intersection is not the empty set, remove asset containing the excluded by from the list presented by the generator, not the one containing the tag. Sounds really simple. Maybe too simple.

But we currently are able to mix and match some things like a female head to a male body. Or an adult head to a child body. So I guess we have to be careful and use tags like male_head and child_body, not just male or child.

Also there will be times like an an asset might be designed for a single hairstyle, and right now we have so many hairstyles. Are we really going to list every hairstyle except one in an excluded by set?

Finally, are we sure tags should just be strings? We might run into problems with synonyms. Does 'hose' mean the legwear or the tube socks? We might want to use key:tag pairs.

@jrconway3
Copy link
Contributor Author

Also there will be times like an an asset might be designed for a single hairstyle, and right now we have so many hairstyles. Are we really going to list every hairstyle except one in an excluded by set?

Yeah that would be a PITA and wouldn't be worth it. That being said, I don't see any assets we'd need to have an "excluded" set for anyway. I mean, at most, I guess, perhaps you might say certain heads shouldn't have it, but I don't think that's worth it really. If anything, I think its fine to keep hair on everything.

The only types of assets we want to restrict are assets that clearly need to be laid over another asset. The faces and eyes obviously. Some hairstyles, perhaps, should have restrictions on certain headpieces. There's some hairstyles that have hairbands or other such things in the hair, and IMO those should have overlays so they can get recolors... and those overlays should be restricted to the hairstyles they're based on.

Similarly, some hats may have restricted assets, too. Again, mostly overlays.

This is why I think we should have both required AND excludes. Depending on the asset, either-or may be good options.

Finally, are we sure tags should just be strings? We might run into problems with synonyms. Does 'hose' mean the legwear or the tube socks? We might want to use key:tag pairs.

There are no hose socks to my knowledge. Those are "Long Socks". But I get your point. I agree that it should be a key-value pair of some kind.

However, I think the use of tags for "related" assets can be a viable option. We can label certain heads as "animal" or "human" (or perhaps "humanoid", as some non-human heads may still work with most human assets). But that could still be a key-value pair thing, in that case, we say "required-tag" or "excludes-tag". But we could also have a general required/excludes that lets you set any asset type you want.

Its up to you how expansive or restrictive you want to go with it. Mostly we just want some ways to restrict certain assets in relation to other assets in a way that is more varied than just the body colors.

@sanderfrenken
Copy link
Contributor

sanderfrenken commented Jan 29, 2025

Interesting discussion. Something to add to the equation:

I understand that from an asset designer's point of view, certain assets, like a visor, are intended to be used in combination solely with certain helmets. The same goes for certain trims, epaulets, mantal shoulders and now also the eyes and expressions as an example.

From a character designer's perspective though, there can be quite interesting combinations possible that you as a creator of the asset might not even had think of when creating the asset in the first place.

For instance, a sack hood with a visor creates a resulting character that could suit for a fencing look a like:

https://liberatedpixelcup.github.io/Universal-LPC-Spritesheet-Character-Generator/#?body=Body_color_light&head=Human_male_light&hat=Hood_white&visor=Narrow_grated_visor_iron

A minotaur with a cyclops eye works as well from some perspectives:

https://liberatedpixelcup.github.io/Universal-LPC-Spritesheet-Character-Generator/#?body=Body_color_fur_white&head=Minotaur_fur_white&eyes=Cyclops_Eyes_cyclops

And with some imagination: Combining these three trims (two that are meant for a pirate jacket, and one for the Santa's hat) on a nude body can yield some native tribe like character look with a tattoo on his body wearing a headband.

https://liberatedpixelcup.github.io/Universal-LPC-Spritesheet-Character-Generator/#?body=Body_color_olive&head=Human_male_olive&jacket_trim=Frock_coat_lace_red&jacket_pockets=none_Jacket+pockets&jacket_collar=Frock_collar_red&hat_trim=Santa_Trim_red&bracers=Bracers_gold

There are so many ways to combine assets, and I think there can be very valuable combinations possible using assets that you wouldn't expect to be compatible at first.

You could add a more "restricted mode" to the generator, but I would propose to make it optional if you do.

@Gaurav0 Gaurav0 added the enhancement New feature or request label Jan 29, 2025
@Gaurav0
Copy link
Member

Gaurav0 commented Jan 29, 2025

"match_body_color": true,

@jrconway3 Coming back to this proposal...

I'm thinking we should rename match_body_color to match_head_variant since it is more accurate. Is this going to be a special case for expressions or is it more general?

So for your expressions, the required/directory swap proposal:

Copying from: https://github.com/jrconway3/Universal-LPC-Spritesheet-Character-Generator/blob/409f1799aaf8f6d92adf02d1d54f67f736e3cd53/sheet_definitions/face_elderly_anger.json

{
  "name": "Angry",
  "type_name": "expression",
  "template": {
    "head": {
      "human_male": "human",
      "human_female": "human",
      "elderly_male": "elderly",
      "elderly_female": "elderly",
      etc.
    }
  },
  "layer_1": {
    "zPos": 101,
    "male": "head/faces/${head}/anger/",
    "muscular": "head/faces/${head}/anger/",
    "female": "head/faces/${head}/anger/",
    "teen": "head/faces/${head}/anger/",
    "pregnant": "head/faces/${head}/anger/"
  },
  "match_head_variant": true

I'm renaming required to template because this required is sort of implied and the substitution is more important here. "head" matches the name of the template variable to be substituted. the key in the head object will be the head value without the variant. The value will be the directory to choose.

@sanderfrenken
Copy link
Contributor

I'm thinking we should rename match_body_color to match_head_variant since it is more accurate.

Not saying the current name is great, but match_head_variant is less accurate imo. The mechanism this boolean works on goes in multiple directions. For instance, if you select a blue head, the body (and eg wings if applicable) will turn blue as well. If you then select a red wing variant (the lizard and bat wings support this boolean as well), the head and the body will also turn red (obviously only if those assets are indeed available in that variant).

So it's not only matching on the head variant, it is just matching the variant for all assets that support this boolean when for any of those assets a new variant is selected.

@Gaurav0
Copy link
Member

Gaurav0 commented Jan 29, 2025

For instance, if you select a blue head, the body (and eg wings if applicable) will turn blue as well.

I thought we wanted to keep the option of mismatched variants for head and body?

If we don't, I suppose match_body_variant will do.

@sanderfrenken How about a "match body" option on head?

@jrconway3 Are we sure we can't use (partially) transparent pixels for the expressions matching the face color?

@sanderfrenken
Copy link
Contributor

I thought we wanted to keep the option of mismatched variants for head and body?

This is possible, as you can toggle this feature off. But I see the checkbox to disable/ enable this feature is removed...
Let me check when that happened..

@sanderfrenken
Copy link
Contributor

Weird. The JS is still there:

$("#match_body-color").click(function () {

But the checkbox itself is removed from the HTML. Well you can easily add that back in.

@sanderfrenken How about a "match body" option on head?

What do you mean by that? in replacement for match_body_color?

@BenCreating
Copy link

Let me understand. tags is a set represented by a javascript array. excluded by is as well. If the intersection of tags and excluded by from asset A and asset B is not the empty set, A and B are not compatible? And if the intersection is not the empty set, remove asset containing the excluded by from the list presented by the generator, not the one containing the tag. Sounds really simple.

Yes, that's how it works.

I only add tags to assets that will restrict others. The body, the head, etc. I wouldn't bother adding any tags to hair unless I knew there were sub-assets that don't fit the other hairstyles. For example, in my generator, the human male body is tagged ["male", "adult"]. The human male head and the human female head have no tags, but the exclusions are ["child"].

I think this approach avoids the need for more specific key:value tags, though you might need those for color matching purposes. Mine live-recolors the images, so I handle color matching differently.

@Gaurav0
Copy link
Member

Gaurav0 commented Jan 29, 2025

A search git log -S match_body-color came up empty.

@Gaurav0
Copy link
Member

Gaurav0 commented Jan 29, 2025

#137

@sanderfrenken
Copy link
Contributor

Tried to find it back in history, can't find it either. I think it might have been lost years ago. Thanks for putting it back anyway!

@Gaurav0
Copy link
Member

Gaurav0 commented Jan 29, 2025

Hey, I thought it was a new proposal.

@sanderfrenken
Copy link
Contributor

I might have removed it by mistake when I introduced the source_index.html. 🤦

@Gaurav0
Copy link
Member

Gaurav0 commented Jan 29, 2025

@jrconway3 Is there any branch I can use to start working on this (doesn't matter if the PR is closed) for our required/template proposal?

{
  "name": "Angry",
  "type_name": "expression",
  "template": {
    "head": {
      "human_male": "human",
      "human_female": "human",
      "elderly_male": "elderly",
      "elderly_female": "elderly",
      etc.
    }
  },
  "layer_1": {
    "zPos": 101,
    "male": "head/faces/${head}/anger/",
    "muscular": "head/faces/${head}/anger/",
    "female": "head/faces/${head}/anger/",
    "teen": "head/faces/${head}/anger/",
    "pregnant": "head/faces/${head}/anger/"
  },
  "match_body_color": true

@jrconway3
Copy link
Contributor Author

Here:
https://github.com/jrconway3/Universal-LPC-Spritesheet-Character-Generator/tree/issue-102-sheet-definitions-only

I uploaded just the sheet definitions, only one sheet per face. I also kept the adult/child split for eyes, but only one eyes (eyes_adult). My only real concern with child_eyes is that child eyes has less animations available for its bodies, but I can always re-merge these back anyway.

@jrconway3
Copy link
Contributor Author

For instance, if you select a blue head, the body (and eg wings if applicable) will turn blue as well.

I thought we wanted to keep the option of mismatched variants for head and body?

If we can support mismatched variants... well, I actually added some variants of the same assets that had both a matched variant AND a mismatched variant. I did this for Lizard Tail and Lizard Wings. If we have the toggle in place, then we don't need to have two sets.

@jrconway3 Are we sure we can't use (partially) transparent pixels for the expressions matching the face color?

I have absolutely no idea what you're talking about...

Interesting discussion. Something to add to the equation:

I understand that from an asset designer's point of view, certain assets, like a visor, are intended to be used in combination solely with certain helmets. The same goes for certain trims, epaulets, mantal shoulders and now also the eyes and expressions as an example.

From a character designer's perspective though, there can be quite interesting combinations possible that you as a creator of the asset might not even had think of when creating the asset in the first place.

I have zero problems with this. I want to be as flexible as possible. I only want to add restrictions where it makes sense to have restrictions. This is for things like trim layers that ONLY match one asset or overlay recolors that ONLY match one asset.

These literally cannot work with other assets, don't make any sense, and is bad UI design to not have tighter restrictions on them, such as removing them when the required asset is swapped off.

I get having flexibility, but some assets literally do not work with other assets whatsoever.

Sorry I wasn't able to participate in the discussion, but I was literally asleep... :P

@jrconway3
Copy link
Contributor Author

So the more I look at this and go more in-depth with these expressions... I might not need that eye overlay after all. So far I haven't seen a single asset that actually moves the eye color around. At least for Male/Female anyway, Elderly looks way different. What appears to be happening is each expression just covers up parts of the eyes.

I'm going for the objective of minimizing the overlays as much as possible. If any pixel is in the same spot in the default head and I can hide the expression and the pixel is identical, then I remove that pixel from the overlay.

Look L, Look R, and Closing might be the only assets that move the irises from what I'm seeing currently.

However, the swap will still be useful, and we might still want it for elderly. Also, if I want an as-accurate-as-possible expression, happy/blush should be 1px down, which will require a full overlay for male and female. Sad between Male/Female has a 2px difference, but the difference is so minimal I'm planning to just keep these the same anyway.

  "template": {
    "head": {
      "human_male": "human",
      "human_female": "human",
      "elderly_male": "elderly",
      "elderly_female": "elderly",
      etc.
    }
  },
  "layer_1": {
    "zPos": 101,
    "male": "head/faces/${head}/anger/",
    "muscular": "head/faces/${head}/anger/",
    "female": "head/faces/${head}/anger/",
    "teen": "head/faces/${head}/anger/",
    "pregnant": "head/faces/${head}/anger/"
  },

Perhaps the template should have a "default" option as well?

jrconway3 added a commit to jrconway3/Universal-LPC-Spritesheet-Character-Generator that referenced this issue Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants