Skip to content

fix: NetcodeIntegrationTest preserve player prefab #2275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Oct 24, 2022

This fixes an issue where the player prefab would get destroyed if you shutdown the NetworkManager assigned to the NetworkManager.Singleton (which is typically the server). This would occur with any test prefab created with NetcodeIntegrationTest.CreateNetworkObjectPrefab.

Testing and Documentation

  • No documentation changes or additions were necessary.
  • Includes Integration Test: PreserveIntegrationTestPrefabs

This fixes the issue where shutting down the server and the clients during a UnityTest would result in the player prefab getting deleted.
adding comments
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner October 24, 2022 20:52
This fix includes preserving any prefabs created with NetcodeIntegrationTest.CreateNetworkObjectPrefab
This validates the updates to NetcodeIntegrationTest

yield return WaitForClientsConnectedOrTimeOut();

CheckPrefabs();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be great if there could be a quick netvar sync or RPC call here, just to make sure those NMs are really connected together. I'm saying this cause we had silent fails in BR tests where NMs wouldn't connect and the tests would still pass (the connection failures would be outputed as warnings, ignored by our tests)

SamuelBellomo added a commit to Unity-Technologies/com.unity.multiplayer.samples.coop that referenced this pull request Oct 24, 2022
With latest develop fixes (see PR Unity-Technologies/com.unity.netcode.gameobjects#2275 ) this workaround shouldn't be needed anymore
@NoelStephensUnity NoelStephensUnity marked this pull request as draft October 25, 2022 03:08
LPLafontaineB pushed a commit to Unity-Technologies/com.unity.multiplayer.samples.coop that referenced this pull request Dec 9, 2022
With latest develop fixes (see PR Unity-Technologies/com.unity.netcode.gameobjects#2275 ) this workaround shouldn't be needed anymore
@0xFA11
Copy link
Contributor

0xFA11 commented Jan 3, 2023

is this still a relevant PR or should we close it? @NoelStephensUnity

@NoelStephensUnity
Copy link
Collaborator Author

is this still a relevant PR or should we close it? @NoelStephensUnity

I have been meaning to circle back to this... it still needs some work and is something that could be useful.
We can always keep the branch but close out the PR if needed.

@NoelStephensUnity
Copy link
Collaborator Author

Closing this PR as this integration testing feature should be looked into after some up-coming architectural changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants