Skip to content

fix: Rename 'Send Queue Batch Size' field in adapter #1584

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 3 commits into from
Jan 18, 2022

Conversation

simon-lemay-unity
Copy link
Contributor

Its actual meaning is to be the maximum payload size. The name is not entirely wrong, since this also happens to be the batch size for the send queue, but the field would still have a utility if we removed batching from the adapter, so 'Max Payload Size' is a more accurate name.

We had not renamed it before because we didn't want to break compatibility, but I just learned of FormerlySerializedAs which allows renaming inspector fields while retaining the value assigned to their previous name. Using that to rename the field, we avoid a breaking change.

This addresses MTT-2175.

Changelog

com.unity.netcode.adapter.utp

  • Changed: Rename the 'Send Queue Batch Size' property to 'Max Payload Size' to better reflect its usage.

Testing and Documentation

  • No tests have been added.
  • No documentation changes or additions were necessary.

Its actual meaning is to be the maximum payload size. The name is not
entirely wrong, since this also happens to be the batch size for the
send queue, but the field would still have a utility if we removed
batching from the adapter, so 'Max Payload Size' is a more accurate
name.
Copy link
Contributor

@jeffreyrainy jeffreyrainy left a comment

Choose a reason for hiding this comment

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

Yay, better naming indeed

@0xFA11 0xFA11 merged commit 4f0a859 into develop Jan 18, 2022
@0xFA11 0xFA11 deleted the fix/rename-send-queue-batch-size-field branch January 18, 2022 17:23
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