Skip to content

Conversation

@judubu
Copy link
Contributor

@judubu judubu commented Feb 17, 2021

Purpose of this PR

Ticket/Jira #: https://jira.unity3d.com/browse/USDU-155

Preparatory work for the mesh import/conversion layer fixes.
UsdMenu used to contain a lot of import logic making it hard to reuse and test. All the logic has been moved to 2 new static classes ImportHelpers and ExportHelpers.

Testing

Functional Testing status:
Tried to cover 100% of the Import/ExportHelpers.

Performance Testing status:
N/A

Overall Product Risks

Complexity:
Low

Halo Effect:
Low

Additional information

Note to reviewers:

Reminder:

  • Add entry in CHANGELOG.md (if applicable)

namespace Unity.Formats.USD
{
public class UsdMenu : MonoBehaviour
public class UsdMenu
Copy link
Contributor

Choose a reason for hiding this comment

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

make the class static maybe ?

Copy link
Contributor Author

@judubu judubu Feb 22, 2021

Choose a reason for hiding this comment

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

Fixed in 3cd29eb

#if UNITY_EDITOR
public static Scene InitForSave(string defaultName, string fileExtension = "usd,usda,usdc")
{
var filePath = EditorUtility.SaveFilePanel("Export USD File", "", defaultName, fileExtension);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would split this function differently:
The only editor only function is EditorUtility.SaveFilePanel: add that in the editor assembly and keep the rest of of this function as a runtime function.

Copy link
Contributor Author

@judubu judubu Feb 22, 2021

Choose a reason for hiding this comment

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

Fixed in 3cd29eb

{
Scene scene = null;

foreach (GameObject go in Selection.gameObjects)
Copy link
Contributor

Choose a reason for hiding this comment

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

Selection exists only in the editor. Make a function that exports a list of GOs and call that with the selection from the editor assembly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3cd29eb

}
catch (SceneImporter.ImportException)
{
Object.DestroyImmediate(root);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it makes a difference: in playmode you wanna call Destroy, and out of playmode DestroyImmediate. Make yourself an utility function that abstracts this.

}

#if UNITY_EDITOR
public static void ImportAsPrefab(Scene scene)
Copy link
Contributor

Choose a reason for hiding this comment

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

Return the import results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 53a493c

}
}

public static void ImportAsTimelineClip(Scene scene)
Copy link
Contributor

Choose a reason for hiding this comment

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

Return the import result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 53a493c

/// </summary>
static string GetSelectedAssetPath()
{
Object[] selectedAsset = Selection.GetFiltered(typeof(Object), SelectionMode.Assets);
Copy link
Contributor

Choose a reason for hiding this comment

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

Think Selection is Editor only

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 53a493c

Copy link
Collaborator

@mfe mfe left a comment

Choose a reason for hiding this comment

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

LGTM
Did some testing on 2020.2 (win), change looks indeed transparent.

@judubu judubu requested review from clusty and mfe February 27, 2021 15:25
@judubu judubu changed the title Create import/export helpers to clean the UsdMenu #USDU-155 Create import/export helpers to clean the UsdMenu Feb 27, 2021
Copy link
Collaborator

@mfe mfe left a comment

Choose a reason for hiding this comment

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

LGTM

@judubu judubu merged commit 23fb5ce into dev Mar 12, 2021
@judubu judubu deleted the import_helpers branch March 12, 2021 21:55
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.

4 participants