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

Add Allow Debug option to Build Configuration UI #1413

Merged
merged 8 commits into from
Jul 3, 2020

Conversation

seanjparker
Copy link
Contributor

@seanjparker seanjparker commented Jul 2, 2020

For UTY-2015

Description

  • Added Allow Debug option to build config that is only accessible when Development is enabled
    image

  • Small refactor of the code that creates the UI for each platform

Tests

How did you test these changes prior to submitting this pull request?
Manually tested that debugging works in both Rider and Visual Studio, thanks @jamiebrynes7!

Documentation

How is this documented (for example: release note, upgrade guide, feature page, in-code documentation)?
Docs: https://documentation.improbable.io/gdk-for-unity/v0.3.8/docs/debugging-in-the-player

@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/no-ticket Indicates a PR has no corresponding JIRA ticket size/M Denotes a PR that changes 40-149 lines, ignoring generated files. labels Jul 2, 2020
@seanjparker seanjparker changed the title Add debug option to build config menu Add Allow Debug option to Build Configuration UI Jul 2, 2020
@improbable-prow-robot improbable-prow-robot added the A: build-system Area: Build system feature module label Jul 2, 2020
@zeroZshadow
Copy link
Contributor

Could you include a screenshot of what this looks like?

@seanjparker seanjparker marked this pull request as ready for review July 2, 2020 14:37
@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 Jul 2, 2020
CHANGELOG.md Outdated Show resolved Hide resolved
@@ -801,37 +758,11 @@ private BuildOptions ConfigureAndroid(BuildTargetConfig buildTarget)
options &= ~BuildOptions.Development;
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently you can get into a state where development is off and debug is on:

  • Enabled Development
  • Enable Debug
  • Disable Development

The UI will show them both disabled, but the build options will still have the debug flag set.

Should this line also turn off the debug flag for consistency between the UI & the backing data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that it's a good idea to explicitly disabled the flag when the development toggle is disabled.

Something interesting that I didn't realise happened in the UI code:

Starting at the third step in your comment

  • Development flag is disabled
  • The UI and flags are now in an inconsistent state since the UI shows the Allow Debug option as disabled whereas the flag internally is enabled
  • On the next render of the UI, in the ConfigureDebug function the isDebugEnabled variable is now false. This means the if statement inside the using block will always trigger the else case causing the AllowDebugging flag to be disabled.

Even though I don't explicitly disable the flag, due to the way EditorGUILayout.Toggle() works in the EditorGUI.DisabledScope it will always return false when the development flag is disabled causing an update to the AllowDebugging flag and disabling the flag anyway.

Sean Parker and others added 2 commits July 3, 2020 12:23
@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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@seanjparker seanjparker merged commit 77dba90 into develop Jul 3, 2020
@improbable-prow-robot improbable-prow-robot deleted the feature/debug-build-option branch July 3, 2020 14:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A: build-system Area: Build system feature module jira/no-ticket Indicates a PR has no corresponding JIRA ticket size/M Denotes a PR that changes 40-149 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants