-
Notifications
You must be signed in to change notification settings - Fork 104
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
Added selective skin color for certain races #10
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, you should add something to the README explaining the race option.
I'm already working on how BBGM will integrate with this new option :)
src/generate.ts
Outdated
@@ -6,9 +6,11 @@ const getID = (type: string): string => { | |||
return svgsIndex[type][Math.floor(Math.random() * svgsIndex[type].length)]; | |||
}; | |||
|
|||
type Race = "Caucasian" | "African" | "Asian" | "Latino"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No real reason other than it looking better to me... can you change this to: "white" | "black" | "asian" | "hispanic"
src/generate.ts
Outdated
} | ||
return options.chosenRace; | ||
})(); | ||
const eyeAngleAdjust = (() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should get rid of this. There are various facial features that vary across races, and in theory they could be supported here, but it probably should be done carefully to make sure it's done with appropriate sensitivity. (Not saying you did anything wrong, I'm just cautious.)
src/generate.ts
Outdated
]; | ||
|
||
const defaultTeamColors = ["#0d435e", "#f0494a", "#cccccc"]; | ||
|
||
const roundTwoDecimals = (x: number) => Math.round(x * 100) / 100; | ||
|
||
const generate = (overrides: Overrides) => { | ||
const eyeAngle = Math.round(Math.random() * 25 - 10); | ||
const generate = (overrides: Overrides, options: { chosenRace: Race }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change chosenRace
to race
src/generate.ts
Outdated
@@ -6,9 +6,11 @@ const getID = (type: string): string => { | |||
return svgsIndex[type][Math.floor(Math.random() * svgsIndex[type].length)]; | |||
}; | |||
|
|||
type Race = "Caucasian" | "African" | "Asian" | "Latino"; | |||
|
|||
const colors = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be better as an object, like:
const colors = {
white: {
skin: ["#f2d6cb", "#ddb7a0"],
hair: [
"#272421",
"#3D2314",
"#5A3825",
"#CC9966",
"#2C1608",
"#B55239",
"#e9c67b",
"#D7BF91",
],
},
asian: {
skin: ["#f2e4cb", "#f5dbad"],
hair: ["#272421", "#0f0902"],
},
hispanic: {
skin: ["#bb876f", "#aa816f", "#a67358"],
hair: ["#272421", "#1c1008"],
},
black: { skin: ["#ad6453", "#74453d", "#5c3937"], hair: ["#272421"] },
};
That way you can just glance at it and know immediately what each set of colors corresponds to, rather than having to count the array index and scroll down to see the labels.
I think that would also make the code using colors
below clearer too.
No description provided.