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 hair and skin colours (done but awaiting a possible error-trap) #27821

Closed
wants to merge 35 commits into from

Conversation

Projects
None yet
9 participants
@I-am-Erk
Copy link
Contributor

commented Jan 24, 2019

Summary

SUMMARY: Features "Adds hair and skin colour traits"

Purpose of change

As I add more NPCs it is increasingly clear that they look pretty ridiculous as clones of each other.

Describe the solution

Zet's Hair Extensions adds a huge number of hair and skin colour traits that are already supported by the livepeople tileset. edit: After going through Zet's stuff I just don't think it follows a reasonable pattern. I've made my own settings. Probably existing sprites can still be used but will need indexing to the new names. The skin colours are still largely unchanged.

The major problem of the mods for hair style in the past is that they clutter the mutations list, but now we can disable that by setting player_display to false, which I have done. It is now also much easier to tell what kind of hair style you're selecting during character creation.

refcenterplus

Describe alternatives you've considered

I could remove some of the hair styles, since I am only really interested in the colours, but if we're adding variety we may as well add variety.

I considered adding the eye colours, freckles, and unnatural hair colours, but this is already much much longer than I want in one PR.

Problems I'm aware of

  • I have to change the default character sprite to bald. This means existing characters are all going to be bald unless you debug in a hair style. I'm sorry, I can't see any way around this.
  • The character creation stuff is messy. You can choose all kinds of features but the list is huge. I made it a bit tidier and gave it better descriptions but it's still bad.
  • In my opinion the skin colours all look pretty similar (except dark brown), but this is a start at least.

Next wave features

  • Appearance traits should probably get their own tab in character creation instead of cluttering the interface.

  • add eye colour, beards, freckles, and possibly tattoos if I feel artistic

  • Import skin and hair colour code from Zet's mod

  • Completely rewrite hair code

  • Update mutation ordering

  • Fix out-of-date mutation replacements with much more concise 'types'

  • add trait flags to hair and skin colours so that hairdresser can recognize them

  • rename hair styles to useful player-readable names instead of numbers

  • add trait groups to assign appearances randomly to NPCs according to real demographics

  • If the PR isn't too long at that point, add eye colour too. - it's too long.

  • fix a typo where I said "grey" instead of "gray"

I-am-Erk added some commits Jan 22, 2019

Revert "Merge pull request #6 from CleverRaven/master"
This reverts commit 753b1ac, reversing
changes made to 9a32197.
Mainline hair and skin traits
Mainlines hair and skin traits from Zet's hair extensions mod. Removes some backwards compatibility and fixes some of the out of date names, which will break graphics links for now. Does not yet implement trait groups to add these to NPCs.
@Vasyan2006

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

You can replace all 4k of cancels with "types" : ["HAIR_COLOR"], and "types" : ["SKIN_COLOR"],
Why these traits are professions? It makes no sense.
What is "player_visible": false,? It will never be used whatever it is.

@Kelenius

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

You can replace all 4k of cancels with "types" : ["HAIR_COLOR"], and "types" : ["SKIN_COLOR"],

Please, do this.

@Vasyan2006

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

It will require some coding, but hair styles and colors must be separated. So instead of hair6black character will have hair6 and hair_black. It will significantly reduce total number of mutations and make adding new styles/colors easier. After that proper hair tile will be assigned based on both mutations for style and color.

@paulenka-aleh

This comment has been minimized.

Copy link

commented Jan 24, 2019

Can these be applied to the player character as well? Even if randomly!

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

It will require some coding, but hair styles and colors must be separated. So instead of hair6black character will have hair6 and hair_black. It will significantly reduce total number of mutations and make adding new styles/colors easier. After that proper hair tile will be assigned based on both mutations for style and color.

Great suggestion. Maybe we should even add some new entity instead of using traits, e.g. appearance which would contains fields like body_part, color, style? Or expand body parts to include additional parameters like hairstyle, hair color, scars, etc?

@Vasyan2006

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

So this appearance can be divided into different slots, and new entities will work more like mutations (with mutual requirements and exclusivity), but only inside their slot. Something like

"appearance": [
      { "head_hair" : [ "black", "length6", "braids" ] },
      { "scar" : [ "left_arm3", "right_leg2" ] },
      { "tattoo" : [ "right_arm1", "left_leg4" ] }

or

"appearance": [
      { "head_hair" : [ "black", "length6", "braids" ] },
      { "left_arm" : [ "scar3", "tattoo2" ] },
      { "right_leg" : [ "sca1", "tattoo4" ] }

Because updating appearance is the most important thing in the apocalypse.

@I-am-Erk

This comment has been minimized.

Copy link
Contributor Author

commented Jan 24, 2019

You can replace all 4k of cancels with "types" : ["HAIR_COLOR"], and "types" : ["SKIN_COLOR"],
Why these traits are professions? It makes no sense.

The professions thing seems to have been carried over from the mod, not sure why they have that. Should be safe to remove.

What is "player_visible": false,? It will never be used whatever it is.

What do you mean? It's described even within the PR. That's a flag mlangsdorf added a little while ago that hides the trait from the list of mutations in the player status window. It will definitely be used.

@I-am-Erk

This comment has been minimized.

Copy link
Contributor Author

commented Jan 24, 2019

It will require some coding, but hair styles and colors must be separated. So instead of hair6black character will have hair6 and hair_black. It will significantly reduce total number of mutations and make adding new styles/colors easier. After that proper hair tile will be assigned based on both mutations for style and color.

Great suggestion. Maybe we should even add some new entity instead of using traits, e.g. appearance which would contains fields like body_part, color, style? Or expand body parts to include additional parameters like hairstyle, hair color, scars, etc?

It would be really nice to be able to decouple appearance elements, but I'm not sure they need a whole separate system from traits. There isn't much about them that differs from other traits to merit their own system, and trait groups work perfectly for what we want.

While it would be great to have such a system, I'm hampered in NPC development without being able to set their appearance, and nobody has been jumping at the idea of coding anything. If someone wants to do it, and can suggest a rudimentary way I could get NPC traits rolling until then that they could build on, that would be neat.

@Vasyan2006

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

That's a flag mlangsdorf added a little while ago

Didn't know that, sorry. But is it already in the game? I initially just searched yesterday's source code for player_visible and got zero results. And now for testing added it to common mutation and this mutation is still visible in @ window on PC and NPC. Or I am just testing it wrong.

@Kelenius

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

I think that adding them as traits works for now and leaves room for upgrade.

@AMurkin

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

What is "player_visible": false,? It will never be used whatever it is.

What do you mean? It's described even within the PR. That's a flag mlangsdorf added a little while ago that hides the trait from the list of mutations in the player status window. It will definitely be used.

It should be player_display. #27432
@I-am-Erk

This is the only occurrence of player_visible (#27408):

{
"type": "effect_type",
"id": "BGSS_Merc_haswhisky",
"//": "Defined here because this should be the only time this effect is referenced.",
"name": [ "I got whisky" ],
"player_visible": false,
"desc": [ "AI tag: This merc is happy with the whisky you brought. This is a bug if you have it." ]
}

@I-am-Erk

This comment has been minimized.

Copy link
Contributor Author

commented Jan 24, 2019

What is "player_visible": false,? It will never be used whatever it is.

What do you mean? It's described even within the PR. That's a flag mlangsdorf added a little while ago that hides the trait from the list of mutations in the player status window. It will definitely be used.

It should be player_display. #27432
@I-am-Erk

This is the only occurrence of player_visible (#27408):
Cataclysm-DDA/data/json/npcs/Backgrounds/scavenger_merc_1.json

Lines 175 to 182 in b5b2770

{
"type": "effect_type",
"id": "BGSS_Merc_haswhisky",
"//": "Defined here because this should be the only time this effect is referenced.",
"name": [ "I got whisky" ],
"player_visible": false,
"desc": [ "AI tag: This merc is happy with the whisky you brought. This is a bug if you have it." ]
}

It should already be, but apparently I didn't push my last fix to GitHub before bed.

@I-am-Erk

This comment has been minimized.

Copy link
Contributor Author

commented Jan 24, 2019

Also I didn't realize that Merc whisky still had that effect style. I took it down everywhere else last week... Thanks for the catch.

@AMurkin

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

And now JSON is broken after your last commit.

@I-am-Erk

This comment has been minimized.

Copy link
Contributor Author

commented Jan 24, 2019

As it says on the last commit, it's not a completed update. There was just too much changing to do for me to get it done before going to work, so I uploaded what I had so I could finish it here.

This may be the most active pr I've made. Add fifty NPC backstories, nobody reads the code ... Add hair styles, and each commit is getting checked with a fine tooth comb.

@Vasyan2006

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

It should be player_display

Ah, player_display is working on my side, kk.

Mutation PALE is one of default mutations, so it must be changed in mutation.json.

@I-am-Erk

This comment has been minimized.

Copy link
Contributor Author

commented Jan 24, 2019

It should be player_display

Ah, player_display is working on my side, kk.

Mutation PALE is one of default mutations, so it must be changed in mutation.json.

Interesting, I wonder why zet had it in the mod. I would rather leave it to mutation.json then, I have plenty of caucasian skin colours.

Some thoughts I had on my way to work today:

  • I could pull hairstyles out of this mod and just have it affect hair colour, leaving hair style for later on where someone can figure out how to have hair style inherit colour from the base trait.
  • I want to edit the skin colour tiles, the deadpeople ones look kind of odd.
  • I might add freckles and eyes into this since removing the out-of-date replacements has given me much more room to work with
  • I think beard styles need to be separated from hair styles, especially if we can figure out a way to get hair styles to work as a separate trait that inherits colour from hair_colour.

@I-am-Erk I-am-Erk changed the title Mainline zet's hair and skin colours Add hair and skin colours Jan 24, 2019

Update mutation_type.json
I accidentally added a line.
@kevingranade

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/new-appearance-system/18549/1

I-am-Erk added some commits Jan 26, 2019

Update tile_config.json
More carefully this time
Update 5_tiles_character_customization_32x32_9568-11567.png
Change bodies with hair to bald bodies. This will screw up existing saves but I don't have a good solution just yet. Some have been suggested.

@I-am-Erk I-am-Erk changed the title Add hair and skin colours Add hair and skin colours (done but awaiting input from Kevin) Jan 26, 2019

@Leviathan21

This comment has been minimized.

Copy link
Contributor

commented Jan 26, 2019

you forgot to mainline tattoo mod

I-am-Erk added some commits Jan 26, 2019

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jan 26, 2019

Btw the feedback from me out-of-band was that it should be possible to add a random hairstyle to old characters instead of setting them to bald.

Update appearance_trait_groups.json
caught another error

@I-am-Erk I-am-Erk changed the title Add hair and skin colours (done but awaiting input from Kevin) Add hair and skin colours (done but awaiting a possible error-trap) Jan 27, 2019

@@ -3913,7 +3913,8 @@
"description" : "Your skin is rather pale.",
"changes_to" : ["ALBINO"],
"leads_to" : ["TROGLO"],
"category" : ["LUPINE"]
"category": [ "LUPINE" ],
"types": [ "skin_tone" ]
},{

This comment has been minimized.

Copy link
@Vasyan2006

Vasyan2006 Feb 4, 2019

Contributor

Remove PALE with all references from here and move it to cosmetic mutations, it will simplify a lot of things. There is even an open issue about it somewhere.

This comment has been minimized.

Copy link
@I-am-Erk

I-am-Erk Feb 4, 2019

Author Contributor

Pale is so far defined as a mutation, with visibility and ugliness stats and everything, and is connected to developing further mutations. I'm reluctant to move it around at the moment, particularly as I have no shortage of caucasian skin tones.

@@ -4,7 +4,7 @@
"id": "NC_NONE",
"name": "No class",
"job_description": "I'm just wandering.",
"traits": [ { "group": "BG_survival_story_EVACUEE" }, { "group": "NPC_starting_traits" } ],
"traits": [ { "group": "BG_survival_story_EVACUEE" }, { "group": "NPC_starting_traits" }, { "group": "Appearance_demographics" } ],
"skills": [

This comment has been minimized.

Copy link
@Vasyan2006

Vasyan2006 Feb 4, 2019

Contributor

You can add politically incorrect demographics to NPC_starting_traits instead of adding it to every class.
And dont forget mutant NPCs from the mod, they want new look too.

@@ -13,7 +13,7 @@
"worn_override": "REFUGEE_Jenny_worn",
"carry_override": "REFUGEE_Jenny_carried",
"weapon_override": "REFUGEE_Jenny_wield",
"traits": [ { "trait": "PACIFIST" }, { "trait": "Exp_Eng_Mechanical2" } ],
"traits": [ { "trait": "PACIFIST" }, { "trait": "Exp_Eng_Mechanical2" }, { "group": "Appearance_Caucasian" } ],
"skills": [

This comment has been minimized.

Copy link
@Vasyan2006

Vasyan2006 Feb 4, 2019

Contributor

Jenny does not have common traits. Is is intended?

This comment has been minimized.

Copy link
@I-am-Erk

I-am-Erk Feb 4, 2019

Author Contributor

Yep. Not really related to this PR though.

@SomeDeadGuy

This comment has been minimized.

Copy link
Contributor

commented Feb 4, 2019

Cant wait for this!
Please give me propper id's what i should assign sprites to. If they were changed from Zetsuke mod.
In case i will need to redraw or add new customizations in my tileset.

It would be cool for characters to close their eyes when they sleep too. I can do it now, with one skin collor, but with multiple collors i dunno... You know, we would need different eyelids depending on character skin color. I guess i can make skin sprite with closed eyes and when character sleeps eyes should disappear from the character revealing closed eyes on body sprite. But i will need your help with this. When character sleep their eyes sprite should become invisible, thus revealing closed eyes drawn on "skin" of the character.

@I-am-Erk

This comment has been minimized.

Copy link
Contributor Author

commented Feb 4, 2019

I've actually coded sprite id's in here already. Most work well but the curly hair I'm using for a 'fro isn't very puffy, a real big seventies afro would be awesome. Also I think skin_tan and skin_lightbrown could both be a bit darker.

Otherwise, not much art change needed yet. Soon I'll be doing another pr with more colours and maybe more styles depending on interface limits

@kevingranade

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

Merged into master, I resolved the tileset conflict in favor of master, i.e. the tileset changes have been ignored and need to be re-added.

@I-am-Erk I-am-Erk deleted the I-am-Erk:mainline-appearance branch Mar 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.