-
Notifications
You must be signed in to change notification settings - Fork 36
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
Mesh Helper multi-mesh support #241
Conversation
/// </summary> | ||
/// <param name="source">Imported Ready Player Me avatar.</param> | ||
/// <param name="target">Photon Network Player Character.</param> | ||
public static void TransferMesh(GameObject source, GameObject target) |
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 is breaking change? We can't remove the TransferMesh with old definition yet?
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.
Yes changed the signature. Here is the PR for photon sample changes: readyplayerme/rpm-unity-sdk-photon-support#7
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.
What I meant is that this would require a move to the 7.0.0 version, since it might break functionality for the users. Do we want that?
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 is a minor functionality change probably only used by people who used latest version of photon demo with this method, I would not expect it to be that impactful but you would know better if it is widely used somewhere.
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 better bring back old signature and mark it obsolete.
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 don't on usage, but I rather don't want to find it out 😅
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.
Okay now it wont break, I brought old signature, incase anyone still relied on it. Will work and warn that the method is obsolete.
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.
Looks good in general, but we need the old definition back as well.
<!-- Copy the TICKETID for this task from Jira and add it to the PR name in brackets --> <!-- PR name should look like: [TICKETID] My Pull Request --> <!-- Add link for the ticket here editing the TICKETID--> ## [TICKETID](https://ready-player-me.atlassian.net/browse/TICKETID) ## Description ### Updated - AvatarMeshHelper now supports multiple mesh and material transfer. [#241](#241) ### Added - GetHeadMeshes method to AvatarMeshHelper to get head related meshes from an avatar. [#242](#242) - Template avatar with all possible meshes is added to the Resources folder. ## Checklist - [x] Tests written or updated for the changes. - [ ] Documentation is updated. - [x] Changelog is updated. <!--- Remember to copy the Changes Section into the commit message when you close the PR --> --------- Co-authored-by: Harrison Hough <[email protected]> Co-authored-by: Raigo Kovask <[email protected]> Co-authored-by: Raigo Kõvask <[email protected]>
SDK-524
Description
How to Test
Checklist