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

Parent game objects #156

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

in3orn
Copy link

@in3orn in3orn commented Jul 6, 2020

Motivation

In bigger projects it may be convenient to use GameObjectContext as a parent for SceneContext.

Consider case where you have some non-standard context that is recreated every time player switches accounts / characters. The best approach would be to use scene contexts and their contracts functionality. However in some legacy projects it can be pretty hard and time consuming to achieve this (e.g. because of the usage of LoadSceneMode.Single). Switching to LoadSceneMode.Additive may expose many issues connected with scene unloading order.

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • No compiler errors or warnings

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Issue Number

Issue Number: N/A

Create or search an issue here: Extenject/Issues

What is the current behavior?

  • Currently it's not possible to use GameObjectContext as parent for SceneContext.

What is the new behavior?

  • GameObjectContext has additional field named ContractNames which is used within the initialisation of SceneContext.
  • Introduced field is used as a fallback value - when SceneContext cannot find the parent SceneContext for given ParentContractName, it searches for matching GameObjectContext.

Does this introduce a breaking change?

  • Yes
  • No

Other information

  • There's GameObjectAsParentContract test added to the Integration Tests section.

On which Unity version has this been tested?

  • 2020.4 LTS
  • 2020.3
  • 2020.2
  • 2020.1
  • 2019.4 LTS
  • 2019.3
  • 2019.2
  • 2019.1
  • 2018.4 LTS

Scripting backend:

  • Mono
  • IL2CPP

Note: Every pull request is tested on the Continuous Integration (CI) system to confirm that it works in Unity.

Ideally, the pull request will pass ("be green"). This means that all tests pass and there are no errors. However, it is not uncommon for the CI infrastructure itself to fail on specific platforms or for so-called "flaky" tests to fail ("be red"). Each CI failure must be manually inspected to determine the cause.

CI starts automatically when you open a pull request, but only Releasers/Collaborators can restart a CI run. If you believe CI is giving a false negative, ask a Releaser to restart the tests.

@Mathijs-Bakker Mathijs-Bakker self-requested a review July 6, 2020 19:50
@Mathijs-Bakker Mathijs-Bakker added the enhancement New feature or request label Jul 6, 2020
@sceeter89
Copy link

Would it be possible to merge this PR? Anything we can do to make it happen? Thanks 🙏

@Mathijs-Bakker
Copy link
Collaborator

I am reluctant to merge this. As I am not convinced it solves the problem.

The GOC is a sub container of the Scene Context. When you change this behavior you just make it unpredictable for the users.

There must be another way to solve the issue.

Eg.

  1. Add another (dedicated) parent context for the SC.
  2. Make this available as an 'Optional' module for Zenject. As this targets legacy projects.
  3. ...

@sceeter89
Copy link

Actually it does not affect only legacy projects (however mostly), but also cases where complex scene management is an overkill compared to having some persistent context.

Actually what we would like to see here is something between ProjectContext and SceneContext. Make use of GameObjectContext is quite interesting as there are no new concepts here: contracts are already known, it's just allowing contracts from SceneContext to GameObjectContext.

Could you elaborate a bit more on "you just make it unpredictable for the users."? My understanding of this PR is:
"We should be able to set named contract and abstract away if it's provided by SceneContext or GamObjectContext".

Another approach I can think of is to make use of SubContainers, however currently (if I understand it correctly) it might be tricky to dynamically remove and add bindings at runtime.

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

Successfully merging this pull request may close these issues.

3 participants