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

Small CAC refactoring in prep for CAC 2.0 and wizardless flow #151

Merged
merged 9 commits into from
Nov 14, 2023

Conversation

HarrisonHough
Copy link
Collaborator

@HarrisonHough HarrisonHough commented Nov 13, 2023

TICKETID

Description

  • This PR adds some minor changes which should add some extra flexibility and help us in CAC 2.0. Please share your feedback though and point out any potential floors in the updates. I will list some main changes and the reasoning below.

1. Added an extra return property to the CreateAvatar(AvatarProperties avatarProperties) method (made it return a tuple same as CreateAvatarFromTemplate.
Reasoning: To create an avatar from a photo with gender prediction enabled we need to also have the avatar properties, in particular we can get the gender of the predicted avatar which is required to load/update the avatar.

2. Removed gender from AvatarManager constructor
Reasoning: this makes AvatarManager easier to work with as you don't need to create a new one each time you change genders. Also CreateAvatar() function is passed avartarProperties (which can optionally have gender predefined) and CreateAvatarFromTemplate gets the gender from Template data. The only thing that may be useful in future is to add a separate "SetGender" function but at the moment I don't really see a use for it.

3. Removed partner parameter from CreateAvatarFromTemplate() function.
Reasoning: Why would anybody ever want to pass a "partner" value that is not the same as in their CoreSettings ? Doing so would probably break API's anyways (due to non matching subdomain and API key

How to Test

  • Mainly just test that current CAC sample still works (and it should)

Checklist

  • Tests written or updated for the changes.
  • Documentation is updated.
  • Changelog is updated.

@HarrisonHough HarrisonHough requested a review from a team as a code owner November 13, 2023 12:03
@HarrisonHough HarrisonHough changed the title [] Small CAC refactoring in prep for CAC 2.0 and wizardless flow Small CAC refactoring in prep for CAC 2.0 and wizardless flow Nov 13, 2023
Copy link
Contributor

@rYuuk rYuuk left a comment

Choose a reason for hiding this comment

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

🟢 Looks good

@HarrisonHough HarrisonHough merged commit f7bee7b into develop Nov 14, 2023
@HarrisonHough HarrisonHough deleted the feature/cac-playground branch November 14, 2023 14:20
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.

2 participants