o3de / o3de-extras

Other
58 stars 60 forks source link

Make Multiplayer Template Usable Again #680

Closed ShaunaGordon closed 2 months ago

ShaunaGordon commented 3 months ago

What does this PR do?

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

  1. 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.
  2. 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:

  1. Download the Multiplayer template
  2. Create a new project using it
  3. It
    • Should not give Blast errors
    • Should build without error
    • Should run a Game, start the Game Server, and connect to it without error
ShaunaGordon commented 2 months ago

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 commented 2 months ago

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

jhanca-robotecai commented 2 months ago

@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 commented 2 months ago

@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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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