feat: add forceRenderer property
#5259
Conversation
|
After the CI passes: WebThis branch can be previewed at:
Desktop:If you have the launcher installed (download launcher) you can press open on the following link: SDK 6/7:More |
|
We have some places where we generate random profiles such as in |
|
@Julieta11 I think you can leave it as it is |
lorux0
force-pushed
the
feat/backpack-force-renderer
branch
from
4b9d01a to
2862ae2
Compare
unity-renderer/Assets/Scripts/MainScripts/DCL/UserProfile/UserProfileModel.cs
Show resolved
Hide resolved
unity-renderer/Assets/Scripts/MainScripts/DCL/Models/AvatarModel/AvatarModelDTO.cs
Show resolved
Hide resolved
| } | ||
|
|
||
| public override BaseModel GetDataFromJSON(string json) => | ||
| Utils.SafeFromJson<AvatarModel>(json); | ||
|
|
||
|
|
||
| public AvatarModelDTO ToAvatarModelDto() |
There was a problem hiding this comment.
The dependency should be inverted. The DTO should be in an outter layer from the model class. The model class should not depend on DTOs. So instead you should do:
AvatarModelDTO AvatarModelDTO.FromModel(avatarModel);
AvatarModel model = avatarModelDto.ToAvatarModel();
You can postpone this change tho, but please add the TODO comment
| var payload = new SaveAvatarPayload() | ||
| { | ||
| avatar = avatar, | ||
| avatar = avatar.ToAvatarModelDto(), |
There was a problem hiding this comment.
You could replace the parameter to AvatarModelDTO instead of doing an implicit conversion
| @@ -221,7 +221,7 @@ async UniTaskVoid LoadUserProfileAsync(UserProfile userProfile, CancellationToke | |||
| model.skinColor = userProfile.avatar.skinColor; | |||
| model.hairColor = userProfile.avatar.hairColor; | |||
| model.eyesColor = userProfile.avatar.eyeColor; | |||
|
|
|||
| model.forceRender = new HashSet<string>(userProfile.avatar.forceRender); | |||
There was a problem hiding this comment.
Should we set a null value into the HashSet in case forceRender is null? maybe in that case we should asign an empty HashSet, right?
| } | ||
|
|
||
| avatarSlotsHUDController.Recalculate(model.forceRender); |
There was a problem hiding this comment.
Make sure nothing happens if we pass in a null value to the Recalculate() method.
Fixes #5221
New property for profiles as discussed in ADR: decentraland/adr#239