FIX: ISXB-1221 input action binding discarded #2119
Conversation
smnwttbr
force-pushed
the
isxb-1221-input-action-binding-discarded
branch
from
77b3fe9 to
9dc97d8
Compare
|
@smnwttbr seeing a lot of CI failures, is this one's ready? |
There was a problem hiding this comment.
Thanks for looking into this. I think you are right about the underling root cause and this may (or may not) be a bit of duck taping on top of the underlying problem. It's great if this resolves the issue but I am still thinking it may have other side-effects but maybe I am wrong on this. I added some comments / questions as a start on the review process.
| @@ -89,10 +90,10 @@ public void OnGUI() | |||
| EditorGUILayout.EndHorizontal(); | |||
| } | |||
|
|
|||
| //TODO: on next major version remove property argument. | |||
There was a problem hiding this comment.
I would recommend filing a JIRA ticket on this if/when this lands, otherwise it would be forgotten.
| // The picker dropdown is a separate window and does not have access to the changed serializedObject reference. | ||
| // This could be removed if the InputControlPathEditor is converted to UITK with a stable, persistent serializedObject backing this editor. | ||
| // This property will be shared among multiple asset editor windows. | ||
| private static SerializedProperty s_pathProperty { get; set; } |
There was a problem hiding this comment.
This static SerializedProperty, wouldn't it leak memory? Its referencing a SerializedObject that was dynamically allocated and may/would be disposed from outside this editor?
There was a problem hiding this comment.
Additionally this assumes there can only exist one InputControlPathEditor instance right?
There was a problem hiding this comment.
Yes, there will be a dangling reference tht may hang around. I'll add some code to clean it up when the editor is closed.
Yes, only one instance of the InputControlPathEditor will work at a time. I think this is ok, as it is a popup window, and disappears when it loses focus.
There was a problem hiding this comment.
Description
Fix for ISXB-1221 where the chose input action binding can be discarded and not saved with the asset in different circumstances.
The root cause of this issue is the two-asset-solution we have, where we edit a copy of the asset using a serializedObject, then write that out to disk, then reload a new copy of the asset into a new serializedObject to continue editing.
This causes our UITK editor to be rebuilt and rebound to the new asset on every user interaction, which causes this issue. The workaround involves storing a reference in a static variable which is updated whenever the editor is rebuilt.
Testing status & QA
Please describe the testing already done by you and what testing you request/recommend QA to execute. If you used or created any testing project please link them here too for QA.
Overall Product Risks
Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any.
Comments to reviewers
Please describe any additional information such as what to focus on, or historical info for the reviewers.
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.After merge: