Make Multiplayer Template Usable Again #680

ShaunaGordon · 2024-04-04T16:25:06Z

What does this PR do?

We noticed that the Multiplayer template was in an unusable state. This fixes the two errors I ran into.

The Blast Gem had been moved to a now-stale Experimental branch. The template had included it as a dependency, causing Project Manager to throw up errors about it being missing. Removing the references to the Blast Gem fixes this.
Once past that, it wouldn't build at all, choking on an explicit template function call, despite the types matching. Removing the types in the call fixes this.

How was this PR tested?

Manual.

To test:

Download the Multiplayer template
Create a new project using it
It
- Should not give Blast errors
- Should build without error
- Should run a Game, start the Game Server, and connect to it without error

Signed-off-by: Shauna Gordon <shauna@shaunagordon.com>

Building a project with this template throws "you cannot bind an lvalue to an rvalue reference" error. Copilot suggests removing the template declaration in the call to fix this. Signed-off-by: Shauna Gordon <shauna@shaunagordon.com>

nick-l-o3de

I assume there are no blast assets it depended on?

nick-l-o3de · 2024-04-09T17:22:08Z

Templates/Multiplayer/Template/Gem/Code/Source/Spawners/RoundRobinSpawner.cpp

 * Copyright (c) Contributors to the Open 3D Engine Project. For complete copyright and license terms please see the LICENSE at the root of this distribution.
- * 
+ *
 * SPDX-License-Identifier: Apache-2.0 OR MIT
 *


Might be worth taking a look at this on github here and seeing all the whitespace-only changes... its not a big deal but it makes it cleaner if whitespace only changes are reverted.

I waffled a bit about that a bit, myself, but ended up erring on the side of leaving them, because it complies with the style guide (as enforced by the .editorconfig in the main repo). I can pull them out, though.

jhanca-robotecai

Thanks for the update!

I noticed one more problem with that template while testing which requires a fix:

diff --git a/Templates/Multiplayer/Template/Gem/Code/Source/Components/NetworkAiComponent.cpp b/Templates/Multiplayer/Template/Gem/Code/Source/Components/NetworkAiComponent.cpp
index d8a7cb49..1865666b 100644
--- a/Templates/Multiplayer/Template/Gem/Code/Source/Components/NetworkAiComponent.cpp
+++ b/Templates/Multiplayer/Template/Gem/Code/Source/Components/NetworkAiComponent.cpp
@@ -16,8 +16,6 @@
 
 namespace ${SanitizedCppName}
 {
-    constexpr static float SecondsToMs = 1000.f;
-
     NetworkAiComponentController::NetworkAiComponentController(NetworkAiComponent& parent)
         : NetworkAiComponentControllerBase(parent)
     {

The variable is not used and clang 14 on my Ubuntu 22.04 can clearly see that:

/devroot/projects/MultiplayerTemplate/Gem/Code/Source/Components/NetworkAiComponent.cpp:19:28: error: unused variable 'SecondsToMs' [-Werror,-Wunused-const-variable]
    constexpr static float SecondsToMs = 1000.f;

You may add it to this PR or I will add it in the following PR.

ShaunaGordon · 2024-04-11T16:09:59Z

I assume there are no blast assets it depended on?

Not that I saw. I tried to make sure I removed all the Blast references, since it's not available anymore.

ShaunaGordon · 2024-04-11T16:15:33Z

@jhanca-robotecai I can incorporate it, yeah, unless you've got a more targeted PR coming.

jhanca-robotecai · 2024-04-12T11:44:55Z

@jhanca-robotecai I can incorporate it, yeah, unless you've got a more targeted PR coming.

Feel free. We will trigger tests and merge this PR on Monday. I also keep it on my list for cherry-picking for the incoming point release.

ShaunaGordon · 2024-04-15T14:53:36Z

@jhanca-robotecai SecondsToMs does indeed get used, on lines 30 and 98.

It looks like your compiler resolves the #if AZ_TRAIT_SERVER to false (my VSCode on my Linux partition does as well), and thus it's technically not being used...but it is used under certain circumstances. This will probably be something we need to look deeper into and see if/when that does resolve to true and maybe make note of it and figure out a way to exclude it from error checking when in an environment that resolves to false.

jhanca-robotecai · 2024-04-16T10:47:17Z

@jhanca-robotecai SecondsToMs does indeed get used, on lines 30 and 98.

Thanks for pointing that out, I did not browse the code to be honest, only the compiler errors. I will create the issue.

amzn-changml · 2024-04-16T23:42:20Z

I ran a clean build in AR, which succeeds compilation but fails on one warning as error and few tests: https://jenkins.build.o3de.org/blue/organizations/jenkins/o3de-extras/detail/PR-680/7/pipeline

The warning as error looks like (added other warnings for context):

 D:/workspace/o3de-extras/Gems/OpenXRVk/Code/Source/OpenXRVkUtils.cpp(47,62): error C2220: the following warning is treated as an error [D:\workspace\o3de\build\windows\External\OpenXRVk-753c2f2e\Code\OpenXRVk.Static.vcxproj]
15:48:52        void PrintXrError(const char* windowName, const XrResult error, const char* fmt, ...)
15:48:52                                                                 ^
15:48:52  D:/workspace/o3de-extras/Gems/OpenXRVk/Code/Source/OpenXRVkUtils.cpp(47,62): warning C4100: 'error': unreferenced formal parameter [D:\workspace\o3de\build\windows\External\OpenXRVk-753c2f2e\Code\OpenXRVk.Static.vcxproj]
15:48:52  D:/workspace/o3de-extras/Gems/OpenXRVk/Code/Source/OpenXRVkUtils.cpp(47,35): warning C4100: 'windowName': unreferenced formal parameter [D:\workspace\o3de\build\windows\External\OpenXRVk-753c2f2e\Code\OpenXRVk.Static.vcxproj]
15:48:52        void PrintXrError(const char* windowName, const XrResult error, const char* fmt, ...)

The tests in question fail here:

[2024-04-16T23:07:28.687Z] The following tests FAILED:
[2024-04-16T23:07:28.687Z] 	 28 - AZ::SerializeContextTools.Tests.main::TEST_RUN (Not Run)
[2024-04-16T23:07:28.687Z] 	 29 - AZ::AssetBundler.Tests.main::TEST_RUN (Not Run)
[2024-04-16T23:07:28.687Z] Errors while running CTest

Digging deeper, I see the following errors:

[2024-04-16T22:59:39.130Z] Could not find executable D:/workspace/o3de/build/windows/bin/profile/SerializeContextTools.Tests.exe
[2024-04-16T22:59:39.130Z] Looked in the following places:
[2024-04-16T22:59:39.130Z] D:/workspace/o3de/build/windows/bin/profile/SerializeContextTools.Tests.exe
[2024-04-16T22:59:39.130Z] D:/workspace/o3de/build/windows/bin/profile/SerializeContextTools.Tests.exe.exe
[2024-04-16T22:59:39.130Z] D:/workspace/o3de/build/windows/bin/profile/profile/SerializeContextTools.Tests.exe
[2024-04-16T22:59:39.130Z] D:/workspace/o3de/build/windows/bin/profile/profile/SerializeContextTools.Tests.exe.exe
[2024-04-16T22:59:39.130Z] profile/D:/workspace/o3de/build/windows/bin/profile/SerializeContextTools.Tests.exe
[2024-04-16T22:59:39.130Z] profile/D:/workspace/o3de/build/windows/bin/profile/SerializeContextTools.Tests.exe.exe
[2024-04-16T22:59:39.130Z] Unable to find executable: D:/workspace/o3de/build/windows/bin/profile/SerializeContextTools.Tests.exe
[2024-04-16T22:59:39.130Z]  1/51 Test  #28: AZ::SerializeContextTools.Tests.main::TEST_RUN .............***Not Run   0.00 sec
[2024-04-16T22:59:39.130Z]       Start  29: AZ::AssetBundler.Tests.main::TEST_RUN
[2024-04-16T22:59:39.130Z] Could not find executable D:/workspace/o3de/build/windows/bin/profile/AssetBundler.Tests.exe
[2024-04-16T22:59:39.130Z] Looked in the following places:
[2024-04-16T22:59:39.130Z] D:/workspace/o3de/build/windows/bin/profile/AssetBundler.Tests.exe
[2024-04-16T22:59:39.130Z] D:/workspace/o3de/build/windows/bin/profile/AssetBundler.Tests.exe.exe
[2024-04-16T22:59:39.130Z] D:/workspace/o3de/build/windows/bin/profile/profile/AssetBundler.Tests.exe
[2024-04-16T22:59:39.130Z] D:/workspace/o3de/build/windows/bin/profile/profile/AssetBundler.Tests.exe.exe
[2024-04-16T22:59:39.130Z] profile/D:/workspace/o3de/build/windows/bin/profile/AssetBundler.Tests.exe
[2024-04-16T22:59:39.130Z] profile/D:/workspace/o3de/build/windows/bin/profile/AssetBundler.Tests.exe.exe
[2024-04-16T22:59:39.130Z] Unable to find executable: D:/workspace/o3de/build/windows/bin/profile/AssetBundler.Tests.exe
[2024-04-16T22:59:39.130Z]  2/51 Test  #29: AZ::AssetBundler.Tests.main::TEST_RUN ......................***Not Run   0.00 sec
[2024-04-16T22:59:39.130Z]       Start  33: test_commit_validation.smoke::TEST_RUN

This fails 100% of the time on clean or incremental builds. Somehow the serializer and assetbundler exes are not getting built from the development branch O3DE code

amzn-changml · 2024-04-16T23:44:57Z

We may want to investigate the above, but if we feel that this unrelated (which this might be given OpenXR and AssetBundler are not being changed here) and need to bypass AR to resolve, please thumbs-up this message to vote on override.

jhanca-robotecai · 2024-04-17T07:18:30Z

We may want to investigate the above, but if we feel that this unrelated (which this might be given OpenXR and AssetBundler are not being changed here) and need to bypass AR to resolve, please thumbs-up this message to vote on override.

Last changes in OpenXRVk Gem took place in late February:

jhanca@jhanca-buntu ~/devroot/o3de-extras/Gems [jh/remove_duplicated_textures]
± % git log -- OpenXRVk/ | head
commit 68a38ab3b79600f3c37fd4fd8988592c6d0b6a9e
Author: galibzon <66021303+galibzon@users.noreply.github.com>
Date:   Fri Feb 23 16:14:40 2024 -0600

    New OpenXR Spaces and Actions Interfaces (#673)
    
    * New OpenXR Spaces and Actions Interfaces
    
    This PR completes the work regarding the new
    APIs that helps applications to work with Spaces

and multiple PRs were pushed since then. Strangely, the tests passed, e.g. the last merged. I understand there might be some changes in o3de that caused problems with tests in extras, but I do not believe this is the main cause as the tests keep failing randomly in all PRs. The one linked earlier failed in the first run as well, it passed only in the second one.

amzn-changml · 2024-04-17T09:12:58Z

Sounds good, I'm in agreement with your assessment as it seems to be reported in other runs, @nick-l-o3de can you also verify the above as you were the second approver and sign off on the error being unrelated? I'll issue the bypass once we're all agreed.

amzn-changml · 2024-04-19T15:54:17Z

Discussed with @Ulrick28 and @nick-l-o3de. The changes are not related to this error. I'll start the bypass process now

o3de-issues-bot · 2024-04-19T15:58:02Z

Initial AR build failed https://jenkins.build.o3de.org/job/o3de-extras/job/PR-680/8/display/redirect.
Status check rule override by amzn-changml

AMZN-Gene · 2024-04-22T20:35:58Z

Templates/Multiplayer/Template/Gem/Code/Source/Spawners/RoundRobinSpawner.cpp

@@ -1,6 +1,6 @@
 /*


Ideally RoundRobinSpawner.h/cpp and Source/Spawners/IPlayerSpawner.h is deleted entirely, and removed from the cmake file list: https://github.com/o3de/o3de-extras/blob/development/Templates/Multiplayer/Template/Gem/Code/%24%7BNameLower%7D_files.cmake

Remove game code references to RoundRobinSpawner and IPlayerSpawner as well (I think there's references inside the Source/${Name}SystemComponent.cpp

RoundRobinSpawner is replaced by the built-in Multiplayer gem component called SimplePlayerSpawner.
https://www.docs.o3de.org/docs/user-guide/components/reference/multiplayer/simple-player-spawner/

You'd slap that component to the root level entity (whichever levels Multiplayer template ships with). The component takes the network prefab to spawn, and the player spawn points.

Thanks for your input, I created an issue based on that.

ShaunaGordon added 3 commits March 28, 2024 16:59

Remove references to the Blast gem

70658c0

Signed-off-by: Shauna Gordon <shauna@shaunagordon.com>

Remove template declaration per copilot suggestion.

10d2267

Building a project with this template throws "you cannot bind an lvalue to an rvalue reference" error. Copilot suggests removing the template declaration in the call to fix this. Signed-off-by: Shauna Gordon <shauna@shaunagordon.com>

Merge branch 'o3de:development' into MultiplayerTemplate

544ae55

ShaunaGordon requested a review from a team as a code owner April 4, 2024 16:25

nick-l-o3de approved these changes Apr 9, 2024

View reviewed changes

jhanca-robotecai approved these changes Apr 11, 2024

View reviewed changes

jhanca-robotecai mentioned this pull request Apr 16, 2024

Multiplayer Template possibly unused variable #684

Open

byrcolin added sig/content Categorizes an issue or PR as relevant to SIG Content. sig/network Categorizes an issue or PR as relevant to SIG Network. labels Apr 16, 2024

amzn-changml merged commit 938076c into o3de:development Apr 19, 2024
2 checks passed

jhanca-robotecai mentioned this pull request Apr 22, 2024

Remove duplicated textures #682

Merged

ShaunaGordon mentioned this pull request Apr 22, 2024

Multiplayer Template Requires Blast Gem #612

Closed

AMZN-Gene reviewed Apr 22, 2024

View reviewed changes

jhanca-robotecai mentioned this pull request Apr 24, 2024

Cherry-picks for 23103 point-release #690

Merged

jhanca-robotecai mentioned this pull request May 7, 2024

Refactor Multiplayer Template spawner #696

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Multiplayer Template Usable Again #680

Make Multiplayer Template Usable Again #680

ShaunaGordon commented Apr 4, 2024 •

edited

nick-l-o3de left a comment

nick-l-o3de Apr 9, 2024

ShaunaGordon Apr 11, 2024

jhanca-robotecai left a comment

ShaunaGordon commented Apr 11, 2024

ShaunaGordon commented Apr 11, 2024

jhanca-robotecai commented Apr 12, 2024

ShaunaGordon commented Apr 15, 2024 •

edited

jhanca-robotecai commented Apr 16, 2024

amzn-changml commented Apr 16, 2024

amzn-changml commented Apr 16, 2024 •

edited

jhanca-robotecai commented Apr 17, 2024

amzn-changml commented Apr 17, 2024 •

edited

amzn-changml commented Apr 19, 2024

o3de-issues-bot commented Apr 19, 2024

AMZN-Gene Apr 22, 2024

jhanca-robotecai May 7, 2024

Make Multiplayer Template Usable Again #680

Make Multiplayer Template Usable Again #680

Conversation

ShaunaGordon commented Apr 4, 2024 • edited

What does this PR do?

How was this PR tested?

nick-l-o3de left a comment

Choose a reason for hiding this comment

nick-l-o3de Apr 9, 2024

Choose a reason for hiding this comment

ShaunaGordon Apr 11, 2024

Choose a reason for hiding this comment

jhanca-robotecai left a comment

Choose a reason for hiding this comment

ShaunaGordon commented Apr 11, 2024

ShaunaGordon commented Apr 11, 2024

jhanca-robotecai commented Apr 12, 2024

ShaunaGordon commented Apr 15, 2024 • edited

jhanca-robotecai commented Apr 16, 2024

amzn-changml commented Apr 16, 2024

amzn-changml commented Apr 16, 2024 • edited

jhanca-robotecai commented Apr 17, 2024

amzn-changml commented Apr 17, 2024 • edited

amzn-changml commented Apr 19, 2024

o3de-issues-bot commented Apr 19, 2024

AMZN-Gene Apr 22, 2024

Choose a reason for hiding this comment

jhanca-robotecai May 7, 2024

Choose a reason for hiding this comment

ShaunaGordon commented Apr 4, 2024 •

edited

ShaunaGordon commented Apr 15, 2024 •

edited

amzn-changml commented Apr 16, 2024 •

edited

amzn-changml commented Apr 17, 2024 •

edited