Skip to content

fix: unique handlers per rpc method #1694

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

Merged
merged 28 commits into from
Feb 14, 2022
Merged

fix: unique handlers per rpc method #1694

merged 28 commits into from
Feb 14, 2022

Conversation

0xFA11
Copy link
Contributor

@0xFA11 0xFA11 commented Feb 14, 2022

MTT-1703

as given an example/test-case in this PR with OnSendGlobalCounterClientRpc method, overloading rpc methods was not possible because rpc handlers always took their name from their source rpc method, which were identical even though overloaded.

with this PR, rpc handlers get their names from source rpc method's hash which also includes full params info and that makes them unique therefore rpc handler names are unique too.

(also updated changelog, implemented minimal test to verify the fix)

NoelStephensUnity and others added 27 commits February 3, 2022 18:44
commit 36c52d7
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 22:39:30 2022 -0600

    style

    arrg!!!!!! white space

commit 861ef54
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 19:51:09 2022 -0600

    fix and update

    fixed issue with WhenListContainsManyLargeValues_OverflowExceptionIsNotThrown count comparison value.

    Updated removed FixedString32Test by removing  yield return that was not needed.

commit 3e70580
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 19:41:57 2022 -0600

    update

    Reviewing the updated methods and making some minor adjustments.

commit ba3e09d
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 19:19:15 2022 -0600

    style

    removing a using statement for a namespace that was not being used.

commit 3088309
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 19:16:51 2022 -0600

    fix

    The AllNetworkVariableTypes was not properly creating the NetworkManager based on the value of useHost

commit 0f9d8ca
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 18:56:08 2022 -0600

    update

    removing more MultiInstanceHelpers.Run calls

commit 38ec0fe
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 18:48:47 2022 -0600

    update

    removing MultiInstanceHelpers.Run calls from NetworkTransform as they are not needed.

commit f8fd60c
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 18:32:53 2022 -0600

    test update and style

    removing some of the Run calls and beginning to convert over some of the problematic tests to  just yield return the conditional check itself to avoid potential timing issues when checking the condition and yielding to the current test group's newly created coroutine.

commit 71a4d4e
Merge: 994d459 27e8498
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 17:56:21 2022 -0600

    Merge branch 'develop' into fix/console-tests-failing

commit 994d459
Merge: 35b47a5 74df5b2
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 17:29:17 2022 -0600

    Merge branch 'develop' into fix/console-tests-failing

commit 35b47a5
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 17:29:07 2022 -0600

    test refactor

    This update collapses all of the time out code into the BaseMultiInstanceTest with an added method WaitForConditionOrTimeOut that leverages from an added TimeOutHelper class.

commit 73340d6
Merge: feb9e47 096f614
Author: Noel Stephens <noel.stephens@unity3d.com>
Date:   Wed Feb 2 02:59:21 2022 -0600

    Merge branch 'develop' into fix/console-tests-failing

commit feb9e47
Merge: a66d4f3 e07450f
Author: Noel Stephens <noel.stephens@unity3d.com>
Date:   Tue Feb 1 16:14:06 2022 -0600

    Merge branch 'develop' into fix/console-tests-failing

commit a66d4f3
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 16:13:53 2022 -0600

    style

    removing whitespace.

commit 02e4681
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 13:50:30 2022 -0600

    style

    making sure NetworkTransform was exactly as it was before, removing a LF/CR

commit 33e6601
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 13:49:13 2022 -0600

    refactor

    Came up with what I hope is a much better fix to get NetworkTransformTests passing on all consoles.  Passes on a windows stand alone test runner build...will need to run this on the consoles to verify the fix.

commit 91d8f31
Merge: 72d3d49 ef257a9
Author: Noel Stephens <noel.stephens@unity3d.com>
Date:   Tue Feb 1 09:29:42 2022 -0600

    Merge branch 'develop' into fix/console-tests-failing

commit 72d3d49
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Mon Jan 31 12:46:02 2022 -0600

    refactor

    removing the timeout code to reduce confusion on the changes to NetworkVariableTests.

commit 82d243f
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Mon Jan 31 11:38:44 2022 -0600

    revert

    re-applying readonly to the m_TestWithClientNetworkTransform and m_TestWithHost properties as that was changed during troubleshooting.

commit 3a90687
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Mon Jan 31 11:34:56 2022 -0600

    refactor

    moving the HasTimedOut check to the end of each while loop to assure that any possible edge case scenario where it finishes just as it would be timing out is not missed.  An example would be the condition being checked is validated but it lands right on the timeout period.  The test would have been a success, but timedOut would have been set too.

commit 71ae813
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Mon Jan 31 11:23:24 2022 -0600

    refactor

    using the bool as opposed to direct method call for the while loops.

commit 96b62b4
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Mon Jan 31 10:37:07 2022 -0600

    update

    Applied suggestion by Fatih to use deltaTime with normal Update selected and fixedDeltaTime for FixedUpdate selected.

commit 90b9ed9
Merge: d509fca c17f5b8
Author: Noel Stephens <noel.stephens@unity3d.com>
Date:   Sat Jan 29 15:09:31 2022 -0600

    Merge branch 'develop' into fix/console-tests-failing

commit d509fca
Merge: 8244627 58f194d
Author: Noel Stephens <noel.stephens@unity3d.com>
Date:   Fri Jan 28 17:17:15 2022 -0600

    Merge branch 'develop' into fix/console-tests-failing

commit 8244627
Merge: 43e9382 402210e
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Fri Jan 28 13:18:33 2022 -0600

    Merge branch 'fix/console-tests-failing' of https://github.com/Unity-Technologies/com.unity.netcode.gameobjects into fix/console-tests-failing

commit 43e9382
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Fri Jan 28 13:18:12 2022 -0600

    update

    Removing the auto-assignment of the NetworkTransformTests's constructor's properties that were left over from troubleshooting.

commit 402210e
Merge: 554bf0d e4108e5
Author: Noel Stephens <noel.stephens@unity3d.com>
Date:   Fri Jan 28 13:14:11 2022 -0600

    Merge branch 'develop' into fix/console-tests-failing

commit 554bf0d
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Fri Jan 28 09:06:52 2022 -0600

    style

    removing a portion of the comment as it was a copy-paste of code used in other fixes.

commit 7766f27
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Fri Jan 28 02:37:06 2022 -0600

    fix

    delta read was failing.

commit 041e10d
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Fri Jan 28 01:39:59 2022 -0600

    fix

    This includes updates to the NetworkObjectOnSpawnTests in order to get it passing in stand alone test runner builds in order to get it to pass on consoles.

commit 26d2335
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Fri Jan 28 01:16:37 2022 -0600

    style

    white space missing.

commit 5b6146a
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Fri Jan 28 01:16:08 2022 -0600

    fix

    This includes the fixes for getting NetworkVarBufferCopyTest to run on a stand alone test runner build.

commit 0e4435b
Merge: 9628212 5032214
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Jan 27 20:41:58 2022 -0600

    Merge branch 'develop' into fix/console-tests-failing

commit 9628212
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Jan 27 20:41:44 2022 -0600

    style

    Removed unused using statement.

commit c022e08
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Jan 27 20:15:02 2022 -0600

    fix - partial

    This is a "quasi-fix" where I had to add the ability to run NetworkTransform in the FixedUpdate in order to get the NetworkTransform tests all passing when running in a stand alone test runner build.

commit ae1c750
Merge: 859d88c 1704918
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Jan 27 14:50:38 2022 -0600

    Merge branch 'develop' into fix/console-tests-failing

commit 859d88c
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Jan 27 14:45:56 2022 -0600

    style

    Just added some additional comments.

commit 4b40c04
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Jan 27 14:31:20 2022 -0600

    fix

    This should fix the timing issues on consoles for the NetworkVariableTests.
Fixes and  minor modifications due to some differences between develop and v1.1.0
weird merge issue fix
fixing a few more tests that failed on some of the console platforms.
Still running into timing issues during transport tests.  Bumped the wait up to 2s and had to adjust FilledSendQueueMultipleSends to wait for the packet to be received before proceeding to send the next (this could probably be handled better but just putting a place holder fix in place for transport issues so I can confirm this the NGO runtime tests are all passing and for desktop server to see what I had to do to get it passing on some of the platforms under v1.1.0).

Also had to update the rest of the NetworkVariable tests to use the newer WaitForConditionOrTimeOut method.
Putting System.Linq reference back as even though it shows it is not used...it is.
Realized the fixes for this were not backported to v1.0.0.
This is a temporary back port for my branch to get past the SendMaximumPayloadSize failures
This fixes the issue with the standards checking of data.ToArray().
Finally looked at the very top of the TransportTests and realized they should be disabled for consoles.
commit 455a756
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Feb 3 12:19:02 2022 -0600

    Revert "test"

    This reverts commit a2f6fca.

commit 1c2d493
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Feb 3 12:17:02 2022 -0600

    style

    white space update

commit ce84aba
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Feb 3 11:34:22 2022 -0600

    test update

    This test was failing on some of the console platforms.  Made adjustments to avoid timing issues.

commit 8ad9b42
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Feb 3 07:53:49 2022 -0600

    fix

    This is a work around for the MTT-1703 issue with RPCs.

commit 85fded2
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Thu Feb 3 05:40:31 2022 -0600

    style

    white spaces.

commit 82606ea
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 22:31:35 2022 -0600

    test update

    Still trying to get rid of IL2CPP error

commit 5915c3e
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 20:13:45 2022 -0600

    test fix

    Trying to figure out weird ILCPP serialization issue.

commit 091d82f
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 18:01:13 2022 -0600

    test refactor

    Needed some MULTIPLAYER_TOOLS helpers that I accidentally excluded.

commit a2f6fca
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 12:03:10 2022 -0600

    test

    disabling all tests in Unity.Netcode.RuntimeTest

commit 5319473
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 12:02:28 2022 -0600

    revert

    reverting as this was a dumb approach.

commit 312a5f5
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 11:19:59 2022 -0600

    test

    Seeing if this fixes the weird issue with the standards failing.

commit 76af4d4
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 09:08:26 2022 -0600

    test

    turning off the Unity.Netcode.RuntimeTests on console platforms temporarily in order to run just the TestProject.RuntimeTests on consoles.

commit 67566d6
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Wed Feb 2 09:06:27 2022 -0600

    refactor

    Removing assembly references not being used.

commit 2480daf
Merge: bee18d3 096f614
Author: Noel Stephens <noel.stephens@unity3d.com>
Date:   Tue Feb 1 20:36:59 2022 -0600

    Merge branch 'develop' into fix/test-roject-runtimetests-standalone-testrunner-build

commit bee18d3
Merge: 2fa12fa e07450f
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 19:28:11 2022 -0600

    Merge branch 'develop' into fix/test-roject-runtimetests-standalone-testrunner-build

commit 2fa12fa
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 19:05:18 2022 -0600

    fix

    This fixes the issues with running the NetworkVariableInitializationOnNetworkSpawnTest when it was run in a stand alone test runner build which should be console friendly at this point.

commit c47ea65
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 19:04:03 2022 -0600

    fix

    Fixing issues with the NetworkPrefabPoolAdditive class that is used in several tests including the NetworkSceneManagerTests.  The issues fixed were related to object references not being around (i.e. NetworkManager) as well as came up with a better way to detect if the coroutine was running in order to determine if it needed to be stopped.

commit 1ee1297
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 17:56:57 2022 -0600

    fix

    The following updates to NetworkSceneManagerDDOLTests in NetworkSceneManagerTests.cs get it passing in a stand alone test runner build which should make it console friendly.

commit 0b955fe
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 17:44:06 2022 -0600

    fix

    This fixes the issues with running NestedNetworkManagerTests in a stand alone test runner build and should run on consoles.

commit 2f18881
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 17:30:41 2022 -0600

    fix

    The following updates to the MessageOrdering tests should get them working on consoles.

commit a967264
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 17:04:25 2022 -0600

    update

    Modifying the TestProject.RuntimeTests assembly to be a runtime only assembly.
    Had to migrate the TestProject.RuntimeTests out of the #if UNITY_EDITOR region in order to be able to compile a stand alone test runner build.

commit d8e5b1f
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 12:22:06 2022 -0600

    refactor

    This commit changes the NetworkObjectParentingTests and ReparentingCubeNetBhv namespace to TestProject.RuntimeTests and uses the standard SceneManager to load the scene as opposed to the editor version.

commit d5efbe6
Author: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>
Date:   Tue Feb 1 12:14:48 2022 -0600

    refactor

    This commit migrates the NetworkPrefabGlobalObjectIdHashTests and NetworkObjectGlobalObjectIdHashTests into the TestProject.EditorTests assembly since they depend upon editor specific library calls and could be run as EditMode tests.
NetworkVariableInitOnNetworkSpawn needed to be updated.
disabling tests that were specific to snapshot.
…-platforms-v1.1.0' into test/all-tests-pass-on-all-consoles-v1.0.0
Account for tick rounding error.
fixes a mistake I made in the IndexOf test
indent
Added the ConditionalPredicateBase implementation of the IConditionalPredicate and implemented an overloaded version of WaitForConditionOrTimeOut.
Changed the original WaitForConditionOrTimeOut to just return true as all use cases of these UnityTest attribute decorated methods had access to the values and were relatively simple conditions.

I used the NetworkVariableTests as an example of how to use the a ConditionalPredicateBase derived class to simplify the scenario where you might be re-using the same code or conditional checks in many integration tests.
Condensed the overloaded version of WaitForConditionOrTimeOut that accepts an IConditionalPredicate implementation.

Updates and added comments.
Migrating a fix for and issue that was seen when testing both test project and runtime tests together.  The update takes into consideration the rounding error that can occur when calculating tick from time (which was an earlier fix for the tick system).
…v1.1.0' into test/all-tests-pass-on-all-consoles-v1.1.0
adding an overloaded RPC to the RpcQueueManualTests to try and get a hash collision
@0xFA11 0xFA11 enabled auto-merge (squash) February 14, 2022 20:10
@0xFA11 0xFA11 merged commit 65b4144 into develop Feb 14, 2022
@0xFA11 0xFA11 deleted the fix/unique-rpc-handlers branch February 14, 2022 21:05
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.

2 participants