From 93e267345fb2fb8954ec090e25296dc7c625c98e Mon Sep 17 00:00:00 2001
From: puvvadar <puvvadar@amazon.com>
Date: Mon, 17 May 2021 18:46:21 -0700
Subject: [PATCH] Address string/mem feedback plus some misc cleanup

---
 .../Multiplayer/MultiplayerConstants.h        | 10 ++---
 .../Editor/MultiplayerEditorConnection.cpp    |  7 ++--
 .../Source/Editor/MultiplayerEditorGem.cpp    | 12 +++---
 .../MultiplayerEditorSystemComponent.cpp      | 38 ++++++++-----------
 .../Editor/MultiplayerEditorSystemComponent.h |  4 +-
 .../Source/MultiplayerSystemComponent.cpp     | 20 +++++-----
 .../Pipeline/NetworkPrefabProcessor.cpp       | 12 +++---
 7 files changed, 49 insertions(+), 54 deletions(-)

diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/MultiplayerConstants.h b/Gems/Multiplayer/Code/Include/Multiplayer/MultiplayerConstants.h
index 892691177a..b82fab91be 100644
--- a/Gems/Multiplayer/Code/Include/Multiplayer/MultiplayerConstants.h
+++ b/Gems/Multiplayer/Code/Include/Multiplayer/MultiplayerConstants.h
@@ -21,12 +21,12 @@
 
 namespace Multiplayer
 {
-    static constexpr AZStd::string_view MPNetworkInterfaceName("MultiplayerNetworkInterface");
-    static constexpr AZStd::string_view MPEditorInterfaceName("MultiplayerEditorNetworkInterface");
+    constexpr AZStd::string_view MPNetworkInterfaceName("MultiplayerNetworkInterface");
+    constexpr AZStd::string_view MPEditorInterfaceName("MultiplayerEditorNetworkInterface");
 
-    static constexpr AZStd::string_view LocalHost("127.0.0.1");
-    static constexpr uint16_t DefaultServerPort = 30090;
-    static constexpr uint16_t DefaultServerEditorPort = 30091;
+    constexpr AZStd::string_view LocalHost("127.0.0.1");
+    constexpr uint16_t DefaultServerPort = 30090;
+    constexpr uint16_t DefaultServerEditorPort = 30091;
 
 }
 
diff --git a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp
index 2fdd29c542..f88a314ea5 100644
--- a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp
+++ b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp
@@ -12,16 +12,17 @@
 
 #include <Multiplayer/IMultiplayer.h>
 #include <Multiplayer/MultiplayerConstants.h>
-#include <Source/Editor/MultiplayerEditorConnection.h>
+#include <Editor/MultiplayerEditorConnection.h>
 #include <Source/AutoGen/AutoComponentTypes.h>
-#include <AzNetworking/ConnectionLayer/IConnection.h>
-#include <AzNetworking/Framework/INetworking.h>
+
 #include <AzCore/Asset/AssetManager.h>
 #include <AzCore/Interface/Interface.h>
 #include <AzCore/Utils/Utils.h>
 #include <AzCore/Serialization/SerializeContext.h>
 #include <AzCore/Serialization/Utils.h>
 #include <AzFramework/Spawnable/Spawnable.h>
+#include <AzNetworking/ConnectionLayer/IConnection.h>
+#include <AzNetworking/Framework/INetworking.h>
 
 namespace Multiplayer
 {
diff --git a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorGem.cpp b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorGem.cpp
index ae38ef1d6d..2fe0aabe5f 100644
--- a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorGem.cpp
+++ b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorGem.cpp
@@ -10,13 +10,13 @@
 *
 */
 
-#include <Source/Multiplayer_precompiled.h>
-#include <Source/MultiplayerGem.h>
-#include <Source/MultiplayerSystemComponent.h>
-#include <Source/Editor/MultiplayerEditorGem.h>
-#include <AzNetworking/Framework/NetworkingSystemComponent.h>
+#include <Multiplayer_precompiled.h>
+#include <MultiplayerGem.h>
+#include <MultiplayerSystemComponent.h>
+#include <Editor/MultiplayerEditorGem.h>
+#include <Editor/MultiplayerEditorSystemComponent.h>
 
-#include <Source/Editor/MultiplayerEditorSystemComponent.h>
+#include <AzNetworking/Framework/NetworkingSystemComponent.h>
 
 namespace Multiplayer
 {
diff --git a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorSystemComponent.cpp b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorSystemComponent.cpp
index d837b15b05..159f3b944b 100644
--- a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorSystemComponent.cpp
+++ b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorSystemComponent.cpp
@@ -13,13 +13,15 @@
 #include <Multiplayer/IMultiplayer.h>
 #include <Multiplayer/IMultiplayerTools.h>
 #include <Multiplayer/MultiplayerConstants.h>
+
+#include <MultiplayerSystemComponent.h>
+#include <Editor/MultiplayerEditorSystemComponent.h>
 #include <Source/AutoGen/Multiplayer.AutoPackets.h>
-#include <Source/MultiplayerSystemComponent.h>
-#include <Source/Editor/MultiplayerEditorSystemComponent.h>
-#include <AzCore/Interface/Interface.h>
+
 #include <AzCore/Component/ComponentApplicationBus.h>
 #include <AzCore/Console/IConsole.h>
 #include <AzCore/Console/ILogger.h>
+#include <AzCore/Interface/Interface.h>
 #include <AzCore/Serialization/SerializeContext.h>
 #include <AzCore/Utils/Utils.h>
 #include <AzNetworking/Framework/INetworking.h>
@@ -31,11 +33,11 @@ namespace Multiplayer
 
     AZ_CVAR(bool, editorsv_enabled, true, nullptr, AZ::ConsoleFunctorFlags::DontReplicate,
         "Whether Editor launching a local server to connect to is supported");
-    AZ_CVAR(bool, editorsv_launch, false, nullptr, AZ::ConsoleFunctorFlags::DontReplicate,
+    AZ_CVAR(bool, editorsv_launch, true, nullptr, AZ::ConsoleFunctorFlags::DontReplicate,
         "Whether Editor should launch a server when the server address is localhost");
     AZ_CVAR(AZ::CVarFixedString, editorsv_process, "", nullptr, AZ::ConsoleFunctorFlags::DontReplicate,
         "The server executable that should be run. Empty to use the current project's ServerLauncher");
-    AZ_CVAR(AZ::CVarFixedString, editorsv_serveraddr, LocalHost.data(), nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "The address of the server to connect to");
+    AZ_CVAR(AZ::CVarFixedString, editorsv_serveraddr, AZ::CVarFixedString(LocalHost), nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "The address of the server to connect to");
     AZ_CVAR(uint16_t, editorsv_port, DefaultServerEditorPort, nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "The port that the multiplayer editor gem will bind to for traffic");
 
     void MultiplayerEditorSystemComponent::Reflect(AZ::ReflectContext* context)
@@ -115,34 +117,24 @@ namespace Multiplayer
         }
     }
 
-    void LaunchEditorServer(AzFramework::ProcessWatcher* outProcess)
+    AzFramework::ProcessWatcher* LaunchEditorServer()
     {
         // Assemble the server's path
         AZ::CVarFixedString serverProcess = editorsv_process;
+        AZ::IO::FixedMaxPath serverPath;
         if (serverProcess.empty())
         {
             // If enabled but no process name is supplied, try this project's ServerLauncher
             serverProcess = AZ::Utils::GetProjectName() + ".ServerLauncher";
-        }
 
-        AZ::IO::FixedMaxPathString serverPath = AZ::Utils::GetExecutableDirectory();
-        if (!serverProcess.contains(AZ_TRAIT_OS_PATH_SEPARATOR))
-        {
-            // If only the process name is specified, append that as well
-            serverPath.append(AZ_TRAIT_OS_PATH_SEPARATOR + serverProcess);
+            serverPath = AZ::Utils::GetExecutableDirectory();
+            serverPath /= serverProcess + AZ_TRAIT_OS_EXECUTABLE_EXTENSION;
         }
         else
         {
-            // If any path was already specified, then simply assign
             serverPath = serverProcess;
         }
 
-        if (!serverProcess.ends_with(AZ_TRAIT_OS_EXECUTABLE_EXTENSION))
-        {
-            // Add this platform's exe extension if it's not specified
-            serverPath.append(AZ_TRAIT_OS_EXECUTABLE_EXTENSION);
-        }
-
         // Start the configured server if it's available
         AzFramework::ProcessLauncher::ProcessLaunchInfo processLaunchInfo;
         processLaunchInfo.m_commandlineParameters = AZStd::string::format("\"%s\" --editorsv_isDedicated true", serverPath.c_str());
@@ -150,9 +142,11 @@ namespace Multiplayer
         processLaunchInfo.m_processPriority = AzFramework::ProcessPriority::PROCESSPRIORITY_NORMAL;
 
         // Launch the Server and give it a few seconds to boot up
-        outProcess = AzFramework::ProcessWatcher::LaunchProcess(
+        AzFramework::ProcessWatcher* outProcess = AzFramework::ProcessWatcher::LaunchProcess(
             processLaunchInfo, AzFramework::ProcessCommunicationType::COMMUNICATOR_TYPE_NONE);
         AZStd::this_thread::sleep_for(AZStd::chrono::milliseconds(15000));
+
+        return outProcess;
     }
 
     void MultiplayerEditorSystemComponent::OnGameEntitiesStarted()
@@ -188,9 +182,9 @@ namespace Multiplayer
             }
 
             const AZ::CVarFixedString remoteAddress = editorsv_serveraddr;
-            if (editorsv_launch && LocalHost.compare(remoteAddress.c_str()) == 0)
+            if (editorsv_launch && LocalHost == remoteAddress)
             {
-                LaunchEditorServer(m_serverProcess);
+                m_serverProcess = LaunchEditorServer();
             }
 
             // Now that the server has launched, attempt to connect the NetworkInterface         
diff --git a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorSystemComponent.h b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorSystemComponent.h
index 569092e981..81b138c675 100644
--- a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorSystemComponent.h
+++ b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorSystemComponent.h
@@ -14,18 +14,16 @@
 
 #include <IEditor.h>
 
-#include <Source/Editor/MultiplayerEditorConnection.h>
+#include <Editor/MultiplayerEditorConnection.h>
 
 #include <AzCore/Component/Component.h>
 #include <AzCore/Component/TickBus.h>
 #include <AzCore/Console/IConsole.h>
 #include <AzCore/Console/ILogger.h>
-
 #include <AzFramework/Entity/GameEntityContextBus.h>
 #include <AzFramework/Process/ProcessWatcher.h>
 #include <AzToolsFramework/Entity/EditorEntityContextBus.h>
 
-
 namespace AzNetworking
 {
     class INetworkInterface;
diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp
index c6a8458eae..aa6fe6e72c 100644
--- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp
+++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp
@@ -11,15 +11,16 @@
  */
 
 #include <Multiplayer/MultiplayerConstants.h>
-#include <Source/MultiplayerSystemComponent.h>
-#include <Source/AutoGen/AutoComponentTypes.h>
-#include <Source/ConnectionData/ClientToServerConnectionData.h>
-#include <Source/ConnectionData/ServerToClientConnectionData.h>
-#include <Source/ReplicationWindows/NullReplicationWindow.h>
-#include <Source/ReplicationWindows/ServerToClientReplicationWindow.h>
-#include <Source/EntityDomains/FullOwnershipEntityDomain.h>
 #include <Multiplayer/Components/MultiplayerComponent.h>
-#include <AzNetworking/Framework/INetworking.h>
+
+#include <MultiplayerSystemComponent.h>
+#include <ConnectionData/ClientToServerConnectionData.h>
+#include <ConnectionData/ServerToClientConnectionData.h>
+#include <EntityDomains/FullOwnershipEntityDomain.h>
+#include <ReplicationWindows/NullReplicationWindow.h>
+#include <ReplicationWindows/ServerToClientReplicationWindow.h>
+#include <Source/AutoGen/AutoComponentTypes.h>
+
 #include <AzCore/Serialization/SerializeContext.h>
 #include <AzCore/Serialization/Utils.h>
 #include <AzCore/Interface/Interface.h>
@@ -29,6 +30,7 @@
 #include <AzCore/Asset/AssetManagerBus.h>
 #include <AzCore/Utils/Utils.h>
 #include <AzFramework/Spawnable/Spawnable.h>
+#include <AzNetworking/Framework/INetworking.h>
 
 namespace AZ::ConsoleTypeHelpers
 {
@@ -63,7 +65,7 @@ namespace Multiplayer
     using namespace AzNetworking;
 
     AZ_CVAR(uint16_t, cl_clientport, 0, nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "The port to bind to for game traffic when connecting to a remote host, a value of 0 will select any available port");
-    AZ_CVAR(AZ::CVarFixedString, cl_serveraddr, LocalHost.data(), nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "The address of the remote server or host to connect to");
+    AZ_CVAR(AZ::CVarFixedString, cl_serveraddr, AZ::CVarFixedString(LocalHost), nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "The address of the remote server or host to connect to");
     AZ_CVAR(AZ::CVarFixedString, cl_serverpassword, "", nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "Optional server password");
     AZ_CVAR(uint16_t, cl_serverport, DefaultServerPort, nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "The port of the remote host to connect to for game traffic");
     AZ_CVAR(uint16_t, sv_port, DefaultServerPort, nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "The port that this multiplayer gem will bind to for game traffic");
diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp
index 8bea44e893..bd6899aad6 100644
--- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp
+++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp
@@ -10,7 +10,11 @@
  *
  */
 
-#include <Source/Pipeline/NetworkPrefabProcessor.h>
+#include <Multiplayer/IMultiplayerTools.h>
+#include <Multiplayer/Components/NetBindComponent.h>
+#include <Pipeline/NetBindMarkerComponent.h>
+#include <Pipeline/NetworkPrefabProcessor.h>
+#include <Pipeline/NetworkSpawnableHolderComponent.h>
 
 #include <AzCore/Serialization/Utils.h>
 #include <AzFramework/Components/TransformComponent.h>
@@ -18,10 +22,6 @@
 #include <AzToolsFramework/Prefab/Instance/Instance.h>
 #include <AzToolsFramework/Prefab/PrefabDomUtils.h>
 #include <Prefab/Spawnable/SpawnableUtils.h>
-#include <Multiplayer/IMultiplayerTools.h>
-#include <Multiplayer/Components/NetBindComponent.h>
-#include <Source/Pipeline/NetBindMarkerComponent.h>
-#include <Source/Pipeline/NetworkSpawnableHolderComponent.h>
 
 namespace Multiplayer
 {
@@ -40,7 +40,7 @@ namespace Multiplayer
             ProcessPrefab(context, prefabName, prefab);
         });
 
-        if (mpTools && context.GetProcessedObjects().size() > 0)
+        if (mpTools && !context.GetProcessedObjects().empty())
         {
             mpTools->SetDidProcessNetworkPrefabs(true);
         }