Skip to content

refactor: use unity.mathematics.half in place of added types#2473

Merged
0xFA11 merged 8 commits intodevelopfrom
refactor/unity-mathematics-components-only
Mar 25, 2023
Merged

refactor: use unity.mathematics.half in place of added types#2473
0xFA11 merged 8 commits intodevelopfrom
refactor/unity-mathematics-components-only

Conversation

@NoelStephensUnity
Copy link
Copy Markdown
Member

Reducing the footprint of the Unity.Mathematics types due to the vast amount of added public API changes required.
This should have minimal impact on the API footprint.

Testing and Documentation

  • No tests have been added.
  • Includes minor modification to existing integration tests.
  • No documentation changes or additions were necessary.

adding asmdef references to Unity.Mathematics
updated HalfVector3 and HalfVector4 with Unity.Mathematics types.
removed classes no longer needed.
renamed HalfVector3DeltaPosition to NetworkDeltaPosition and made that use HalfVector3.
removing whitespace
setting the Vector4 to size of 4 (not 3).
Updating the teleport fix for testing purposes.
mirroring PR-2451 changes to NetworkTransformStateTests for the teleport fix.
Reverting out the serialization pipeline updates
A simpler approach to adding Unity.Mathematics.Half without adding any additional runtime public APIs.
/// </remarks>
public struct HalfVector3 : INetworkSerializable
{
public const int Size = 3;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this Size or Length?

Size kind of implies size in the memory or on the network, usually in bytes but it's not it right?
Length is implying "how many items does this container contain?" and that's 3 items — and 3 items x 2 bytes = 6 bytes in total.

Maybe a nit-pick but wanted to bring it up (same applies to HalfVector4 too)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

or in general, maybe never expose this constant value to public since we're using it internally for the for loops and 3 for HalfVector3 and 4 for HalfVector4 are kind of obvious by their name — what do you think?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good call out. That needed to be non-public and length does indeed make more sense then size.

Implementing Fatih's suggestion which is to rename size to length and non-public.
@0xFA11 0xFA11 merged commit 39f9a1c into develop Mar 25, 2023
@0xFA11 0xFA11 deleted the refactor/unity-mathematics-components-only branch March 25, 2023 08:09
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