Fixed several o3de python package and install layout issues (#1714)

* Updated the CrashLog directory path to save to the project user
directory instead of the engine-root directory

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Removing the custom OUTPUT_NAME for the Multiplayer Gem Builder target

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Changed the default value for the LY_3RDPARTY_PATH cache variable to be
~/.o3de/3rdParty

This simplifies the first time user experience when running cmake

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Fixed Windows only issue where using creating a VS Solution for an O3DE project on a different drive resulted in an unloaded "<drive letter>:" entry appearing in the solution explorer

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Refactored install layout logic for External Subdirectories

Instead of performing a regular expression over the Gems/* directory, now the each external subdirectory including the project (external) directory is recursied over and scanned for any folders that aren't excluded.
By default those folders are the [Cc]ache, [Bb]uild and [Uu]ser directories

Afterwards the list files to copy over are then split into a directory list and a file list that is filtered by an include regex
Next the directory list is iterated over and the directories are copied to the install layout
Finally the file list is iterated and the list of files are also copied to the install layout

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Fixed the o3de.bat script changing the working directory before running the o3de.py script

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Fixed exception in engine-template.py script when the create-gem command
is invoked with the --template-name parameter that does not correspond
to a registered Template

Updated the create-project command to register the project with the
o3de_manifest.json and the engine with the project as the final step

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Fixed register.py over-registration of non-engine directories when then
--this-engine parameter is supplied.
All the projects, gems and templates inside of the default o3de_manifest
folder locations were being registered with the engine that was being
registered

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Fixed register.py register_project_path() method to return 0 when the
project path is successfully registered.

This issue was that the save_o3de_manifest method "return" value was
being checked and that method doesn't actually return a value

Added question mark to the engine_template.py to correct text around
notifying the user if the project was registered

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Added doc comment on the new cmke ly_get_vs_folder_directory function()

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Added clarifying comment to the ly_setup_others() command logic to copy over directories and files from any external subdirectories(Gems) that are registered with the engine at the time of install

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Fixed infinite recursion when trying to create a template with the
source directory used to seed the template

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

Code/Editor/IEditorImpl.cpp

@@ -22,7 +22,9 @@ AZ_PUSH_DISABLE_WARNING(4251 4355 4996, "-Wunknown-warning-option")
 AZ_POP_DISABLE_WARNING
 
 // AzCore
+#include <AzCore/IO/Path/Path.h>
 #include <AzCore/JSON/document.h>
+#include <AzCore/Settings/SettingsRegistryMergeUtils.h>
 
 // AzFramework
 #include <AzFramework/Terrain/TerrainDataRequestBus.h>
@@ -195,8 +197,15 @@ CEditorImpl::CEditorImpl()
     m_pAssetBrowserRequestHandler = nullptr;
     m_assetEditorRequestsHandler = nullptr;
 
-    AZ::IO::SystemFile::CreateDir("SessionStatus");
-    QFile::setPermissions(m_crashLogFileName, QFileDevice::ReadOther | QFileDevice::WriteOther);
+    if (auto settingsRegistry = AZ::SettingsRegistry::Get(); settingsRegistry != nullptr)
+    {
+        if (AZ::IO::FixedMaxPath crashLogPath; settingsRegistry->Get(crashLogPath.Native(), AZ::SettingsRegistryMergeUtils::FilePathKey_ProjectUserPath))
+        {
+            crashLogPath /= m_crashLogFileName;
+            AZ::IO::SystemFile::CreateDir(crashLogPath.ParentPath().FixedMaxPathString().c_str());
+            QFile::setPermissions(crashLogPath.c_str(), QFileDevice::ReadOther | QFileDevice::WriteOther);
+        }
+    }
 }
 
 void CEditorImpl::Initialize()

Gems/Multiplayer/Code/CMakeLists.txt

@@ -107,7 +107,6 @@ if (PAL_TRAIT_BUILD_HOST_TOOLS)
     ly_add_target(
         NAME Multiplayer.Builders GEM_MODULE
         NAMESPACE Gem
-        OUTPUT_NAME Gem.Multiplayer.Builders
         FILES_CMAKE
             multiplayer_tools_files.cmake
         INCLUDE_DIRECTORIES

cmake/3rdParty.cmake

@@ -8,7 +8,24 @@
 # Do not overcomplicate searching for the 3rdParty path, if it is not easy to find,
 # the user should define it.
 
-set(LY_3RDPARTY_PATH "" CACHE PATH "Path to the 3rdParty folder")
+#! get_default_third_party_folder: Stores the default 3rdParty directory into the supplied output variable
+#
+# \arg:output_third_party_path name of variable to set the default project directory into
+# It defaults to the ~/.o3de/3rdParty directory
+function(get_default_third_party_folder output_third_party_path)
+    cmake_path(SET home_directory "$ENV{USERPROFILE}") # Windows
+    if(NOT EXISTS ${home_directory})
+        cmake_path(SET home_directory "$ENV{HOME}") # Unix
+        if (NOT EXISTS ${home_directory})
+            return()
+        endif()
+    endif()
+
+    set(${output_third_party_path} ${home_directory}/.o3de/3rdParty PARENT_SCOPE)
+endfunction()
+
+get_default_third_party_folder(o3de_default_third_party_path)
+set(LY_3RDPARTY_PATH "${o3de_default_third_party_path}" CACHE PATH "Path to the 3rdParty folder")
 
 if(LY_3RDPARTY_PATH)
     file(TO_CMAKE_PATH ${LY_3RDPARTY_PATH} LY_3RDPARTY_PATH)

cmake/LYWrappers.cmake

@@ -257,14 +257,7 @@ function(ly_add_target)
     # IDE organization
     ly_source_groups_from_folders("${ALLFILES}")
     source_group("Generated Files" REGULAR_EXPRESSION "(${CMAKE_BINARY_DIR})") # Any file coming from the output folder
-    file(RELATIVE_PATH project_path ${LY_ROOT_FOLDER} ${CMAKE_CURRENT_SOURCE_DIR})
-    # Visual Studio cannot load a project with a FOLDER that starts with a "../" relative path
-    # Strip away any leading ../ and then add a prefix of ExternalTargets/ as that in this scenario
-    # A relative directory with ../ would be outside of the Lumberyard Engine Root therefore it is external
-    set(ide_path ${project_path})
-    if (${project_path} MATCHES [[^(\.\./)+(.*)]])
-        set(ide_path "${CMAKE_MATCH_2}")
-    endif()
+    ly_get_vs_folder_directory(${CMAKE_CURRENT_SOURCE_DIR} ide_path)
     set_property(TARGET ${project_NAME} PROPERTY FOLDER ${ide_path})
 
 
@@ -639,3 +632,33 @@ function(ly_de_alias_target target_name output_variable_name)
     endif()
     set(${output_variable_name} ${de_aliased_target_name} PARENT_SCOPE)
 endfunction()
+
+#! ly_get_vs_folder_directory: Sets the Visual Studio folder name used for organizing vcxproj
+#  in the IDE
+#
+# Visual Studio cannot load projects that with a ".." relative path or contain a colon ":" as part of its FOLDER
+# Therefore if the .vcxproj is absolute, the drive letter must be removed from the folder name
+#
+# What this method does is first check if the target being added to the Visual Studio solution is within
+# the LY_ROOT_FOLDER(i.e is the LY_ROOT_FOLDER a prefix of the target source directory)
+# If it is a relative path to the target is used as the folder name
+# Otherwise the target directory would either 
+# 1. Be a path outside of the LY_ROOT_FOLDER on the same drive.
+#    In that case forming a relative path would cause it to start with ".." which will not work
+# 2. Be an path outside of the LY_ROOT_FOLDER on a different drive
+#    Here a relative path cannot be formed and therefore the path would start with "<drive>:/Path/To/Project"
+#    Which shows up as unloaded due to containing a colon
+# In this scenario the relative part of the path from the drive letter is used as the FOLDER name
+# to make sure the projects show up in the loaded .sln
+function(ly_get_vs_folder_directory absolute_target_source_dir output_source_dir)
+    # Get a relative directory to the LY_ROOT_FOLDER if possible for the Visual Studio solution hierarchy
+    # If a relative path cannot be formed, then retrieve a path with the drive letter stripped from it
+    cmake_path(IS_PREFIX LY_ROOT_FOLDER ${absolute_target_source_dir} is_target_prefix_of_engine_root)
+    if(is_target_prefix_of_engine_root)
+        cmake_path(RELATIVE_PATH absolute_target_source_dir BASE_DIRECTORY ${LY_ROOT_FOLDER} OUTPUT_VARIABLE relative_target_source_dir)
+    else()
+        cmake_path(GET absolute_target_source_dir RELATIVE_PART relative_target_source_dir)
+    endif()
+
+    set(${output_source_dir} ${relative_target_source_dir} PARENT_SCOPE)
+endfunction()

cmake/Platform/Common/Install_common.cmake

@@ -163,6 +163,7 @@ function(ly_setup_target OUTPUT_CONFIGURED_TARGET ALIAS_TARGET_NAME absolute_tar
         unset(RUNTIME_DEPENDENCIES_PLACEHOLDER)
     endif()
 
+
     get_target_property(inteface_build_dependencies_props ${TARGET_NAME} INTERFACE_LINK_LIBRARIES)
     unset(INTERFACE_BUILD_DEPENDENCIES_PLACEHOLDER)
     if(inteface_build_dependencies_props)
@@ -522,47 +523,75 @@ function(ly_setup_others)
         DESTINATION .
     )
 
-    # Gem Source Assets and Registry
+    # Gem Source Assets and configuration files
     # Find all gem directories relative to the CMake Source Dir
+
+    # This first loop is to filter out transient and .gitignore'd folders that should be added to
+    # the install layout from the root directory. Such as <external-subdirectory-root>/Cache.
+    # This is also done to avoid globbing thousands of files in subdirectories that shouldn't
+    # be processed.
+    foreach(gem_candidate_dir IN LISTS LY_EXTERNAL_SUBDIRS LY_PROJECTS)
+        file(REAL_PATH ${gem_candidate_dir} gem_candidate_dir BASE_DIRECTORY ${LY_ROOT_FOLDER})
+        # Don't recurse immediately in order to exclude transient source artifacts
+        file(GLOB
+            external_subdir_files
+            LIST_DIRECTORIES TRUE
+            "${gem_candidate_dir}/*"
+        )
+        # Exclude transient artifacts that shouldn't be copied to the install layout
+        list(FILTER external_subdir_files EXCLUDE REGEX "/([Bb]uild|[Cc]ache|[Uu]ser)$")
+        list(APPEND filtered_asset_paths ${external_subdir_files})
+    endforeach()
+
+    # At this point the filtered_assets_paths contains the list of all directories and files
+    # that are non-excluded candidates that can be scanned for target directories and files
+    # to copy over to the install layout
+    foreach(filtered_asset_path IN LISTS filtered_asset_paths)
+        if(IS_DIRECTORY ${filtered_asset_path})
             file(GLOB_RECURSE
-        gems_assets_path
+                recurse_assets_paths
                 LIST_DIRECTORIES TRUE
-        RELATIVE "${LY_ROOT_FOLDER}/"
-        "Gems/*"
+                "${filtered_asset_path}/*"
             )
-    list(FILTER gems_assets_path INCLUDE REGEX "/(Assets|Registry)$")
+            set(gem_file_paths ${recurse_assets_paths})
+            # Make sure to prepend the current path iteration to the gem_dirs_path to filter
+            set(gem_dir_paths ${filtered_asset_path} ${recurse_assets_paths})
+
+            # Gather directories to copy over
+            # Currently only the Assets, Registry and Config directories are copied over
+            list(FILTER gem_dir_paths INCLUDE REGEX "/(Assets|Registry|Config)$")
+            list(APPEND gems_assets_dir_path ${gem_dir_paths})
+        else()
+            set(gem_file_paths ${filtered_asset_path})
+        endif()
 
-    foreach (gem_assets_path ${gems_assets_path})
-        set(gem_abs_assets_path ${LY_ROOT_FOLDER}/${gem_assets_path}/)
-        if (EXISTS ${gem_abs_assets_path})
+        # Gather files to copy over
+        # Currently only the gem.json file is copied over
+        list(FILTER gem_file_paths INCLUDE REGEX "/(gem.json)$")
+        list(APPEND gems_assets_file_path "${gem_file_paths}")
+    endforeach()
+
+    # gem directories to install
+    foreach(gem_absolute_dir_path ${gems_assets_dir_path})
+        cmake_path(RELATIVE_PATH gem_absolute_dir_path BASE_DIRECTORY ${LY_ROOT_FOLDER} OUTPUT_VARIABLE gem_relative_dir_path)
+        if (EXISTS ${gem_absolute_dir_path})
             # The trailing slash is IMPORTANT here as that is needed to prevent
             # the "Assets" folder from being copied underneath the <gem-root>/Assets folder
-            install(DIRECTORY ${gem_abs_assets_path}
-                DESTINATION ${gem_assets_path}
+            install(DIRECTORY "${gem_absolute_dir_path}/"
+                DESTINATION ${gem_relative_dir_path}
             )
         endif()
     endforeach()
 
-    # gem.json files
-    file(GLOB_RECURSE
-        gems_json_path
-        LIST_DIRECTORIES FALSE
-        RELATIVE "${LY_ROOT_FOLDER}"
-        "Gems/*/gem.json"
-    )
-    foreach(gem_json_path ${gems_json_path})
-        get_filename_component(gem_relative_path ${gem_json_path} DIRECTORY)
-        install(FILES ${gem_json_path}
-            DESTINATION ${gem_relative_path}
+    # gem files to install
+    foreach(gem_absolute_file_path ${gems_assets_file_path})
+        cmake_path(RELATIVE_PATH gem_absolute_file_path BASE_DIRECTORY ${LY_ROOT_FOLDER} OUTPUT_VARIABLE gem_relative_file_path)
+        cmake_path(GET gem_relative_file_path PARENT_PATH gem_relative_parent_dir)
+        install(FILES ${gem_absolute_file_path}
+            DESTINATION ${gem_relative_parent_dir}
         )
     endforeach()
 
-    # Additional files needed by gems
-    install(DIRECTORY
-        ${LY_ROOT_FOLDER}/Gems/Atom/Asset/ImageProcessingAtom/Config
-        DESTINATION Gems/Atom/Asset/ImageProcessingAtom
-    )
-
     # Templates
     install(DIRECTORY
         ${LY_ROOT_FOLDER}/Templates

scripts/o3de.bat

@@ -9,7 +9,7 @@ REM
 pushd %~dp0%
 CD %~dp0..
 SET "BASE_PATH=%CD%"
-CD %~dp0
+popd
 SET "PYTHON_DIRECTORY=%BASE_PATH%\python"
 IF EXIST "%PYTHON_DIRECTORY%" GOTO pythonPathAvailable
 GOTO pythonDirNotFound
@@ -25,10 +25,8 @@ GOTO fail
 ECHO Python executable not found: %PYTHON_EXECUTABLE%
 GOTO fail
 :fail
-popd
 EXIT /b 1
 :end
-popd
 EXIT /b %ERRORLEVEL%
 
 

scripts/o3de/o3de/engine_template.py

@@ -17,7 +17,7 @@ import uuid
 import re
 
 
-from o3de import manifest, validation, utils
+from o3de import manifest, register, validation, utils
 
 logger = logging.getLogger()
 logging.basicConfig()
@@ -388,10 +388,11 @@ def create_template(source_path: pathlib.Path,
     if not source_path:
         logger.error('Src path cannot be empty.')
         return 1
-    if not os.path.isdir(source_path):
+    if not source_path.is_dir():
         logger.error(f'Src path {source_path} is not a folder.')
         return 1
 
+    source_path = source_path.resolve()
     # source_name is now the last component of the source_path
     if not source_name:
         source_name = os.path.basename(source_path)
@@ -401,11 +402,11 @@ def create_template(source_path: pathlib.Path,
     if not template_path:
         logger.info(f'Template path empty. Using source name {source_name}')
         template_path = source_name
-    if not os.path.isabs(template_path):
+    if not template_path.is_absolute():
         default_templates_folder = manifest.get_registered(default_folder='templates')
-        template_path = default_templates_folder/ template_path
+        template_path = default_templates_folder / template_path
         logger.info(f'Template path not a full path. Using default templates folder {template_path}')
-    if not force and os.path.isdir(template_path):
+    if not force and template_path.is_dir():
         logger.error(f'Template path {template_path} already exists.')
         return 1
 
@@ -415,8 +416,7 @@ def create_template(source_path: pathlib.Path,
     except ValueError:
         pass
     else:
-        logger.error(f'Template output path {template_path} cannot be a subdirectory of the source_path {source_path}:\n'
-                     f'{err}')
+        logger.error(f'Template output path {template_path} cannot be a subdirectory of the source_path {source_path}\n')
         return 1
 
     # template name is now the last component of the template_path
@@ -1200,7 +1200,7 @@ def create_from_template(destination_path: pathlib.Path,
 
     if not destination_name:
         # destination name is now the last component of the destination_path
-        destination_name = os.path.basename(destination_path)
+        destination_name = destination_path.name
 
     # destination name cannot be the same as a restricted platform name
     if destination_name in restricted_platforms:
@@ -1654,28 +1654,10 @@ def create_project(project_path: pathlib.Path,
                             d.write('# SPDX-License-Identifier: Apache-2.0 OR MIT\n')
                             d.write('# {END_LICENSE}\n')
 
-    # set the "engine" element of the project.json
-    engine_json_data = manifest.get_engine_json_data(engine_path=manifest.get_this_engine_path())
-    try:
-        engine_name = engine_json_data['engine_name']
-    except KeyError as e:
-        logger.error(f"engine_name for this engine not found in engine.json.")
-        return 1
 
-    project_json_data = manifest.get_project_json_data(project_path=project_path)
-    if not project_json_data:
-        # get_project_json_data already logs an error if the project.json is mising
-        return 1
-
-    project_json_data.update({"engine": engine_name})
-    with open(project_json, 'w') as s:
-        try:
-            s.write(json.dumps(project_json_data, indent=4) + '\n')
-        except OSError as e:
-            logger.error(f'Failed to write project json at {project_path}.')
-            return 1
-
-    return 0
+    # Register the project with the global o3de_manifest.json and set the project.json "engine" field to match the
+    # engine.json "engine_name" field
+    return register.register(project_path=project_path)
 
 
 def create_gem(gem_path: pathlib.Path,
@@ -1745,6 +1727,11 @@ def create_gem(gem_path: pathlib.Path,
     if template_name and not template_path:
         template_path = manifest.get_registered(template_name=template_name)
 
+    if not template_path:
+        logger.error(f'Could not find the template path using name {template_name}.\n'
+                     'Has the template been registered yet? It can be registered via the '
+                     '"o3de.py register --tp <template-path>" command')
+        return 1
     if not os.path.isdir(template_path):
         logger.error(f'Could not find the template {template_name}=>{template_path}')
         return 1
@@ -2068,7 +2055,7 @@ def _run_create_from_template(args: argparse) -> int:
     return create_from_template(args.destination_path,
                                 args.template_path,
                                 args.template_name,
-                                args.destination_path,
+                                args.destination_name,
                                 args.destination_restricted_path,
                                 args.destination_restricted_name,
                                 args.template_restricted_path,

scripts/o3de/o3de/manifest.py

@@ -194,9 +194,9 @@ def load_o3de_manifest(manifest_path: pathlib.Path = None) -> dict:
             return json_data
 
 
-def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> None:
+def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> bool:
     """
-        Save the json dictionary to the supplied manifest file or ~/.o3de/o3de_manifest.json if None
+        Save the json dictionary to the supplied manifest file or ~/.o3de/o3de_manifest.json if manifest_path is None
 
         :param json_data: dictionary to save in json format at the file path
         :param manifest_path: optional path to manifest file to save
@@ -206,8 +206,10 @@ def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> N
     with manifest_path.open('w') as s:
         try:
             s.write(json.dumps(json_data, indent=4) + '\n')
+            return True
         except OSError as e:
             logger.error(f'Manifest json failed to save: {str(e)}')
+            return False
 
 
 # Data query methods

scripts/o3de/o3de/register.py

@@ -425,7 +425,6 @@ def register_project_path(json_data: dict,
         if not manifest.save_o3de_manifest(project_json_data, project_json_path):
             return 1
 
-
     return 0
 
 
@@ -754,9 +753,6 @@ def _run_register(args: argparse) -> int:
         return repo.refresh_repos()
     elif args.this_engine:
         ret_val = register(engine_path=manifest.get_this_engine_path(), force=args.force)
-        error_code = register_shipped_engine_o3de_objects(force=args.force)
-        if error_code:
-            ret_val = error_code
         return ret_val
     elif args.all_engines_path:
         return register_all_engines_in_folder(args.all_engines_path, args.remove, args.force)