[Draft][USDU-331] Make package samples more user friendly #367

lee-aandrew · 2023-04-04T21:31:27Z

Purpose of this PR

Ticket/Jira #:

Currently our package samples can be a bit confusing for users,
To increase user friendly-ness the following have been addressed:

In general, all Samples are much less Play Mode reliant - I've changed all samples to be done via Editor buttons, with changes being made in Edit Mode
All Samples should be able to be fully executed within the "SAMPLE ENTRY POINT" object, without having to select different Unity GameObjects
ExportAnimationWithRecorder (New):
-- Quick example scene with serialized fields that show the proper set up of how to record animation of Unity GameObject and export it as a USD file
ExportMesh:
-- Made the process of exporting a more step-by-step process
HelloUsd:
-- Added more console debug logs
-- Added auto focus to console window
ExportMeshTransformOverrides
-- Set up a scene where a USD asset is imported
-- Randomly changes the transform value
-- Exports it as a transform overrides USD file
ExportMeshWithAnimation
-- Made it more a step by step process
ImportMaterials
-- Made it more a step by step process
ImportMesh
-- Made it more a step by step process
ImportProcessor:
-- Added a new scene that show off the functionalities of CombineMeshes.cs and SetHideFlags.cs that implements Unity USD's ImportProcessor
UsdTimelinePlayable:
-- Quick example scene with serialized fields that show the proper set up of how to play the animation of an imported USD asset

Testing

Functional Testing status:

Performance Testing status:

Overall Product Risks

Complexity:

Halo Effect:

Additional information

Note to reviewers:

Reminder:

Add entry in CHANGELOG.md (if applicable)

…-sdk into dev

package/com.unity.formats.usd/Samples/UsdTimelinePlayable/README.md

package/com.unity.formats.usd/Samples/HelloUsd/HelloUsdExample.cs

…-sdk into dev

vickycl

I've run through the samples and I think there's a confusing mix of samples running in playmode and edit mode. Because we don't really support USD at runtime, I think we should make all of the samples consistently run in Editor only and maybe use more buttons to trigger behaviours (which I think would be more similar to what users will probably want to do).

Let's have a call to discuss next steps :)

package/com.unity.formats.usd/package.json

…-sdk into dev

…linePlayable

This reverts commit 24392f2.

This reverts commit 8f76aee.

…ample project

vickycl

A few comments based only on the PR, let me know when you want me to check out and test the samples :)

package/com.unity.formats.usd/Samples/ExportMeshWithAnimation/Explode.cs

package/com.unity.formats.usd/Samples/ExportMeshWithAnimation/ExportMeshWithAnimationExample.cs

package/com.unity.formats.usd/Samples/HelloUsd/Editor/HelloUsdExampleEditor.cs

package/com.unity.formats.usd/Samples/ImportProcessor/Editor/SetHideFlagsEditor.cs

package/com.unity.formats.usd/Samples/UsdTimelinePlayable/README.md

… prefab

…use custom step by step method rather than simply using the ExporterHelper functions

vickycl

ImportMesh samples are much nicer now :) There's just some small points on those ones for consistency.

I found the new layout for the PostProcessing samples to be confusing, as a couple of things were hidden away in folder structures or different scenes. Hopefully the comments made sense?

For the ImportMaterials samples, I think the step breakdown is too much. There's no value to having each step as a separate button, and more chance of breakage. I think we should return that to being more simple, as a user wanting to emulate that sample would need to look at the code anyway and not care about the button steps. I also think it's missing the previous behaviour of applying the imported material to the cube in the scene and rotating in playmode to show it off.

There are two missing .sample.json files- I think they were for ExportMesh and ExportMeshTransformOverrides. These files help the UPM-CI find the Samples and add them to the package json.

package/com.unity.formats.usd/Samples/ExportMesh/Editor/ExportMeshExampleEditor.cs

...rocessor/PostProcessComponents_CombineMeshes/ImportProcessorExample_PostProcessComponents.cs

package/com.unity.formats.usd/Samples/ImportProcessor/ExampleAsset.meta

package/com.unity.formats.usd/Samples/ImportMaterials/Editor/ImportMaterialsExampleEditor.cs

package/com.unity.formats.usd/Samples/ImportMaterials/ImportMaterialsExample.cs

...com.unity.formats.usd/Samples/UsdTimelinePlayable/Editor/UsdTimelineRecorderExampleEditor.cs

vickycl · 2023-05-04T09:45:35Z

...com.unity.formats.usd/Samples/UsdTimelinePlayable/Editor/UsdTimelineRecorderExampleEditor.cs

+                GUILayout.Label($"  5. Set the <color={SampleUtils.TextColor.Blue}>Source Object</color> within the newly created <b>'Usd Playable Track'</b> as the USD GameObject, <color={SampleUtils.TextColor.Yellow}>'UsdAssetWithTimeline'</color> (first child of this object).", labelStyle);
+                GUILayout.Label($"  6. Within the 'Timeline Window' Right Click within the newly created <b>Usd Playable Track</b>, and select <color={SampleUtils.TextColor.Yellow}>'Add From Usd Asset'</color>.", labelStyle);
+                GUILayout.Label($"  7. In the new pop up <color={SampleUtils.TextColor.Blue}>'Select Usd Asset'</color> window select the USD GameObject, <color={SampleUtils.TextColor.Yellow}>'UsdAssetWithTimeline'</color>.", labelStyle);
+                GUILayout.Label($"  8. You can now play the Timeline animation and see the animation being played in your <b>Scene View</b> window.", labelStyle);


This is only meant to be a 'Sample' so that a user can play around with the features, not a full walkthrough. Having the steps written out makes it quite brittle and easy to become out of date. I personally think these steps would be better suited to being in the documentation instead of a Sample, where they will be easier and more obvious to keep up to date.

I've removed a lot of steps from both this and ExportAnimationWithRecorderExample with the belief that all the serialized fields will be saved 🤞

Lemme know what you think!

package/com.unity.formats.usd/Samples/UsdTimelinePlayable/UsdTimelineRecorderExample.cs

vickycl · 2023-05-09T14:34:44Z

package/com.unity.formats.usd/Samples/ExportMesh/Editor/ExportMeshExampleEditor.cs

+                            SampleUtils.FocusConsoleWindow();
+
+                            script.SetUpExportContext();
+                            Debug.Log($"Export Context has been set up.");


I think each of these debug.logs should use the same green, so that they all stand out as behaviour caused by the samples

'Setup Export Context' isn't really a useful step. It doesn't explain what is happening or why. Perhaps just combine this with the export step, or at least call it "Generate settings for export"

vickycl · 2023-05-09T14:36:46Z

package/com.unity.formats.usd/Samples/ExportMesh/Editor/ExportMeshExampleEditor.cs

+
+                        if (GUILayout.Button("6. Close Scene"))
+                        {
+                            script.CloseScene();


I don't see a reason to do the Save and Close steps seperately- I think they should be combined into one step to reduce clicks.

vickycl · 2023-05-09T16:14:51Z

...y.formats.usd/Samples/ExportMeshWithAnimation/Editor/ExportMeshWithAnimationExampleEditor.cs

+            {
+                if (script.IsFinishedRecording)
+                {
+                    if (GUILayout.Button("Recording Complete - Stop Scene"))


Should be "Recording Complete - Exit Playmode"

vickycl · 2023-05-10T08:44:01Z

package/com.unity.formats.usd/Samples/ExportMesh/.sample.json

@@ -1,4 +1,4 @@
 {
    "displayName" : "ExportMesh",
-    "description" : "A sample showing how to record a scene to a USD file."
+    "description" : "A sample showing how Unity Game Object can be exported as a USD file (without animation)."


Add something along the lines of "using the Unity USD APIs directly" so that it's clear that this isn't the only way of doing it for a first time user

vickycl · 2023-05-10T08:45:40Z

package/com.unity.formats.usd/Samples/ExportMeshTransformOverrides/.sample.json

@@ -0,0 +1,4 @@
+{
+    "displayName" : "ExportMeshTransformOverrides",
+    "description" : "A sample showing an example of imported USD file exported as transform override."


I think this should be "A sample showing how to export an imported USD asset with modified transforms as a USD override file using the Unity USD APIs."

vickycl · 2023-05-10T08:46:33Z

package/com.unity.formats.usd/Samples/ExportMeshWithAnimation/.sample.json

@@ -0,0 +1,4 @@
+{
+    "displayName" : "ExportMeshWithAnimation",
+    "description" : "A sample showing how to record a scene to a USD file."


Should be "A sample showing how to record an animation to a USD file without using the recorder package."

vickycl · 2023-05-10T08:49:48Z

package/com.unity.formats.usd/Samples/ImportMesh/Editor/ImportMeshExampleEditor.cs

+                script.InitializeUsd();
+            }
+
+            if (GUILayout.Button("2. Set USD File Path..."))


This additional step makes the sample confusing. To make it less confusing and also remove the risk of it going wrong, just remove it and always import the USD file provided with the Sample.

vickycl · 2023-05-10T09:02:05Z

....unity.formats.usd/Samples/ImportProcessor/Editor/ImportProcessorExample_EntryPointEditor.cs

I think this approach is too confusing and detracts away from the automatic post processing we're trying to demonstrate. I think in the interest of getting this landed we should revert this sample to how it was for now, but I've sent you a message on Slack so we can discuss.

vickycl · 2023-05-10T12:34:58Z

package/com.unity.formats.usd/Samples/UsdTimelinePlayable/SetDefaultUSDMesh.cs

+
+        void Update()
+        {
+            SetSampleUsdFilePath();


This shouldn't be called every frame :)

I think having this run every frame is beneficial to make sure the user cant change the path for this USD Asset to another USD file since not every USD file will contain animation
I just want to enforce this specific asset on this Sample

The file path isn't easily modifiable anyway, and it's demonstrating a bad practice for a user (running something every editor frame, especially something doing a GetComponent and a string manipulation). It would be a very edge case behaviour if a user modified that file path manually, and personally if they did so I think it would be better if it caused an issue (as it would in a real case) instead of silently 'fixed' itself

vickycl · 2023-05-10T14:38:09Z

package/com.unity.formats.usd/Samples/HelloUsd/Unity.Formats.USD.HelloUsd.asmdef

[hijacking this file to say] I think the Hello USD would benefit from calling out in the .sample.json that it is the introductory walkthrough of how to write and read a USD file through code, so that it's easy for a user to pick a first sample to look at and also understand why it's broken down to be so granular.

lee-aandrew added 3 commits March 21, 2023 14:48

Ignores two unstable test cases until a fix has been found

c4b7a8b

Merge branch 'dev' of https://github.com/Unity-Technologies/usd-unity…

464ea5b

…-sdk into dev

Merge branch 'dev' of https://github.com/Unity-Technologies/usd-unity…

7af60ed

…-sdk into dev

lee-aandrew requested review from michaeljblain and vickycl April 5, 2023 13:22

michaeljblain requested a review from a team April 6, 2023 10:27

CraigHutchinson reviewed Apr 6, 2023

View reviewed changes

package/com.unity.formats.usd/Samples/UsdTimelinePlayable/README.md Outdated Show resolved Hide resolved

package/com.unity.formats.usd/Samples/HelloUsd/HelloUsdExample.cs Outdated Show resolved Hide resolved

Merge branch 'dev' of https://github.com/Unity-Technologies/usd-unity…

20dc1c7

…-sdk into dev

vickycl requested changes Apr 14, 2023

View reviewed changes

package/com.unity.formats.usd/package.json Outdated Show resolved Hide resolved

lee-aandrew added 11 commits April 14, 2023 15:45

Merge branch 'dev' of https://github.com/Unity-Technologies/usd-unity…

4236bd0

…-sdk into dev

Merge branch 'dev' of https://github.com/Unity-Technologies/usd-unity…

2d14b87

…-sdk into dev

Merge branch 'dev' of https://github.com/Unity-Technologies/usd-unity…

bb60d3e

…-sdk into dev

HelloUSD to be a bit more user-friendly

fa6e626

Make Samples more user-friendly: ExportMesh, ImportProcessor, UsdTime…

c354a2a

…linePlayable

fix formatting issue

8ec4518

Set up Samples properly according to Unity documentation

8f76aee

add removal of Samples.meta file

24392f2

Revert "add removal of Samples.meta file"

8399af7

This reverts commit 24392f2.

Revert "Set up Samples properly according to Unity documentation"

bbd2308

This reverts commit 8f76aee.

Update HelloUSD and Separated out + updated ExportMeshWithAnimation s…

9656bc2

…ample project

lee-aandrew force-pushed the make_samples_user_friendly branch from c563da3 to 9656bc2 Compare April 21, 2023 15:34

lee-aandrew changed the title ~~Make package samples more user friendly~~ [Draft] Make package samples more user friendly Apr 21, 2023

vickycl requested changes Apr 24, 2023

View reviewed changes

lee-aandrew added 7 commits April 25, 2023 14:11

Adds Export Mesh Example

5bc6608

Applies code review comments & fixes formatting issue

7eff80c

fixes variable naming miss

786ea4b

Quick clean up of non-used using references and Additional comments

9891944

Adds ExportMeshTransformOverride Sample

e02a734

Removes hard coded color codes & adds more comments where needed

9588f2c

Removed tree object from Export Mesh example as the tree object was a…

92da6e3

… prefab

lee-aandrew added 10 commits April 26, 2023 14:41

Quick Scene update for ExportMeshTransformOverride

33e3782

Fix naming issues

be78943

Changes ExportMeshExample and ExportMeshWithAnimationExample back to …

721337d

…use custom step by step method rather than simply using the ExporterHelper functions

Redo HelloUSD to be more informative

5d5827b

Applies Vicky's PR Review suggestions

fcba3e5

Apply formatting fix

e9f8a52

Adds Import Materials Sample Sample changes

fba3a79

Update Copyright comments

b446bff

Changes Import Mesh Sample to only be Import Mesh

94483f0

Changes Import Processort Samples

04d33ab

vickycl requested changes May 3, 2023

View reviewed changes

lee-aandrew added 2 commits May 3, 2023 19:19

Adds ExportAnimationWithRecorder sample

eddf389

Change UsdTimelinePlayable Sample

149d34f

vickycl requested changes May 4, 2023

View reviewed changes

lee-aandrew added 2 commits May 4, 2023 20:33

Apply review suggestions

8891a7b

Apply review suggestion for ExportAnimationWithRecorderExample

5f6ce5a

lee-aandrew requested a review from vickycl May 5, 2023 01:00

lee-aandrew changed the title ~~[Draft] Make package samples more user friendly~~ [USDU-331] Make package samples more user friendly May 5, 2023

vickycl requested changes May 10, 2023

View reviewed changes

vickycl reviewed May 10, 2023

View reviewed changes

lee-aandrew marked this pull request as draft May 10, 2023 19:53

lee-aandrew changed the title ~~[USDU-331] Make package samples more user friendly~~ [Draft][USDU-331] Make package samples more user friendly May 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Draft][USDU-331] Make package samples more user friendly #367

[Draft][USDU-331] Make package samples more user friendly #367

lee-aandrew commented Apr 4, 2023 •

edited

Loading

vickycl left a comment

vickycl left a comment

vickycl left a comment

vickycl May 4, 2023

lee-aandrew May 5, 2023 •

edited

Loading

vickycl May 9, 2023

vickycl May 9, 2023

vickycl May 9, 2023

vickycl May 9, 2023

vickycl May 10, 2023

vickycl May 10, 2023

vickycl May 10, 2023

vickycl May 10, 2023

vickycl May 10, 2023

vickycl May 10, 2023

lee-aandrew May 10, 2023

vickycl May 10, 2023

vickycl May 10, 2023

[Draft][USDU-331] Make package samples more user friendly #367

Are you sure you want to change the base?

[Draft][USDU-331] Make package samples more user friendly #367

Conversation

lee-aandrew commented Apr 4, 2023 • edited Loading

Purpose of this PR

Testing

Overall Product Risks

Additional information

vickycl left a comment

Choose a reason for hiding this comment

vickycl left a comment

Choose a reason for hiding this comment

vickycl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lee-aandrew May 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lee-aandrew commented Apr 4, 2023 •

edited

Loading

lee-aandrew May 5, 2023 •

edited

Loading