RFC: changes in simulation Gems and Templates #94
Conversation
Signed-off-by: Jan Hanca <[email protected]>
jhanca-robotecai
changed the title
There was a problem hiding this comment.
I fully agree with the proposed changes. Currently, there is a mess in these gems and templates. Some assets are duplicated others are not used or needed anywhere. Also, the structure and naming of gems and templates may be confusing. So I fully support this initiative.
I've added a few comments to the RFC text. One additional general remark:
While working on these gems, they should be reviewed for unneeded and duplicated assets, confusing names, etc., and such issues should be corrected (if possible)
|
@jhanca-robotecai: great job BTW! :) |
Signed-off-by: Jan Hanca <[email protected]>
Signed-off-by: Jan Hanca <[email protected]>
Thank you for the review. I added your comments to the RFC. |
Signed-off-by: Jan Hanca <[email protected]>
There was a problem hiding this comment.
Don't really have any objections to a good reorg. We've found "adding more gems" can add a lot more complexity though. It might be worth considering moving all the reusable and sharable bits into a single gem so that they're all there - my understanding is that the engine doens't actually ship assets that it doesn't use in a level or project, when you actually make a release of your sim or game...
I'm not saying go this way, but consider the pros and cons?
And I'm talking mostly about asset gems, those that contain just models, prefabs, textures, you can add to your project.
Note also that asset gems do not need to contain any code at all, or any module or cpps or cmakes except to add the asset folders.
| All projects using `ProteusRobot` Gem will require an easy update: changing the required Gem name. All asset names (*prefabs*) and paths will remain unchanged, but not necessarily names and paths of each low-level asset (a mesh file or a texture). There is a risk that someone in some project uses these. Such project will require manual fixes or the old version of the Gem. | ||
|
|
There was a problem hiding this comment.
Why is this the case? As far as I know, O3DE generates UUIDs and paths based on the 'scan folder' root. Gems can add new scan folder roots via a config file, which means there's likely a way to make sure that at least the relative paths of all the assets (from the scan roots) remain the same. If you need help with this, I can help investigate.
There was a problem hiding this comment.
Thank you for the review and the comment. I guess we are on the same page with the engine, but I did not put it in the right words.
The paths to the prefabs will remain the same or they will be added to the config file that you mentioned. The same goes for the material files. However, we plan to rename some of the texture files. It is very unlikely someone created their own material file and linked it with the textures from this repo (also due to the problem in the cmake that was recently discovered by us when trying to do the same: o3de/o3de-extras#837 ). Nevertheless, I wanted to point out such possibility in the RFC.
An example for the misleading name within the repository that we want to change:
Warehouse/Textures/WarehouseProps/WarehousePropsMaskMap_A.png
this will be renamed to WarehouseModules_roughness.png (files with _property suffix are discovered automatically by O3DE).
This reorganization will create a new Gem, but it will remove multiple other Gems at the same time. We are trying to find the right balance between downloading obsolete data (not everyone needs all assets) and having to use too many Gems. If I was making decisions, I would remove most of the assets stored within the |
|
There is a full list of changes that need to be implemented created based on the RFC as of today. Some of the tasks rely on each other.
|
|
1 and 2 solved by: 3 and 4 solved by: 5 solved by: 6 partially solved by: 7 solved by: 9 solved by: |
|
6 solved by: 10 solved by: |
|
8 solved by: |
This PR submits an RFC with changes proposed to most of the simulation-related Gems and Templates.