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

UTY-1970: invalid enum definition #1412

Merged
merged 10 commits into from
Jul 3, 2020
Merged

Conversation

BryanJY-Wong
Copy link
Contributor

@BryanJY-Wong BryanJY-Wong commented Jul 1, 2020

Description

Shifted all C# enum definition to start from 0 to ensure no invalid enums are generated by default constructors. Serialization is done by value + min_val and Deserialization value - min_val. A warning is also generated to advice users to redefine the schema enums to start from 0.

Tests

Change the playground schema for Color (sorted and unsorted, sequential and non-sequential) and ran the code generator to ensure the enum definition, serialization and deserialization function in ColorData is what's expected.

Documentation

  • Todo

Did you remember a changelog entry?
Yes

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-1970

@improbable-prow-robot improbable-prow-robot added A: maintenance Area: Project maintenance or hygiene size/L Denotes a PR that changes 150-299 lines, ignoring generated files. A: tooling Area: Tooling labels Jul 1, 2020
@paulbalaji
Copy link
Contributor

Need to repoint this to develop

@BryanJY-Wong BryanJY-Wong changed the base branch from master to develop July 1, 2020 11:40
@improbable-prow-robot improbable-prow-robot added size/S Denotes a PR that changes 15-39 lines, ignoring generated files. and removed size/L Denotes a PR that changes 150-299 lines, ignoring generated files. labels Jul 1, 2020
@@ -60,7 +61,8 @@ public string GetDeserializationExpression(string schemaObject, uint fieldNumber
return
$"{schemaObject}.{SchemaFunctionMappings.GetSchemaFunctionFromType(PrimitiveType.Value)}({fieldNumber})";
case ValueType.Enum:
return $"({FqnType}) {schemaObject}.GetEnum({fieldNumber})";
var shifter = UnityEnumDetails.GetEnumMinimum(FqnType);
return $"({FqnType}) ({schemaObject}.GetEnum({fieldNumber}) - {shifter})";
Copy link
Contributor Author

@BryanJY-Wong BryanJY-Wong Jul 1, 2020

Choose a reason for hiding this comment

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

Tried a val + 0 on https://sharplab.io/. Compiler ignores +- 0.

@BryanJY-Wong BryanJY-Wong self-assigned this Jul 1, 2020
@BryanJY-Wong BryanJY-Wong force-pushed the bug/invalid-enum-definition branch from 11890ad to 1edc476 Compare July 2, 2020 15:47
@BryanJY-Wong BryanJY-Wong requested a review from jamiebrynes7 July 2, 2020 15:48
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
Comment on lines 13 to 15
### Breaking Changes

- All generated C# enums will now start from 0, being shifted to schema values on serialization and shifted back to C# values on deserialization. [#1412](https://github.com/spatialos/gdk-for-unity/pull/1412)
Copy link
Contributor

Choose a reason for hiding this comment

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

Typically we put 'Breaking Changes' at the top as that will be most important to a developer when upgrading

Copy link
Contributor

@paulbalaji paulbalaji left a comment

Choose a reason for hiding this comment

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

LGTM % moving breaking changes to top of changelog

Move breaking changes to top
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 3, 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

No Coverage information No Coverage information
No Duplication information No Duplication information

@BryanJY-Wong BryanJY-Wong merged commit 1fd6a9a into develop Jul 3, 2020
@improbable-prow-robot improbable-prow-robot deleted the bug/invalid-enum-definition branch July 3, 2020 13:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A: maintenance Area: Project maintenance or hygiene A: tooling Area: Tooling jira/UTY size/S Denotes a PR that changes 15-39 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants