Fixed InputManager.asset file growing in size on each Reset call #1821
Conversation
ekcoh
left a comment
ekcoh
left a comment
There was a problem hiding this comment.
LGTM, a minor comment about changelog.
| - Fixed issue where composite part dropdown manipulates binding path and leaves composite part field unchanged. | ||
| - Fixed lingering highlight effect on Save Asset button after clicking. | ||
| - Fixed missing name in window title for Input Action assets. | ||
| - Fixed InputManager.asset file growing in size on each Reset call (ISX-1839) |
There was a problem hiding this comment.
Minor: Missing full-stop in end. Referencing internal case ID for a non-publicly visible issue.
There was a problem hiding this comment.
Changed (online in githib editor ;) )
Note: Some prior change log entries also seem to have some internal bug references but I've not changed those in this PR
|
Seems like PR got hit by CI instability, rerun tests? |
Removed internal bug id and added full stop
Re-running with minor changelog update. |
ritamerkl
left a comment
ritamerkl
left a comment
There was a problem hiding this comment.
Just two little remarks on the comments, looks good, nice to have this fixed!
| /// <summary> | ||
| /// Delete project wide input actions | ||
| /// </summary> | ||
| /// <param name="asset"></param> |
There was a problem hiding this comment.
parameter left over in comment
| /// <summary> | ||
| /// Reset project wide input actions asset | ||
| /// </summary> | ||
| /// <param name="asset"></param> |
…re we never create duplicates Added Start/EndAssetEditing calls to try and make the serialisation code more atomic Removed some unused param fields in the docs Removed some excessiver SaveAssets calls
…Unity-Technologies/InputSystem into ISX_1839_reset_fix_to_remove_duplicates
…doesn't actually impact AddObjectToAsset
|
Params documentation fixed - and moved delete into the create so that when new data created any old data is always cleaned out. This catches an extra case. |
ekcoh
left a comment
ekcoh
left a comment
There was a problem hiding this comment.
Approving but have one outstanding questions around the intended behavior in the DeleteActionAssetAndActionReferences() function regarding intended behavior to consider before merge.
| else if (obj is InputActionAsset) | ||
| { | ||
| AssetDatabase.RemoveObjectFromAsset(obj); | ||
| } |
There was a problem hiding this comment.
What happens if there is something here that isn't an InputActionReference or an InpuitAction asset?
I recommend we either throw or log an error or alternatively change the last else if into an unconditional else which would remove anything? Or is this designed to leave e.g. InputManager or any other root objects untouched?
There was a problem hiding this comment.
Its designed to leave InputManager and others untouched
Description
Reset now clears out old Input Action Assets and Input Action References first to avoid the InputManager.asset file growing in size over each reset
For bug https://jira.unity3d.com/browse/ISX-1839
Changes made
Spinning through contents of InputManager.asset to remove Input Action Assets and Input Action References first
Then using GetOrCreate to create a new version (to make sure only 1 added, if GetOrCreate called in a callback elsewhere - which it is due to AssetDatabase.SaveAssets call)
Notes
Checklist
Before review:
During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.