FIX: Delayed input backend prompt to make sure asset import is not interrupted (ISXB-608)

[ritamerkl]

Description

Like described in this ticket ISXB-608 the prompt to activate the InputSystem backend is delayed to after assets are imported. This means the prompt will appear once the editor launched, and in case of the package being imported after all assets loaded in the current domain reload.

FYI: The delayCall is called AFTER domain reloads when the UI inspectors update.

The dependency to the InputSystem (for affected assets eg Enemies) should be updated to the latest version after this change.

Testing status & QA

I tested this for editor launches and importing the package in newly created projects. Testing this in combination with importing assets is cumbersome, since the import happens based on the version define of the asset dependency.

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.

• Complexity: Low
• Halo Effect: Low

Comments to reviewers

Please describe any additional information such as what to focus on, or historical info for the reviewers.

Checklist

Before review:

• Changelog entry added.
  • Explains the change in Changed, Fixed, Added sections.
  • For API change contains an example snippet and/or migration example.
  • JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
  • Jira port for the next release set as "Resolved".
• Tests added/changed, if applicable.
  • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
  • Performance tests.
  • Integration tests.
• Docs for new/changed API's.
  • Xmldoc cross references are set correctly.
  • Added explanation how the API works.
  • Usage code examples added.
  • The manual is updated, if needed.

During merge:

• Commit message for squash-merge is prefixed with one of the list:
  • NEW: ___.
  • FIX: ___.
  • DOCS: ___.
  • CHANGE: ___.
  • RELEASE: 1.1.0-preview.3.

After merge:

• Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

[Pauliusd01]

LGTM, here's what I've checked:

• Adding just the Input System package while the Editor is open/closed
• Bundling the Input System package with some large, valid assets to import (Editor open) - Pass - packages/assets were imported first -> only then I got the dialog
• Bundling the Input System package with some large, valid assets to import (Editor closed) - Pass - packages/assets were imported first -> Editor opened and I got the dialog
• Bundling the Input System package with some erroring out assets (Editor open) - Pass
• Bundling the Input System package with some erroring out assets (Editor closed) - Pass - safe mode did not interfere with the dialog order
• Reimport All option (Input Manager backend) - Pass - got the prompt on reimport
• Reimport All option (Input System backend) - Pass - no unnecessary prompt or backend resets

[ekcoh]

Does this guarantee that it will execute after asset import? I would have expected this to be triggered by an AssetP since this one will simply but the callback on the editor event queue?
If it works its great, but felt I was not seeing how this is robust.

[ritamerkl]

The AssetPostProcessor was an other option I considered, but it requires another class to inherit from AssetPostprocessor, so I decided for delayedCall instead. delayCall is called after domain reloads (which include importing of assets at the end) when the inspector windows refresh, which I think is suitable for a prompt. I don't see why it's not robust, in which way do you think it couldn't be?

[ekcoh]

I see, yeah we have some classes doing that as part of InputActionImporter. If delayCall runs after domain reloads it's good, but to me it wasn't clear it runs after asset import (which might not trigger a domain reload if no code changed?). At least it's not explicit from https://docs.unity3d.com/6000.0/Documentation/ScriptReference/EditorApplication-delayCall.html, so it seems to relate to inspectors but not necessarily asset imports? Maybe you know something I do not in this area, but to me it wasn't clear that e.g. delayCall could be executed between two imports.

[ritamerkl]

Since InitializeInEditor is called from the static constructor it's only executed when the editor launches or the package gets added into the project (afaik - correct me if I forgot a case), in both cases there is a domain reload happening.
Here a short explanation why I think the call is reliable:
The callbacks in delayCall are invoked in the subsequent editor update.
Editor updates do not happen during a domain reload (which includes import of assets - no matter how many), Editor updates resume only after the domain reload is complete.
One reason for this is that Editor updates depend on stable managed objects and assemblies. Since these objects are being reloaded, updates during this period could result in crashes or undefined behavior.

[ekcoh]

Would also be great to understand why this was moved out of its previous place?

[ritamerkl]

I put it into a private function of the same class to make it more readable, I think having big chunks of code in single functions do no good for the readability some times. (technically still in the same place though)

[ekcoh]

I am not sure I see the function you are referring to? What I meant was basically that previously EnableActions(), RunInitialUpdate(), was executed from within ShowRestartWarning() as part of the deferred ShowRestartWarning. Now it executes directly in InitializeInEditor (before) code in ShowRestartWarning(). Hence why I asked if it still provides the same behavior or what the reason was for relocating it / changing the execution order.

[ritamerkl]

It all is still in the same order, it may look a bit confusing just looking at the diffs, but the only thing I changed is outsourcing the logic of the prompt to a new private function called ShowRestartWarning()