Please allow EnsureOdinInspectorDefine to be optional

[Tarocco]

Odin keeps adding itself to the scripting define symbols list and my scripts which rely on that for #if/#else/#endif statements. Users who are not using Odin are unable to compile my project unless they manually remove ODIN_INSPECTOR from the list in player settings. Please allow this automatic adding to be optional.

[Tarocco]

I tried making another script that removes the define, but the compiler goes into a deadlock because calling PlayerSettings.SetScriptingDefineSymbolsForGroup() forces Unity to try to recompile, and then the EnsureOdinInspectorDefine script and my removal script begin to thrash.

If I could disable this automatic adding, my own script would be able to manage all of the defines for my project with something like the following:

var group = EditorUserBuildSettings.selectedBuildTargetGroup;
using (var context = new ScriptingDefinesOverride.Context(group))
{
    context.Define("SCRIPTING_DEFINES_OVERRIDE_ENABLED");
    context.Define("See_ScriptingDefinesOverride_cs_for_more_information");
    context.Define("SOME_OTHER_DEFINE");
    if(/*check for whether Odin inspector assembly is loaded*/)
        context.Define("ODIN_INSPECTOR");
    else
        context.Undefine("ODIN_INSPECTOR");
}

[Bjarke Elias]

Would placing the script that defines the ODIN_INSPECTOR as an actual .cs suffice? Instead of it being included in the assembly. That way you can just delete the script and the symbol after importing Odin.

Also, I'm curious, why would having a define symbol cause a build to not compile?

[Tarocco]

@Bjarkeck this would probably be perfect for what I am trying to do. It might need to provide instructions to remove that script for other team members who are using Odin. The issue really stems from Unity putting these defines into Library/ProjectSettings.asset (a file that is integral to the Unity project) and that Odin is managing its own define. If Odin is not present, the defines will be added when Odin users commit their changes.

My current workaround is kind of wild. It involves using a managed mcs.rsp file (which is excluded from the repository using .gitignore) and binary patching Sirenix.OdinInspector.Editor.dll.

For instance, I have...

static class AssemblyController
{
    static readonly string OdinMCSDefine = "-define:ODIN_INSPECTOR";
    static bool HasOdinInspector()
    {
        string path = Path.Combine(Application.dataPath, "Plugins", "Sirenix", "Odin Inspector");
        return Directory.Exists(path);
    }
    static bool AddDefineToMCS(ref List<string> mcs, string define)
    {
        if (!mcs.Select(s => s.Trim()).Contains(define))
        {
            mcs.Add(define);
            return true;
        }
        return false;
    }
    static AssemblyController()
    {
        //Debug.Log("Starting patching...");
        var patcher = new AssemblyPatcher();
        patcher.Run(Application.dataPath);
        //Debug.Log("Patching complete.");

        var mcs_path = Path.Combine(Application.dataPath, "mcs.rsp");
        var mcs = File.ReadAllLines(mcs_path).ToList();

        if (HasOdinInspector())
            AddDefineToMCS(ref mcs, OdinMCSDefine);

        File.WriteAllLines(mcs_path, mcs.ToArray());
    }
}

and

public class AssemblyPatcher
{
    #region Embedded Classes
    internal sealed class BoyerMoore
    {
        ...
    }
    internal static class BinaryReplace
    {
        ...
    }
    #endregion Embedded Classes
    private readonly string
        OdinInspectorDefineSymbol = "ODIN_INSPECTOR",
        OdinInspectorDefineSymbolReplacement = "ODIN_MANUALCTL",
        OdinInspectorEditorDLLFileName = "Sirenix.OdinInspector.Editor.dll";
    private BoyerMoore OdinInspectorDefineFinder;

    public async Task Run(string data_path)
    {
        var needle = OdinInspectorDefineSymbol
                .SelectMany(c => new byte[] { (byte)c, 0 })
                .ToArray();
        var replacement = OdinInspectorDefineSymbolReplacement
                .SelectMany(c => new byte[] { (byte)c, 0 })
                .ToArray();
        OdinInspectorDefineFinder = new BoyerMoore(needle);

        string[] odin_assemblies_to_patch = null;
        odin_assemblies_to_patch = await Task.Run(() => Directory.GetFiles(
            data_path,
            OdinInspectorEditorDLLFileName,
            SearchOption.AllDirectories));
        foreach (var dll_path in odin_assemblies_to_patch)
            await BinaryReplace.BinaryReplaceAllInFile(
                dll_path,
                OdinInspectorDefineFinder,
                replacement);
    }
}

This works surprisingly well. This automatically adds ODIN_INSPECTOR to mcs.rsp when it is imported and removes it when it is not. Including a .cs file like you mention would help, but it isn't a perfect solution. I feel like Unity could definitely use a better mechanism for feature toggles and script defines. In the meantime and until Unity are able to provide an elegant solution, the real solution involves the control mechanism being on a higher level than Odin itself (like this does).

And yes, Odin adding its script define automatically does prevent compiling for users who don't add Odin. They would have to remove the define because I have lots of Odin attributes on fields, etc. in my code. My workaround gets around this entirely because I am managing the script define in a way that is ignored by source control and is at a higher level / parent scope, i.e. not Odin trying to manage its own scripting define.

[Bjarke Elias]

Aaah of course, thanks for that! I'll take the script that defines #ODIN_INSPECTOR out so you at least can avoid having to modify the DLL file - So when you upgrade, deleting the script, and adding the mcs.rsp file should do it.

I can't actually really think of a better solution either, I think what you're doing is probably is what we would recommend other users as well. Not even sure what Unity could do in order to ease the pain. Maybe add something like

[assembly:UnityEngine.DefineSymbol("ODIN_INSPECTOR")]

And have them live in a separate read-only list of defines that would get automatically managed by Unity. But that's probably going to introduce a world of other problems.

[Bjarke Elias]

I've added the code-snippet below, as out-commented code to the EnsureOdinInspectorDefine.cs script which is now located in Plugins/Sirenix/OdinInspector/Scripts/Editor/EnsureOdinInspectorDefine.cs

//
// If you have a project where only some users have Odin, and you want to utilize the ODIN_INSPECTOR 
// define symbol. Then, in order to only define the symbol for those with Odin, you can delete this script, 
// which prevent ODIN_INSPECTOR from being added to the Unity's player settings.
// 
// And instead automatically add the ODIN_INSPECTOR define to an mcs.rsp file if Odin exists.
// You can then ignore the mcs.rsp file in source control.
// 
// Remember to manually remove the ODIN_INSPECTOR define symbol in player settings after removing this script.
//
static class AddOdinInspectorDefineIfOdinExist
{
    private const string ODIN_MCS_DEFINE = "-define:ODIN_INSPECTOR";

    [InitializeOnLoadMethod]
    private static void AddOrRemoveOdinDefine()
    {
        var addDefine = AppDomain.CurrentDomain.GetAssemblies().Any(x => x.FullName.StartsWith("Sirenix.OdinInspector.Editor"));

#if ODIN_INSPECTOR
        var hasDefine = true;
#else
        var hasDefine = false;
#endif

        if (addDefine == hasDefine)
        {
            return;
        }

        var mcsPath = Path.Combine(Application.dataPath, "mcs.rsp");
        var hasMcsFile = File.Exists(mcsPath);

        if (addDefine)
        {
            var lines = hasMcsFile ? File.ReadAllLines(mcsPath).ToList() : new List<string>();
            if (!lines.Any(x => x.Trim() == ODIN_MCS_DEFINE))
            {
                lines.Add(ODIN_MCS_DEFINE);
                File.WriteAllLines(mcsPath, lines.ToArray());
                AssetDatabase.Refresh();
            }
        }
        else if (hasMcsFile)
        {
            var linesWithoutOdinDefine = File.ReadAllLines(mcsPath).Where(x => x.Trim() != ODIN_MCS_DEFINE).ToArray();

            if (linesWithoutOdinDefine.Length == 0)
            {
                // Optional - Remove the mcs file instead if it doesn't contain any lines.
                File.Delete(mcsPath);
            }
            else
            {
                File.WriteAllLines(mcsPath, linesWithoutOdinDefine);
            }

            AssetDatabase.Refresh();
        }
    }
}

[Tarocco]

This is awesome @Bjarkeck -- thank you so much!!

[Ernest Suryś]

Is there a way to also get rid of “ODIN_INSPECTOR_EDITOR_ONLY“ from ProjectSettings.asset?