Skip to content
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.

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

MonoBehaviour/Components injection issues #45

Closed
josimard opened this issue May 13, 2016 · 8 comments
Closed

MonoBehaviour/Components injection issues #45

josimard opened this issue May 13, 2016 · 8 comments
Assignees
Milestone

Comments

@josimard
Copy link

Hi,
we had problems troubleshooting injection issues when requesting bindings with components.

Unity did throw some warning, but the stack made it hard to troubleshoot:

You are trying to create a MonoBehaviour using the 'new' keyword. [...]

An error should be implemented to help users pin-point the issue fast. The issue can happen often when using injection with components, either when component values are null or when the binding order is wrong.

I managed to implement an error message that greatly helps finding from which components are problematic:
josimard@21cab92

Ultimately, we did not find how to specify the injector not to instantiate some types... It was hard understand why we had multiple instances of our behaviors when using injection at first. We are not sure why the binding order is so important in our case...

This is how we bind component instances in our container:

container.Bind<T>().To(instance);

@intentor intentor added this to the v2.18 milestone May 23, 2016
@intentor intentor self-assigned this May 23, 2016
@intentor
Copy link
Owner

Thanks for the feedback and solution!

I'll look into the issue and analyse your solution to incorporate it on an official release.

@josimard
Copy link
Author

Hi André,
Thanks for considering,

Alternatively, do you know of a way we could prevent the injector to instantiate,
that it would return null without an instance binding?

@intentor
Copy link
Owner

Currently, there's no way to do that. Adic will always try to instantiate. However, that can be developed!

I opened the issue #46 to develop this new feature.

@intentor intentor modified the milestones: v2.19, v2.18, v2.20, v2.21 Jul 13, 2016
@intentor intentor modified the milestones: v2.21, v2.20, v2.22 Jul 25, 2016
@moathlowahi
Copy link

moathlowahi commented Oct 7, 2016

It's not because "Adic will always try to instantiate", it's because there is a goddman fucking bug in the code!
File: Binder.cs || Line: 58
"this.AddBindingToDictionary(new BindingInfo(valueType, valueType, BindingInstance.Singleton));"
Should be:
"this.AddBindingToDictionary(new BindingInfo(valueType, binding.value, BindingInstance.Singleton));"

@intentor
Copy link
Owner

intentor commented Oct 7, 2016

Hello @moathlowahi!

Thanks for your reply. I'll check and incorporate it on the next release!

However my answer to @josimard remains correct and the issue #46 added the feature he commented about, as described on https://github.com/intentor/adic#instance-resolution-modes.

The code you used as example for the issue is part of the feature added on #46. A singleton of the value being added is always added to the container regardless of the binding type, so it can be reused inside the container, keeping it singleton (as described on #44).

intentor added a commit that referenced this issue Oct 7, 2016
@moathlowahi
Copy link

moathlowahi commented Oct 7, 2016

Before changing that piece of code, whenever you try bind a Singleton as an Instance for the first time to another interface, Adic will try to instantiate it, but this is wrong since that Singleton is provided as an Instance in this case. To reproduce this effect you can do something like this:
myContainer.Bind<IWhateverInterface>().To<MyMonoBehaviour>(FindObjectOfType<MyMonoBehaviour>());

You will get the following error:

You are trying to create a MonoBehaviour using the 'new' keyword. [...]

N.S: This is not because of FindObjectOfType<MyMonoBehaviour>() returning null or any other silly coding mistake.

@intentor
Copy link
Owner

intentor commented Oct 8, 2016

Hello, @moathlowahi!

Thanks for your contribution! I've already been able to reproduce the issue and a fix has been pulled. It'll be available on the version released later next week.

intentor added a commit that referenced this issue Nov 8, 2016
@intentor
Copy link
Owner

The pull request #64 added better descriptive messages.

This issue will be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants