Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

Report the correct editing status to Unity. Fixes #934 #959

Merged
merged 4 commits into from
Nov 28, 2018

Conversation

shana
Copy link
Member

@shana shana commented Nov 7, 2018

There's two issues going on here:

  • The loggedInUser field wasn't getting seeded with user information on load, so checking whether the current user owns a lock would always fail until the connections changed event got triggered somehow

  • GetLock didn't account for not having user information, so if the user was logged out, it would default to not allowing any edits. Since the info wasn't getting seeded, it would always report files as being locked by someone else. It now defaults to allowing edits if the user isn't signed in. Now, this isn't a bug, but if the user is not signed in, all bets are kinda off on validating locks. We should probably warn the user that we can't check whether they own the lock, but we should probably not lock down Unity entirely because of it.

There's two issues going on here

  - The `loggedInUser` field wasn't getting seeded with user information on load,
	so checking whether the current user owns a lock would always fail until the
	connections changed event got triggered somehow

  - `GetLock` didn't account for not having user information, so if the user was
	logged out, it would default to not allowing any edits. Since the info wasn't
	getting seeded, it would always report files as being locked by someone else.
	It now defaults to allowing edits if the user isn't signed in. Now, this isn't
	a bug, but if the user is not signed in, all bets are kinda off on validating
	locks. We should probably warn the user that we can't check whether they own
	the lock, but we should probably not lock down Unity entirely because of it.
@shana shana force-pushed the fixes/934-inspector-disabled branch from 18a3a75 to be137f8 Compare November 19, 2018 14:45
@shana shana force-pushed the fixes/934-inspector-disabled branch from be137f8 to 84c543f Compare November 19, 2018 15:13
@StanleyGoldman StanleyGoldman force-pushed the fixes/934-inspector-disabled branch from 3c68631 to 84c543f Compare November 26, 2018 20:26
Copy link
Contributor

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

Where or how is this preprocessor directive set?

Copy link
Contributor

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

Can you tell me a bit more about this function call?

Copy link
Contributor

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

This seems like something we will forget one day...

@StanleyGoldman StanleyGoldman dismissed their stale review November 27, 2018 15:19

Made the change

@StanleyGoldman
Copy link
Contributor

Okay, I understand all of this now. I'll do some tests against the newer version of Unity and I'll be ready to merge this.

@StanleyGoldman StanleyGoldman merged commit af14cf7 into master Nov 28, 2018
@StanleyGoldman StanleyGoldman deleted the fixes/934-inspector-disabled branch November 28, 2018 12:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants