Skip to content

fix: RPC metrics name table is empty #3782

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 18 commits into
base: develop-2.0.0
Choose a base branch
from
Open

fix: RPC metrics name table is empty #3782

wants to merge 18 commits into from

Conversation

@NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Nov 12, 2025 •
edited
Loading

Purpose of this PR

Fixes the issue where __rpc_name_table was not built (yet) when an RPC, located on another NetworkBehaviour that is ordered later than the invoking NetworkBehaviour, is invoked during OnNetworkSpawn and scene management is disabled.

This PR includes some re-organization:

  • Consolidated the metrics related scripts into NetworkBehaviour.
  • Added an additional define AND condition that completely excludes the metrics associated scripts and tables from a build if the Multiplayer Tools package is not installed.

fix: #3742

Jira ticket

MTTB-1734

Changelog

  • Fixed: Issue where invoking an RPC on another NetworkBehaviour associated with the same NetworkObject that is ordered before the NetworkBehaviour with the RPC and scene management is disabled could throw an exception.

Documentation

  • No documentation changes or additions were necessary.

Testing & QA (How your changes can be verified during release Playtest)

Functional Testing

Manual testing :

  • Manual testing done
    • Replication project was created and posted in the GH issue thread.
    • Metrics test:
      • Open any internally used manual test.
      • Make sure MP Tools package is installed.
      • Open the profiler and enable the message metrics.
      • Start a session with at least 1 MPPM virtual client.
      • Start collecting profile information while the session is active.
      • Verify the metrics are still working as expected.

Automated tests:

  • Covered by existing automated tests
  • Covered by new automated tests
    • RpcDuringOnNetworkSpawn

Does the change require QA team to:

  • Review automated tests?
  • Execute manual tests?
  • Provide feedback about the PR?

If any boxes above are checked the QA team will be automatically added as a PR reviewer.

Backports

No back port is required.

@NoelStephensUnity
fix
b89ccd3 
Improving our defines that determine if metrics should be gathered.
1st pass attempt at unifying this process.
The primary change is that if the __rpc_name_table is not built (yet) when an RPC is invoked that it will attempt to build it for that specific NetworkBehaviour prior to attempting to log RPC metrics.
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review November 12, 2025 22:05
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner November 12, 2025 22:05
@NoelStephensUnity
fix
43e0795 
Missed one set of defines check.
@NoelStephensUnity
fix
ec09106 
Removing reference to NetworkBehaviour.__rpc_name_table as it is not used any longer within the codegen process.
@NoelStephensUnity
fix
d18cf06 
Some additional areas in codegen that requires the extended define wrap.
Wrapping the RPC invocation tests (we will need to find a better way to get the name for this test).
@NoelStephensUnity NoelStephensUnity requested review from a team and EmandM as code owners November 12, 2025 23:39
@NoelStephensUnity
refactor
f9f721a 
Making sure we still have the NetworkManager.__rpc_name_table as that has become part of the public API.
@NoelStephensUnity
fix
593c2f7 
This fixes the issue with invoking RPCs on other child NetworkBehaviours during OnNetworkSpawn.
@NoelStephensUnity
test
4e5dfcc 
Adding a test to validate this specific issue is fixed.
@NoelStephensUnity
style
07ca6e2 
Removing trailing whitespace.
@NoelStephensUnity NoelStephensUnity mentioned this pull request Nov 13, 2025
Open
@NoelStephensUnity
update
dda40db 
Marking newly added methods for aggressive in-lining.
@NoelStephensUnity
update
202e005 
Adding change log entry.
@NoelStephensUnity
style
bee700f 
Fixing a comment.
Adding additional comments.
@NoelStephensUnity
style
fd5ba9b 
Making sure the correct ValidateRpcMessageMetrics name is used when logging an error.
@NoelStephensUnity
style
ce720f2 
Updated and added some comments to the region of code that contains the primary fix.
FelixWentworth-Unity
FelixWentworth-Unity reviewed Nov 14, 2025
View reviewed changes
Copy link

@FelixWentworth-Unity FelixWentworth-Unity left a comment

Choose a reason for hiding this comment

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

Tested replication project provided with NGO 2.5.0, 2.5.1. 2.6.0, 2.7.0 & these changes and can verify that this PR fixes the issues seen in 2.6.0 and 2.7.0.

@NoelStephensUnity
fix
3189abf 
Adding target client that is being sent to.
@NoelStephensUnity
update
0b0cba2 
Adding AggressiveInlining to updated method.
@NoelStephensUnity NoelStephensUnity enabled auto-merge (squash) November 14, 2025 15:54
FelixWentworth-Unity
FelixWentworth-Unity approved these changes Nov 14, 2025
View reviewed changes
Copy link

@FelixWentworth-Unity FelixWentworth-Unity left a comment

Choose a reason for hiding this comment

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

Tested & verified - Looks good!

@sentinel-u3d sentinel-u3d bot requested a review from noellie-velez November 14, 2025 16:49
noellie-velez
noellie-velez approved these changes Nov 14, 2025
View reviewed changes
Copy link
Collaborator

@noellie-velez noellie-velez left a comment

Choose a reason for hiding this comment

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

Nice fix and cleanup! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@noellie-velez noellie-velez noellie-velez approved these changes

@FelixWentworth-Unity FelixWentworth-Unity FelixWentworth-Unity approved these changes

@EmandM EmandM Awaiting requested review from EmandM EmandM is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

The given key 'EnemyAnimations' was not present in the dictionary

5 participants

@NoelStephensUnity @noellie-velez @FelixWentworth-Unity @netcode-ci-service