Skip to content

fix: remove ilpp for network variable [MTT-3438] #1976

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 15 commits into from
May 25, 2022

Conversation

ShadauxCat
Copy link
Collaborator

There were cases where generic types were causing ILPP to perform incorrectly or crash at compile-time. This removes the offending ILPP code entirely - a couple of the generic cases were already addressed and another was found, so confidence that the ILPP wouldn't turn into an infinite series of such fixes was low. Moving to a runtime approach also reduces compile times since we have less ILPP code running.

Changelog

  • Fixed: Generic types that inherit from NetworkBehaviour no longer cause crashes at compile time.

Testing and Documentation

  • Includes integration tests.
  • (Will include) documentation for previously-undocumented public API entry points.

ShadauxCat added 7 commits May 6, 2022 19:13
# Conflicts:
#	com.unity.netcode.gameobjects/Editor/CodeGen/com.unity.netcode.editor.codegen.asmdef
#	testproject/Assets/Tests/Runtime/testproject.runtimetests.asmdef
…atible with extension methods

- Added xmldocs
- Added tests
@ShadauxCat ShadauxCat requested review from 0xFA11 and a team as code owners May 18, 2022 22:53
# Conflicts:
#	com.unity.netcode.gameobjects/Editor/CodeGen/INetworkSerializableILPP.cs
#	com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableHelper.cs
#	com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableSerialization.cs
#	com.unity.netcode.gameobjects/Runtime/Serialization/BufferSerializer.cs
#	com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs
#	com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferWriter.cs
@@ -22,7 +22,8 @@ Additional documentation and release notes are available at [Multiplayer Documen

- Fixed issues when multiple `ConnectionApprovalCallback`s were registered (#1972)
- Fixed endless dialog boxes when adding a NetworkBehaviour to a NetworkManager or vice-versa (#1947)
- `FixedString` types can now be used in NetworkVariables and RPCs again without requiring a `ForceNetworkSerializeByMemcpy<>` wrapper (#1961)
- `FixedString`, `Vector2Int`, and `Vector3Int` types can now be used in NetworkVariables and RPCs again without requiring a `ForceNetworkSerializeByMemcpy<>` wrapper (#1961)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mentioning Vector2Int and Vector3Int was supposed to be part of my last PR but I forgot to push it after I committed it so it ended up in this branch by accident, but also needs to be there... I'll move it into a separate PR if desired but it seems so small as to not warrant it, maybe?

Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity left a comment

Choose a reason for hiding this comment

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

Looks good Kitty!

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.

let's go! 🚀

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.

3 participants