Skip to content
This repository has been archived by the owner on Jan 18, 2022. It is now read-only.

UTY-2661: Improve snapshot api #1479

Merged
merged 11 commits into from
Sep 9, 2020
Merged

UTY-2661: Improve snapshot api #1479

merged 11 commits into from
Sep 9, 2020

Conversation

BryanJY-Wong
Copy link
Contributor

@BryanJY-Wong BryanJY-Wong commented Sep 8, 2020

Description

In Snapshot class:

  • Added method to check if EntityId is valid
  • Improve search for next available EntityId
  • Added warning when overwriting entities

Tests

  • Added unit tests for Snapshot class

Documentation

  • Changelog

Primary reviewers

If your change will take a long time to review, you can name at most two primary reviewers who are ultimately responsible for reviewing this request. @ mention them.

@improbable-prow-robot
Copy link

Corresponding JIRA ticket: https://improbableio.atlassian.net/browse/UTY-2661

@improbable-prow-robot improbable-prow-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. jira/UTY size/M Denotes a PR that changes 40-149 lines, ignoring generated files. labels Sep 8, 2020
@improbable-prow-robot improbable-prow-robot added A: core Area: Core GDK A: playground Area: Playground labels Sep 8, 2020
@BryanJY-Wong BryanJY-Wong marked this pull request as ready for review September 8, 2020 11:36
@improbable-prow-robot improbable-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 8, 2020
Copy link
Contributor

@jamiebrynes7 jamiebrynes7 left a comment

Choose a reason for hiding this comment

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

While you're here, can you cherry pick the Snapshot changes from https://github.com/spatialos/gdk-for-unity/pull/1477/files#diff-6cbb5e5c9ae6005cf38f5951cabcfbecR89 which fixes a memory leak (assuming you dispose the Snapshot).

Copy link
Contributor

@jamiebrynes7 jamiebrynes7 left a comment

Choose a reason for hiding this comment

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

We should also amend the usage of the Snapshot class in the playground (I think only disposing it is required here).

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
Copy link
Contributor

@jamiebrynes7 jamiebrynes7 left a comment

Choose a reason for hiding this comment

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

LGTM % changelog suggestions

CHANGELOG.md Outdated
Comment on lines 10 to 11
- Added new features to the `Snapshot` class [#1479](https://github.com/spatialos/gdk-for-unity/pull/1479)
- Adding duplicate entity ID now throws an exception
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Added new features to the `Snapshot` class [#1479](https://github.com/spatialos/gdk-for-unity/pull/1479)
- Adding duplicate entity ID now throws an exception
- Adding an entity to the `Snapshot` class with a duplicate entity ID now throws an exception. [#1479](https://github.com/spatialos/gdk-for-unity/pull/1479)

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@improbable-prow-robot improbable-prow-robot removed the size/M Denotes a PR that changes 40-149 lines, ignoring generated files. label Sep 8, 2020
@improbable-prow-robot improbable-prow-robot added the size/L Denotes a PR that changes 150-299 lines, ignoring generated files. label Sep 8, 2020
@sonarcloud
Copy link

sonarcloud bot commented Sep 8, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

92.9% 92.9% Coverage
0.0% 0.0% Duplication

@BryanJY-Wong BryanJY-Wong merged commit 05ec260 into develop Sep 9, 2020
@improbable-prow-robot improbable-prow-robot deleted the feature/snapshot-api branch September 9, 2020 09:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A: core Area: Core GDK A: playground Area: Playground jira/UTY size/L Denotes a PR that changes 150-299 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants