modesttree / Zenject

Dependency Injection Framework for Unity3D
MIT License
2.47k stars 272 forks source link

ScriptableObjects that referenced from objects in the scene aren't injected #175

Open doug-w opened 3 years ago

doug-w commented 3 years ago

Describe the bug I went to try what I thought would be a simple use of Extenject, put a Monobehavior in a scene, have some injection on it. Have it link to some ScriptableObjects via a field you can set in the editor, have those ScriptableObjects have some injection. Instead what I see is that the Monobehavior get it's injection called, and none of the fields do. If I manually Inject into one of those objects, it's injection is called but IT'S fields that are also waiting for injection don't.

To Reproduce

I created a sample project that demonstrates it at https://github.com/doug-w/extenject-scriptableobject

public class MonoInstall : MonoInstaller
{
    public override void InstallBindings()
    {
        Container.Bind<Holder>().FromComponentInHierarchy().AsSingle().NonLazy();
    }
}

public class DependenciesNotInjected : ScriptableObject
{
    public int someValue;
    public Holder holder;
    [Inject]
    public void ThisIsNeverCalled(Holder holder)
    {
        this.holder = holder;
        Debug.Log("You will never see this;");
    }
}

public class DepedencyIsInjectedManuallyButFieldOfItIsntInjected : ScriptableObject
{
    public DependenciesNotInjected thisWontBeInjected;
    public Holder holder;
    [Inject]
    public void ThisIsCalledFromHolderStart(Holder holder)
    {
        this.holder = holder;
        Debug.Log("Sandwhiched Between Start messages");
    }
}

public class Holder : MonoBehaviour
{
    public DependenciesNotInjected FirstNotInjected;
    public DependenciesNotInjected SecondNotInjected;
    public DepedencyIsInjectedManuallyButFieldOfItIsntInjected manualCall;
    private DiContainer _container;
    private bool _invokeOnce = false;
    [Inject]
    void ThisIsInvokedFine(DiContainer container)
    {
        Debug.Log("Invoked Holder's attribute message");
        _container = container;
    }
    void Start()
    {
        Debug.Log("About to Manually inject");
        _container.Inject(manualCall);
        Debug.Log("Finished Manual inject");
    }
    void Update()
    {
        if (!_invokeOnce)
        {
            _invokeOnce = true;
            Debug.Log($"In Update with First: {FirstNotInjected.holder?.name ?? "null"} second {SecondNotInjected.holder?.name ?? "null"} manual {manualCall.holder?.name ?? "null"} but manual's thiswontbeinjected; {manualCall.thisWontBeInjected.holder?.name ?? "null"}");
        }
    }
}

Output: Invoked Holder's attribute message About to Manually inject Sandwhiched Between Start messages Finished Manual inject In Update with First: null second null manual GameObject but manual's thiswontbeinjected; null

Expected behavior

If a Monobehavior in a scene has a field to scriptableobjects, those objects and other scriptableobjects referenced in THEIR fields get their [Inject] methods called. In the above demonstration "You will never see this;" should have been printed 3 times.

Screenshots image

Extenject and Unity info (please complete the following information):

Additional context

I opened all of the samples listed in the Extenject and the only reference to ScriptableObject was in the Zenject code itself, so I'd love to see some more samples or documentation on it. I'd be willing to morph the above URL to said sample if this problem can be understood.

WeslomPo commented 3 years ago

ScriptableObjects is same as static objects, kind of. This objects is exists even before Z create himself (and will live after you going back from play mode, or change scene). So Z can't inject them because it does not own them. Also this is a bad practice, you will suffer from memory leak if you will save runtime pointers in scriptable objects - better to avoid that. Treat SO like static unchangeable data and don't try to change their fields in runtime.

WeslomPo commented 3 years ago

I recommend you to change your mind. Inverse your logic a bit. You don't need MB here, and you don't need inject anything in SO, what you really need is make other object that own both MB and SO is being inject (better trough constructor), and do what you want to do from here.