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

Proposed fix for issue 229 Location loader #236

Merged

Conversation

jandd661
Copy link
Contributor

@jandd661 jandd661 commented Nov 29, 2020

Issue: #229

Fixed issue with it not unloading the previous scene correctly/reliably. Added some TODOs for future expansion. Also provided some test scenes to load additively and update the Beach scene Scene Loader to load those scenes additively. Also provided the associated LocationSOs for those scenes.

Video: https://youtu.be/VQWs_yalU6o

@ciro-unity
Copy link
Contributor

Hey @jandd661, a couple of things:

  • As soon as I checkout your branch, I have 7 meta files which get created by Unity.
    image
    I think it's because you haven't saved the assets in Unity before pushing, so the .meta files are not part of the PR. Always make sure to hit File > Save Project in the top menu before you commit for a PR.
  • The LocationExit script in the Beach scene has a bunch of null references, which throws an error when testing it. Always make sure to test your solution as a very last thing before committing.
    image
  • I removed a couple of comments which were really superfluous.

But I'll merge now!

@ciro-unity ciro-unity merged commit 8639c28 into UnityTechnologies:main Dec 6, 2020
@jandd661 jandd661 deleted the Location-Loader-issue-229-fix branch December 8, 2020 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug (critical) This needs to be fixed as soon as possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Location Loader issue
2 participants