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

Support for Start() method that fires after constructor but before the first Update() #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ticking-clock
Copy link
Contributor

Adds an optional overridable Start() method on TestableComponent that will be called after the constructor but before the first Update(). Included a test for verification.

The purpose of this is to allow authors to write Component classes that feel more like they inherited from MonoBehaviour. It is also sometimes desirable to guarantee that code executes after Awake() (which in this case is simulated by the constructor).

I'd like to add support for Awake() but can't see a viable way to do it with injection (Awake must be called after all objects are initialized).

@@ -18,10 +19,17 @@ public class TestableComponent {

public void OnUpdate() {
if (enabled) {
if (!hadFirstUpdate) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this necessary? All you need to do is direct the Start() method of UnityGameObjectBridge through TestableGameObject, which you've already done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right; not only does this not belong here but results in Start() being called twice in Unity. I was trying to support the Start() method in tests since they don't instantiate UnityGameObjectBridge, but now I think that logic should go in Tests.TestUpdatableManager.

@ticking-clock
Copy link
Contributor Author

Fixed per banderous's comment and merged into single commit.

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

Successfully merging this pull request may close these issues.

2 participants