o3de / ROSConDemo

A robotic fruit picking demo project for O3DE with ROS 2 Gem
Other
70 stars 23 forks source link

Updates to support the PhysX 4/5 split #276

Closed spham-amzn closed 7 months ago

spham-amzn commented 8 months ago

What does this PR do?

This updates all the ROSConDemo to reference 'PhysX5' instead of 'PhysX', now that both gems will be available at the same time (without the need of a cmake flag when creating the project from source). (Refer to https://github.com/o3de/o3de/pull/17436 ).

How was this PR tested?

Built and launch the demo, spawning the robots and controlling with the editor

jhanca-robotecai commented 7 months ago

It seems it was me who reverted the changes, but I blame o3de. Michał did not change the PhysX into PhysX5 in physxsystemconfiguration.setreg and o3de simply cleaned the whole configuration of PhysX on the start. I pushed the changed files to the repo.

diff --git a/Project/Registry/physxsystemconfiguration.setreg b/Project/Registry/physxsystemconfiguration.setreg
index 5fef1b5..2288989 100644
--- a/Project/Registry/physxsystemconfiguration.setreg
+++ b/Project/Registry/physxsystemconfiguration.setreg
@@ -1,13 +1,13 @@
 {
     "Amazon": {
         "Gems": {
-            "PhysX": {
+            "PhysX5": {
                 "PhysXSystemConfiguration": {
                     "CollisionConfig": {
                         "Layers": {
                             "LayerNames": [
                                 "Default",
-                                "Robot",
+                                {},
                                 {},
                                 {},
                                 {},
@@ -80,7 +80,7 @@
                                 },
                                 {
                                     "Id": {
-                                        "GroupId": "{BE28E146-905B-44E2-BB05-2CA6CE7B4BE7}"
+                                        "GroupId": "{552F83A1-81FE-47B1-96D3-AE5ED9E7EA67}"
                                     },
                                     "Name": "None",
                                     "Group": {

I believe this is an issue with o3de and it should be fixed. The configuration should NOT be modified without informing the user. What if the user wants to check PhysX4? Please see the outcome of starting ROSCon2023Demo with the new configuration - it is not only the collision layers, but also time step settings and few more:

diff --git a/Project/Registry/physxsystemconfiguration.setreg b/Project/Registry/physxsystemconfiguration.setreg
index d6bd55f..237a967 100644
--- a/Project/Registry/physxsystemconfiguration.setreg
+++ b/Project/Registry/physxsystemconfiguration.setreg
@@ -1,15 +1,14 @@
 {
     "Amazon": {
         "Gems": {
-            "PhysX": {
+            "PhysX5": {
                 "PhysXSystemConfiguration": {
-                    "FixedTimeStep": 0.004999999888241291,
                     "CollisionConfig": {
                         "Layers": {
                             "LayerNames": [
                                 "Default",
-                                "Navigation",
-                                "Lidar",
+                                {},
+                                {},
                                 {},
                                 {},
                                 {},
@@ -81,31 +80,13 @@
                                 },
                                 {
                                     "Id": {
-                                        "GroupId": "{E68FAA37-AE19-4093-939A-A4B92759B4F6}"
+                                        "GroupId": "{7D94C802-D12D-48BC-B479-D59A06DC6664}"
                                     },
                                     "Name": "None",
                                     "Group": {
                                         "Mask": 0
                                     },
                                     "ReadOnly": true
-                                },
-                                {
-                                    "Id": {
-                                        "GroupId": "{EB55E2F1-479F-4D54-9593-38A9DEB9A3DA}"
-                                    },
-                                    "Name": "NavigationOnly",
-                                    "Group": {
-                                        "Mask": 18446744073709551614
-                                    }
-                                },
-                                {
-                                    "Id": {
-                                        "GroupId": "{ECFAD03A-4884-4EE1-A915-8844B7E354B3}"
-                                    },
-                                    "Name": "NavigationExcept",
-                                    "Group": {
-                                        "Mask": 18446744073709551613
-                                    }
                                 }
                             ]
                         }
spham-amzn commented 7 months ago

I don't know what process would modify this file. I only know of a script that may modify project.json, but even so, it will create a .bak file in the process. You are right, we shouldn't be modifying any source file in place.