Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

- Improved New Input System warning dialog, Native Device Inputs Not Enabled [UUM-132151].
- Fixed caching for InputControlPath display name [ISX-2501](https://jira.unity3d.com/browse/ISX-2501)
- Fixed editor closing and not saving the input asset when clicking cancel in the dialog prompt [UUM-134748](https://jira.unity3d.com/browse/UUM-134748)

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

private string m_AssetJson;
private bool m_IsDirty;
private bool m_IsEditorQuitting;

private StateContainer m_StateContainer;
private InputActionsEditorView m_View;
Expand Down Expand Up @@ -316,11 +317,21 @@
private void OnEnable()
{
analytics.Begin();
EditorApplication.wantsToQuit += OnWantsToQuit;
}

private void OnDisable()
{
analytics.End();
EditorApplication.wantsToQuit -= OnWantsToQuit;
}

private bool OnWantsToQuit()
{

Check warning on line 330 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs#L330

Added line #L330 was not covered by tests
// Here the user will be prompted
bool isAllowedToQuit = HandleOnDestroyIfApplicationIsAllowedToQuit(false);
m_IsEditorQuitting = isAllowedToQuit;
return m_IsEditorQuitting;

Check warning on line 334 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs#L332-L334

Added lines #L332 - L334 were not covered by tests
}

private void OnFocus()
Expand All @@ -345,39 +356,52 @@
analytics.RegisterEditorFocusOut();
}

private void HandleOnDestroy()
/// <summary>
/// Shows a dialog when trying to close an input asset without saving changes.
/// </summary>
/// <returns> Returns true if you should allow the Unity Editor to close. </returns>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// <returns> Returns true if you should allow the Unity Editor to close. </returns>
/// <param name="rebuildUIOnCancel">If true, reopens the editor window when user cancels.</param>
/// <returns> Returns true if you should allow the Unity Editor to close. </returns>

private bool HandleOnDestroyIfApplicationIsAllowedToQuit(bool rebuildUIOnCancel)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm having issue with the semantic mismatch of what the method is doing versus what it is returning.
The method is executing a command (should return void) but returns a query (return a value).

What about a change of signature:

Suggested change
private bool HandleOnDestroyIfApplicationIsAllowedToQuit(bool rebuildUIOnCancel)
private bool CanCloseWindow(bool promptIfDirty, bool rebuildUIOnCancel)

Or

Suggested change
private bool HandleOnDestroyIfApplicationIsAllowedToQuit(bool rebuildUIOnCancel)
private bool CheckCanCloseAndPromptIfDirty(bool rebuildUIOnCancel)

{
// Do we have unsaved changes that we need to ask the user to save or discard?
if (!m_IsDirty)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code could be simplify.

Suggested change
if (!m_IsDirty)
// Early out if asset up to date or editor closing.
if (!m_IsDirty || m_IsEditorQuitting)

return;
return true;

// Get target asset path from GUID, if this fails file no longer exists and we need to abort.
var assetPath = AssetDatabase.GUIDToAssetPath(m_AssetGUID);
if (string.IsNullOrEmpty(assetPath))
return;
return true;

Check warning on line 372 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs#L372

Added line #L372 was not covered by tests

// Prompt user with a dialog
var result = Dialog.InputActionAsset.ShowSaveChanges(assetPath);
switch (result)
if (!m_IsEditorQuitting)
{
case Dialog.Result.Save:
Save(isAutoSave: false);
break;
case Dialog.Result.Cancel:
// Cancel editor quit. (open new editor window with the edited asset)
ReshowEditorWindowWithUnsavedChanges();
break;
case Dialog.Result.Discard:
// Don't save, quit - reload the old asset from the json to prevent the asset from being dirtied
break;
default:
throw new ArgumentOutOfRangeException(nameof(result));
// Prompt user with a dialog
var result = Dialog.InputActionAsset.ShowSaveChanges(assetPath);
switch (result)
{
case Dialog.Result.Save:
Save(isAutoSave: false);
return true;

Check warning on line 382 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs#L381-L382

Added lines #L381 - L382 were not covered by tests
case Dialog.Result.Cancel:
if (rebuildUIOnCancel)
{

Check warning on line 385 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs#L384-L385

Added lines #L384 - L385 were not covered by tests
// Cancel editor quit. (open new editor window with the edited asset)
ReshowEditorWindowWithUnsavedChanges();
}

Check warning on line 388 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs#L387-L388

Added lines #L387 - L388 were not covered by tests

return false;

Check warning on line 390 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs#L390

Added line #L390 was not covered by tests
case Dialog.Result.Discard:
// Don't save, quit - reload the old asset from the json to prevent the asset from being dirtied
return true;
default:
throw new ArgumentOutOfRangeException(nameof(result));

Check warning on line 395 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs#L395

Added line #L395 was not covered by tests
}
}

return true;

Check warning on line 399 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs#L399

Added line #L399 was not covered by tests
}

private void OnDestroy()
{
HandleOnDestroy();
HandleOnDestroyIfApplicationIsAllowedToQuit(true);

// Clean-up
CleanupStateContainer();
Expand Down