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

Convert all our IDs from strings to validated GUIDs #476

Merged
merged 10 commits into from
Jan 28, 2020

Conversation

stevenvergenz
Copy link
Contributor

No description provided.

log.error('network', `[ERROR] setAuthoritativeClient: client ${clientId} does not exist.`);
throw new Error(`Client ${clientId} does not exist.`);
Copy link
Contributor

@eanders-ms eanders-ms Jan 28, 2020

Choose a reason for hiding this comment

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

throw new Error(Client ${clientId} does not exist.); [](start = 3, length = 54)

Ensure this exception is handled and doesn't exit SDK code. Or remove it. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it be better to simply return? I don't think this case is ever hit, because the app would throw later on in this function if it did.


In reply to: 372071873 [](ancestors = 372071873)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya I think just returning would be better. We never hit this that we know of. We don't have a way to recover from this if it were to happen.


In reply to: 372075051 [](ancestors = 372075051,372071873)

}

/** @inheritdoc */
public get meshId() { return this._meshId; }
public set meshId(value) {
if (!value || value.startsWith('0000')) {
if (!value) {
Copy link
Contributor

@eanders-ms eanders-ms Jan 28, 2020

Choose a reason for hiding this comment

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

value [](start = 7, length = 5)

The logic here used to also check for a non-null value of all zeros. How is that accounted for now? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All-zero GUIDs don't need this path, since the "then" part is to use the Zero guid instead.


In reply to: 372083083 [](ancestors = 372083083)

}

/** @inheritdoc */
public get mainTextureId() { return this._mainTextureId; }
public set mainTextureId(value) {
if (!value || value.startsWith('0000')) {
if (!value) {
Copy link
Contributor

@eanders-ms eanders-ms Jan 28, 2020

Choose a reason for hiding this comment

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

f (!value) { [](start = 3, length = 12)

Same question here. #Resolved

}

/** @inheritdoc */
public get materialId() { return this._materialId; }
public set materialId(value) {
if (!value || value.startsWith('0000')) {
if (!value) {
Copy link
Contributor

@eanders-ms eanders-ms Jan 28, 2020

Choose a reason for hiding this comment

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

if (!value) { [](start = 2, length = 13)

Same question here. #Resolved

Copy link
Contributor

@eanders-ms eanders-ms left a comment

Choose a reason for hiding this comment

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

🕐

Copy link
Contributor

@eanders-ms eanders-ms left a comment

Choose a reason for hiding this comment

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

Nice cleanup.

@stevenvergenz stevenvergenz merged commit 7a0ee5c into red Jan 28, 2020
@stevenvergenz stevenvergenz deleted the stvergen/convert-guid branch January 28, 2020 22:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants