Skip to content

Enable use outside of play mode #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mcpiroman merged 5 commits into from
Apr 8, 2020
Merged

Enable use outside of play mode #12

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8d45bfd
Enables use outside of play mode
rogerbarton Mar 15, 2020
c7f9937
Merge branch 'master' into edit-mode
rogerbarton Mar 21, 2020
0b77423
Add enableInEditMode, improved and tested execute in edit mode
rogerbarton Mar 21, 2020
3bc630b
Update scripts/Editor/DllManipulatorEditor.cs
rogerbarton Apr 8, 2020
c95ef29
Review fixes
rogerbarton Apr 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions scripts/DllManipulatorScript.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
using System.Threading;
using System.Linq;
using UnityEngine;
using UnityEditor;

@mcpiroman mcpiroman Mar 16, 2020

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default scripts outside of Editor folder (like this one) don't have access to UnityEditor namespace, so you should move the implementation there (maybe sth like DllManipulatorEditor(Script)? actually DllManipulatorEditor seems very appropriate for it).

@rogerbarton rogerbarton Mar 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that you mean to merge the two files DllManipulatorScript and DllManipulatorEditor. Would it not make more sense to just move the DllManipulatorScript to the Editor folder and keep them separate?

@mcpiroman mcpiroman Mar 16, 2020

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really want to move the main executable script to Editor folder, because I want it to be able to run it outside editor. I know I said it isn't recommended to publish games with that way, but a quick Build and Run won't hurt.

What I'm thinking now is to move most of the code from DllManipulatorScript to DllManipulator (which should really be done anyway) and make separate, dedicated script in Editor folder (or maybe use existing DllManipulatorEditor?). Btw, does a script with [ExecuteInEditMode] have to be somehow instantiated in order to be run? I will look at this more tomorrow.

@rogerbarton rogerbarton Mar 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I'd personally keep the custom editor IMGUI code separate.

Yes there has to be an instance for [ExecuteInEditMode].

Makes all instances of a script execute in Edit Mode (Unity docs)

@mcpiroman mcpiroman Mar 19, 2020

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you maybe know if there is a way to bootstrap code in editor, without a script in the scene? Because now that would require instantiating two scripts, one 'normal' and one for editor, with [ExecuteInEditMode]. I remember searching for something like this some time ago, but I think I didn't find anything suitable. For instance, a constructor of a custom editor class get's called automatically, but only when you select the target script. And if there was a way to do that, I could potentially even cease requiring putting my script in the scene at all.

@rogerbarton rogerbarton Mar 19, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I know you always need an instance in the scene (for it to work without any additional scripting by the user). Theres only [RuntimeInitializeOnLoadMethod(RuntimeInitializeLoadType.BeforeSplashScreen)] but that would only be useful when using this tool in play mode.

I don't see why you would need two separate scripts, you can use Application.isPlaying and/or #if UNITY_EDITOR in the same script.

I guess you can use RuntimeInitializeOnLoadMethod for the 'normal' script and add an instance to the scene when you want to use it in edit mode as well? However, why would you not always use the edit mode version (given all the editor parts are in #if UNITY_EDITOR so you can still use it in a build)?

@mcpiroman mcpiroman Mar 19, 2020

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because scripts inside Editor folder are compiled into Assembly-CSharp-Editor assembly and these outside of Editor folder are compiled into Assembly-CSharp and only the former references UnityEditor.dll assembly, so you can't access the UnityEditor namespace in the non-editor scripts, even when UNITY_EDITOR symbol is specified. You can have custom assembly definition files that alter this behavior, but I don't want to force users to use them.

So that's how I remember it working, but now that I check it again I see that Assembly-CSharp also references UnityEditor.dll. So unless I messed this up, this behavior has changed in some version of unity. And if that's the case, I want be stay compatible with previous versions and not rely on this change.

As to the [RuntimeInitializeOnLoadMethod], attribute:

Methods marked [RuntimeInitializeOnLoadMethod] are invoked after the game has been loaded. This is after the Awake method has been invoked.
Note: The execution order of methods marked [RuntimeInitializeOnLoadMethod] is not guaranteed.

Which means that I can't safety use it, because some user code that uses a DLL could be have been run earlier, which would cause the DLL to be loaded by Unity (mono to be specific), and disallow further unloading.

@rogerbarton rogerbarton Mar 19, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, however, I understood it that the Editor folders are just to save you from putting #ifs and you can still freely access the UnityEditor namespace outside of such a folder. Files in an Editor folder are not included in a build. So I think it should be fine if we use #ifs and put the script outside the Editor folder. (citing docs and this from 2015)

Regarding the attribute, I think that's because the default load type is after the scene has been loaded. See decompiled ctor below.

public RuntimeInitializeOnLoadMethodAttribute()
{
  this.loadType = RuntimeInitializeLoadType.AfterSceneLoad;
}

So if you specify the BeforeSplashScreen or BeforeSceneLoad LoadType it might work as intended.

@mcpiroman mcpiroman Mar 19, 2020

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked the Unity 2018 and your're right, although not all UnityEditor assemblies are referenced, the core one is and can be accessed. So this may freely stay how you initially proposed, but with #if UNITY_EDITOR checks.

@mcpiroman mcpiroman Apr 8, 2020

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still hangs there, although it is being guarded below

using UnityNativeTool.Internal;

namespace UnityNativeTool
{
[ExecuteInEditMode]
public class DllManipulatorScript : MonoBehaviour
{
private static DllManipulatorScript _singletonInstance = null;
Expand Down Expand Up @@ -41,11 +43,15 @@ private void OnEnable()

if (_singletonInstance != null)
{
Destroy(gameObject);
if (EditorApplication.isPlaying)
Destroy(gameObject);
else
enabled = false;
return;
}
_singletonInstance = this;
DontDestroyOnLoad(gameObject);
if(EditorApplication.isPlaying)
DontDestroyOnLoad(gameObject);

var timer = System.Diagnostics.Stopwatch.StartNew();

Expand Down Expand Up @@ -85,7 +91,7 @@ private void OnEnable()
InitializationTime = timer.Elapsed;
}

private void OnDestroy()
private void OnDisable()

@mcpiroman mcpiroman Mar 16, 2020

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OnDestroy was used deliberately as it is at the latest called appropriate callback (after OnDisable), which makes things slightly safer (although, admittedly, not fully) in preventing users from calling native dll after it has been unloaded. So it be best if this could stay that way. Perhaps you could use sth like additional flag field?

@rogerbarton rogerbarton Mar 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I think about it, there is no reason to have OnDisable, so we can leave this at OnDestroy. The user can manually unload the dlls when required.

@mcpiroman mcpiroman Mar 19, 2020

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But is it always called when the assembly gets reloaded and when you start in-editor playback? Otherwise it could leak a DLL reference if user didn't perform explicit unload.

@rogerbarton rogerbarton Mar 21, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit see next comments You're right, OnDestroy is not called when exit mode is left if we use [ExecuteInEditMode]. I don't quite understand what you mean in your last comment.

As a better alternative to OnDestroy to call Unload after everything, we can use EditorApplication.playModeStateChanged with the EnteredEditMode state: (example from the docs)

using UnityEditor;

// ensure class initializer is called whenever scripts recompile
[InitializeOnLoadAttribute] //<- not relevant for this point
public static class PlayModeStateChangedExample
{
    // register an event handler when the class is initialized
    static PlayModeStateChangedExample()
    {
        EditorApplication.playModeStateChanged += LogPlayModeState;
    }

    private static void LogPlayModeState(PlayModeStateChange state)
    {
        if(state == PlayModeStateChange.EnteredEditMode)
            // UnloadAll
    }
}

This is executed after all OnDestroys but is in a static context. However, this should work as we have the _singletonInstance variable.

@rogerbarton rogerbarton Mar 21, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a build you would still need to use the OnDestroy of course, so more #ifs.

@rogerbarton rogerbarton Mar 21, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems more complicated than I thought. When the play mode is entered/left the scene is reloaded, so the instance is reset and the static variables as well. So we can't use what I stated above.

So the ordering goes like this

//Recompile/start Unity
OnEnable

//Press Play
ExitingEditMode
OnDisable()
OnDestroy()
//Scene Reloaded and play mode entered
OnEnable()
EnteredPlayMode

//Stop Play mode
ExitingPlayMode
OnDisable()
OnDestroy()
//Scene Reloaded edit mode entered
OnEnable()
EnteredEditMode

This means that in fact leaving it mostly as is without playModeStateChanged and using OnEnable and OnDestroy is the best. The dll's will be loaded/unloaded often, but as we said below you can disable it in edit mode easily if you want.

{
if (_singletonInstance == this)
{
Expand All @@ -96,6 +102,7 @@ private void OnDestroy()
DllManipulator.UnloadAll();
DllManipulator.ForgetAllDlls();
DllManipulator.ClearCrashLogs();
_singletonInstance = null;
}
}
}
Expand Down