FIX: Avoid making new created assets dirty when opened #1761
Conversation
|
LGTM, only one thing: what happens if you have an existing asset and delete all action maps and control schemes? should still be dirty then, right? (it's a edge case, but still worth trying I think) |
Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs
Show resolved
Hide resolved
Good catch, this indeed does not trigger the dirty state even though it should. @jfreire-unity |
Pauliusd01
left a comment
Pauliusd01
left a comment
There was a problem hiding this comment.
Updating status to waiting for fixes
Ideally at this stage, the file on disk should have a serializable equivalent as the serialized asset. However, because the new asset on disk is being created with a specific default json layout, the serialized JSON asset doesn't have not the same content. There are opportunities for change in this area.
|
@Pauliusd01 thanks! you can try again |
Yep, quite on point use case. Thanks! I fixed it now |
| // If it is, there's nothing to save. | ||
| // At the moment, an asset only has the default asset layout content on disk when it is first created. | ||
| // So in this case we cannot go through the normal path and compare what's on disk with what has been serialized. | ||
| if (InputActionAsset.HasDefaultJsonLayout(m_AssetJson) && asset.IsEmpty()) |
There was a problem hiding this comment.
Isn't this specific to this editor? I would recommend moving the logic to the associated utils class or making it an editor-only extension for the asset?
There was a problem hiding this comment.
Thanks. Remove the method and just have it inline as I don't know if it needs to be used elsewhere at the moment.
Checking if the asset is empty is not editor dependent.
| /// InputActionAssets. | ||
| /// </remarks> | ||
| public const string Extension = "inputactions"; | ||
| ////REVIEW: actually pre-populate with some stuff? |
There was a problem hiding this comment.
What is the content after a save of an empty asset? Not sure why do not initialise it like that when its created? Then we wouldn't need these checks at all and the equality check would solve that for us
Pauliusd01
left a comment
Pauliusd01
left a comment
There was a problem hiding this comment.
LGTM, did not notice anymore unusual scenarios where the asset gets dirty when it shouldn't (when undoing changes, redoing, creating new ones, cancelling already made ones and seeing if they are still dirty, etc)
Description
https://jira.unity3d.com/browse/ISX-1591
Changes made
Assets without any action maps and control schemes are not set as "dirty" anymore.
Notes
@Pauliusd01 since you reported this, please try it out and check if it solves the reported issue.
Checklist
Before review:
Changed,Fixed,Addedsections.([case %number%](https://issuetracker.unity3d.com/issues/...)).Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.