Skip to content

feat: expose network behaviour name internally #1057

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 6 commits into from
Aug 17, 2021

Conversation

becksebenius-unity
Copy link
Contributor

@becksebenius-unity becksebenius-unity commented Aug 17, 2021

This PR exposes a method to internal access called __getTypeName(). This is accomplished using ILPP to avoid runtime memory allocations, and to avoid increasing the memory footprint of each NetworkBehaviour instance.

Changes:

  1. Introduce internal virtual string __getTypeName() => nameof(NetworkBehaviour); to NetworkBehaviour.
  2. In the ILPP step, implement the override of this method to return a string literal that matches the component name.
  3. Implement test that verifies that the method correctly returns the concrete class type name instead of the base class

@becksebenius-unity becksebenius-unity requested review from 0xFA11 and a team August 17, 2021 00:41
@becksebenius-unity becksebenius-unity changed the title feat: expose network variable name internally feat: expose network behaviour name internally Aug 17, 2021
@becksebenius-unity
Copy link
Contributor Author

draft of intended usage: #1058

@@ -592,6 +592,25 @@ private void ProcessNetworkBehaviour(TypeDefinition typeDefinition, string[] ass
instructions.Reverse();
instructions.ForEach(instruction => processor.Body.Instructions.Insert(0, instruction));
}

// __getBehaviourName processor
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this scoped (i.e. in brackets)? Is this an ILPP thing or something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think its just an documentation/organizational thing, since the ILPP code can be very hard to follow

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, a block with comment on top is a cool way to document what's going on in that region.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in addition to andrew and fatih's comments, the brackets are to scope the local variables since they share a name with the other scopes. reasonably these could be functions to avoid this but i didn't want to add complexity to an already huge class

Copy link
Contributor

@0xFA11 0xFA11 left a comment

Choose a reason for hiding this comment

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

🚀

@0xFA11 0xFA11 enabled auto-merge (squash) August 17, 2021 15:22
@0xFA11 0xFA11 merged commit 24cebfb into develop Aug 17, 2021
@0xFA11 0xFA11 deleted the feature/expose-network-behaviour-name branch August 17, 2021 15:58
SamuelBellomo added a commit that referenced this pull request Aug 23, 2021
…nsform

* develop: (21 commits)
  test: adding more details to multiprocess readme (#1050)
  refactor!: convert NetworkTransform.NetworkState to `struct` (#1061)
  fix: networkmanager destroy on app quit (#1011)
  feat: snapshot, using unreliable packets, now that the underlying foundation supports it. Only merge after #1062 (#1064)
  feat: snapshot. Fully integrated despawn, mtt-1092, mtt-1056 (#1062)
  fix: eliminate bad use-after-free(destroy) pattern (#1068)
  chore: cleanup meta files for empty dirs (#1067)
  chore: minor MLAPI to Netcode rename (#1065)
  feat: report network behaviour name to the profiler (#1058)
  fix: player movement (#1063)
  test: Add unit tests for NetworkTime properties (#1053)
  chore: remove authority & netvar perms from NetworkTransform (#1059)
  feat: networktransform pos/rot/sca thresholds on state sync (#1055)
  feat: expose network behaviour type name internally (#1057)
  chore: remove all the old profiling code (#1048)
  fix: if-guard `NetworkManager.__rpc_name_table` access (#1056)
  fix: Disabling fixedupdate portion of SpawnRpcDespawn test because it's failing for known reasons that will be fixed in the IMessage refactor. (#1049)
  feat: Implement metrics for the new network profiler (#960)
  chore!: change package name & asmdefs (#1026)
  feat: per axis networktransform state sync (+bitwise state comp) (#1042)
  ...

# Conflicts:
#	com.unity.netcode.gameobjects/Prototyping/NetworkTransform.cs
mollstam pushed a commit to Keepsake-Games/com.unity.netcode.gameobjects that referenced this pull request Feb 13, 2023
…es#1057)

* feat: expose the name of the network behaviour without runtime alloc

* Added test to verify that the name is exposed correctly

* removed unnecessary using

* code review feedback

* fixed bad comment
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.

4 participants