8000 Add `generateRelative` function by dumbmatter · Pull Request #58 · zengm-games/facesjs · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add generateRelative function #58

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

Merged
merged 13 commits into from
Mar 23, 2025
Merged

Add generateRelative function #58

merged 13 commits into from
Mar 23, 2025

Conversation

dumbmatter
Copy link
Member
@dumbmatter dumbmatter commented Mar 21, 2025

This PR adds a generateRelative function which takes an existing face object and a gender and returns a new face object that is fairly similar to the existing one, but with some differences. Basically like an immediate family member.

It does this by regenerating only some properties from the input face, and others are just passed through without change. (The fraction of regenerated properties could be another parameter so you could create cousins or whatever, but I don't need that feature personally and I'm not sure if anyone else does.)

Here's an example of generating a chain of 6 relatives:

facesjs-20250321093735
facesjs-20250321093739
facesjs-20250321093742
facesjs-20250321093746
facesjs-20250321093749
facesjs-20250321093751

You can try it in the editor, I added a new option:

image

I don't love that UI because it looks kind of lonely having one option in "Other", and also because currently when you enable "Relative" it ignores the race options and the locked features. But I'm not sure what would be better.

Also I made this a separate function generateRelative rather than having it as another option in generate because it doesn't work with the existing overrides and race options, so I think it's less confusing as a separate function.

Fixes #5

@dumbmatter
Copy link
Member Author

@tomkennedy22 want to review my PR? No worries if you're too busy just lmk :)

@dumbmatter dumbmatter changed the title Add makeRelative function Add generateRelative function Mar 21, 2025
@tomkennedy22
Copy link
Contributor

@tomkennedy22 want to review my PR? No worries if you're too busy just lmk :)

Would be happy to, will take a look tonight 👍

Copy link
Contributor
@tomkennedy22 tomkennedy22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it works well! Nice work.

I left a few small food-for-thought comments, but all take it or leave it


// Currently, race just affects skin color and hair color. Let's ignore hair color (since you could imagine it being dyed anyway) and figure out someone's race just based on skin color. If no race is found, return undefined.
const imputeRace = (face: FaceConfig) => {
return races.find((race) => colors[race].skin.includes(face.body.color));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a non-issue now (and likely ever), but if multiple races share a skin color, might have conflict w/ this function

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... it would also break if the default skin colors change (unless the old defaults are kept around for this function). But I think it's good enough for now, we just need to remember to deal with it if a future update introduces an issue.

});

// Always regenerate some properties
face.accessories = randomFace.accessories;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder with these lines (30 ➡️ 56) if we have a master list of properties already defined, and then we just toggle some in & out of regenerate? To save from effectively defining two MECE lists back to back

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to google MECE but that is a good acronym, and this is also a good point. The tricky part is getting TypeScript to enforce that the list is actually exhaustive, but I think it's possible...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty nice now! TypeScript will error if anything is ever added to FaceConfig that makes this no longer exhaustive. And it lets you choose if you want to operate on an entire object (like accessories) or an exhaustive list of individual properties (like body).

  const regenerateProperties: RegenerateProperties = {
    accessories: "always",
    body: {
      color: "sometimesIfRaceIsKnown",
      id: "sometimes",
      size: "always",
    },
    ear: "sometimes",
    eye: "sometimes",
    eyebrow: "sometimes",
    eyeLine: "sometimes",
    facialHair: "always",
    fatness: "always",
    glasses: "always",
    hair: {
      color: "sometimesIfRaceIsKnown",
      flip: "always",
      id: "always",
    },
    hairBg: "always",
    head: {
      id: "sometimes",
      shave: "always",
    },
    jersey: "never",
    miscLine: "sometimes",
    mouth: "sometimes",
    nose: "sometimes",
    smileLine: "sometimes",
    teamColors: "never",
  };

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Clever way of doing it

@@ -53,6 +50,51 @@ const defaultTeamColors: TeamColors = ["#89bfd3", "#7a1319", "#07364f"];

const roundTwoDecimals = (x: number) => Math.round(x * 100) / 100;

export const numberRanges = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a fan of all of these additions :D

setShuffleGenderSettingObject,
setShuffleRaceSettingObject,
setShuffleOtherSettingObject,
} = stateStore;
const genders: Gender[] = ["male", "female"];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was my fault originally, but should genders & races be imported from common as well?

@dumbmatter dumbmatter merged commit 9333374 into master Mar 23, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Relatives
2 participants
0