resharper-unity icon indicating copy to clipboard operation
resharper-unity copied to clipboard

Support disabling domain reloading

Open citizenmatt opened this issue 5 years ago • 5 comments

Unity 2019.3 has an experimental feature that disables domain reloading when entering play mode. This skips (re)initialising game state, so it's possible to have stale state that could easily introduce subtle bugs when testing in play mode. See also this forum thread which gives extra details, including a fantastic sequence diagram.

The big problem is that static fields do not get reset, and event handlers registered to static events (e.g. Application.quitting) are not unregistered. This can be handled with a method marked [RuntimeInitializeOnLoadMethod(RuntimeInitializationLoadType.SubsystemRegistration)] or [InitializeOnEnterPlayMode] that resets the static field. This applies to all classes, not just those deriving from UnityEngine.Object.

(Note that the example for unregistering a static event handler simply uses [RuntimeInitializeOnLoadMethod] without any parameters. I'm not sure if this is correct)

Rider can provide some help. If domain reloading is enabled:

  • Add a warning to any static field that is NOT written to in a method with either the [RuntimeInitializeOnLoadMethod] or [InitializeOnEnterPlayMode] attribute. If the field is assigned a value in such a method, don't show the warning. This applies to ALL types, not just Unity related.
  • Add a similar warning to any instance field marked with [NonSerialized]. These are skipped by script serialisation/deserialisation, and therefore would normally be reset by the app domain reloading and new instances being created. I think this also applies to private and internal fields.
    • Private and internal array or List fields that are not initialised and have a default value of null are automatically populated to empty arrays/lists by domain reload (scripts are serialised/deserialised, and this is a side effect of the serialiser). This will no longer happen and could bring null reference exceptions. (This behaviour needs testing)
  • Add a quick fix to re-initialise a field in a reset method. Create the method if it doesn't exist, add to end of body if it already exists, and set to default value. The name of the method doesn't matter.
  • Add a warning to any event subscription on a static event, unless the event is also unregistered in a [RuntimeInitializeOnLoadMethod] or [InitializeOnEnterPlayMode] attributed method.
    • Where the event is registered is important. If it's registered from [InitializeOnLoadMethod], that is only called on domain reload, so there would be no multiple registrations.
  • Editor classes should use [InitializeOnEnterPlayMode] instead of the runtime attribute. Add a warning if the wrong attribute is used?
  • Classes with [ExecuteInEditMode] miss out on a call to OnDisable/OnEnable that normally happens when the app domain is reloaded. Should this be highlighted?
  • Perhaps add tooltips to certain Unity event functions to say that they WILL NOT be called? E.g. for Awake in a class marked with [ExecuteInEditMode], something like "This method does not get called when entering play mode when domain reloading is disabled"? (This might actually be because of scene reloading)
  • [RuntimeInitializeOnLoad] doesn't work on generic classes (needs confirming, but makes sense - what would T be?). This might be worth a separate issue + warning. Presumably something similar with [RuntimeInitializeOnLoadMethod] and generic methods.

Some problems to figure out:

  • What if the field is a structure and reset, rather than re-initialised? E.g. a List<string> could be reset with myList.Clear(). Assume any called method re-initialises?
  • What about non-Unity classes? They will survive, too, and their instance fields will also need to be reset. Is this related to the above point? If all instances are re-assigned, it's not a problem?
  • How to handle registration of an anonymous event handler (lambda)? Would need to extract to method + add unregistration
  • What about private or internal instance fields that are not script instances? Should these be reset too?

You can also disable scene reloading, which normally resets current GameObjects and reloads the entire scene. Instead, Unity uses OnEnable, OnDisable and OnDestroy on the same instance to simulate resetting. It's not really clear what Rider can do here, but this forum post has some details on issues.

Bonus points if Rider can safely suggest disabling domain reload to speed up a project.

citizenmatt avatar Feb 12 '20 14:02 citizenmatt

Awesome points.

This new Unity feature might be the best one I've seen in years and one of the ones that made the most immediate positive impact yet on us.

Overall, I think better support for this in Rider might accelerate adoption of this awesome feature, so I'm all for it.

SugoiDev avatar Feb 13 '20 14:02 SugoiDev

How can Rider suggest this? Some ideas:

  • Try to monitor the length of time it takes to switch to play mode. If takes over e.g. 5 seconds, prompt to disable domain reload
  • The prompt could run a wizard that checked for the known issues above, presents them as a tool window to be fixed before disabling domain reload
  • Add a menu item to run this wizard on demand. Where does this live? How do we make this discoverable?
  • Add a "remind me later" to notification - snooze prompt for a week?
  • Can we run this check as part of SWEA? If so, can prompt to say "Your project looks like it will support fast play mode switching. Click here to enable"
  • Can we run the check even without SWEA? That doesn't sound too feasible
  • Have the inspections always enabled, even when fast play mode switching is not active. Alt+Enter will have a quick fix, plus a "don't tell me again" for this solution. Feels very intrusive - might need to do a lot of work to be able to disable domain reloading and you could have a lot of warnings that are just annoying
  • Add a warning to the Unity Editor plugin. If play mode switching takes too long, output a warning/info message to the console to suggest fast play mode switching. Downside is that logs don't have links, so we can't make this actionable.

citizenmatt avatar Feb 14 '20 16:02 citizenmatt

@SugoiDev any suggestions you have, throw them over. I think Rider can offer useful features once you've disabled domain reloading, but doesn't help with discoverability.

citizenmatt avatar Feb 14 '20 16:02 citizenmatt

I don't really have ideas on how Rider could pro-actively help the discoverability of this feature, and that may not be necessary seeing how good the feature is. It may also become the default in Unity at one point ?

Of course I'm all for the features you listed in your first post !

RDeluxe avatar Jun 15 '20 14:06 RDeluxe

Bumping this - would be nice to add certain code hints when this feature is enabled to help developers avoid subtle bugs.

TigerHix avatar May 19 '22 05:05 TigerHix