Save assets for generated cylinder meshes

[at669]

Proposed change(s)

This PR introduces saving the generated cylinder meshes (to a new folder, Assets/URDF/GeneratedMeshes, as primitive cylinders will have no associated .stl filename/path) such that they can be saved as prefabs. Additionally, minor changes to the Runtime AssetDatabase-related functions to prevent creating duplicate folders and providing an option for creating a unique path when saving a new asset.

Screen.Recording.2021-05-05.at.10.42.44.AM.mov

Useful links (GitHub issues, JIRA tickets, forum threads, etc.)

• Cylinder collider lost when imported URDF isturned into prefab
• AIRO-621

Types of change(s)

• Bug fix
• New feature
• Code refactor
• Documentation update
• Other (please describe)

Testing and Verification

Tested importing user's sample URDF (single cylinder), saving to prefab, and re-adding the prefab to the scene to ensure all colliders still appeared as desired. Repeated the process with Turtlebot (box + cylinders) and Niryo One (many custom meshes, no cylinders). Additionally, ran & verified existing unit tests pass.

Test Configuration:

• Unity Version: 2020.2.0b9
• Unity machine OS + version: MacOS 10.15.7
• ROS machine OS + version: N/A
• ROS–Unity communication: N/A

Checklist

• Ensured this PR is up-to-date with the dev branch
• Created this PR to target the dev branch
• Followed the style guidelines as described in the Contribution Guidelines
• Added tests that prove my fix is effective or that my feature works
• Updated the documentation as appropriate

Other comments

[hyounesy]

Main comments:

• I tested and the porposed PR does seem to fix the issue for Cylinder collider, however a similar problem will still occur for other basic geometries: box and sphere.
• Better not to modify the ConvertMeshToColliders() and instead create a new function to handle the basic geometry (either one for all three, or a separate one for each if there is not much to share in the code).
• Better to create the newly generated meshes inside the same directory as the URDF, rather than a new folder outside.

[hyounesy]

• I confirm your assessment that the issue is just with Cylinder and not with Sphere and Box.
• The issue seems to persist when convex decomposition = unity.
• I think it is better not to create the generated cylinder mesh in a new folder outside. As it fragments the artifacts created by URDF importer. And may lead to mistakes, e.g. when transferring the artifacts to a different project.
• If possible I prefer that we handle the creation of the folder and generating the name, outside the ConvertMeshToColiders(), but I am ok if it can't be done.

[peifeng-unity]

nit: do we want to add a couple of unit tests for the changes? :)

[at669]

nit: do we want to add a couple of unit tests for the changes? :)

good point, i have actually just been wondering about this! i thought some simple but relatively solid tests would be 1) importing a urdf with a) VHACD and b) Unity's mesh decomposer; 2) saving the new GameObjects as prefabs, and 3) adding the prefabs back into the scene. at each point the colliders should be verified to not be null, or something similar. however! given that there are no URDF files inherent to the package, i wasn't sure exactly how to proceed! we could add some dummy URDF files that are just primitive geometries, but it wouldn't cover stl and obj mesh-related tests. what are your thoughts @peifeng-unity?

[peifeng-unity]

@at669 Get you. For unit tests, I think we don't need to make it complicated. The tests of working with both VHACD and unity are out of the scope of this PR's change. It needs to be tested, but they don't have to be in this PR because it is more like integration tests. We only need to verify that the methods/functions in this PR behave as you expected.

In this PR, I personally think we only need to test the following functions:

1. To test the new private method ConvertCylinderToCollider(MeshFilter filter), you can directly test the method UrdfGeometryCollision.Create(Transform parent, GeometryTypes geometryType, Link.Geometry geometry) where the geometryType is GeometryTypes.Cylinder. After the create function, you can verify if the meshes are created correctly and if the assets are saved correctly. Please remember to delete the created assets and gameobjects at the end of each test.
2. For the changes in the ConvertMeshToColliders() method, we only need to test in non-runtime mode. You can directly execute the function and check if the files are created correctly.
3. Similarly, for the change in AssetDatabase_CreateFolder(), we only need to test if the folder can be created correctly in non-runtime mode in two different scenarios.
4. Same for AssetDatabase_CreateAsset(), we only need to verify if the function creates the assets in an expected manner.

[hyounesy]

• [ ] use unity's standard method to generate collision meshes instead of vhacd. either all the time (as the cylinder is always convex), but specially when user has not selected vhacd decomposition method.
• all other changes look good.

[peifeng-unity]

it looks good to me

[hyounesy]

Looks good.
minor suggestion for improvement is naming the unit tests according to this: https://docs.microsoft.com/en-us/dotnet/core/testing/unit-testing-best-practices#naming-your-tests

[hyounesy]

Tests pass and the code looks good.