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

SpawnInsideModifier not resetting on scene reload (with fix) #648

Open
demi180 opened this issue Apr 3, 2018 · 5 comments
Open

SpawnInsideModifier not resetting on scene reload (with fix) #648

demi180 opened this issue Apr 3, 2018 · 5 comments
Assignees
Milestone

Comments

@demi180
Copy link

demi180 commented Apr 3, 2018

This has been the case since we started working with Mapbox, just now got around to fixing it. For our use we often need to just reload the current scene (easier than trying to reset specific objects).

With previous versions the buildings were also having trouble reloading, but now that seems to be working. But trees and other objects spawned with the SpawnInsideModifier are not properly respawning and instead an exception happens. Because the modifiers are ScriptableObjects, and everything still holds the temp data in SOs instead of in a runtime object.

To recreate, simply take a scene with some tree spawns, add in a script to reload current scene, ex:
SceneManager.LoadScene ( SceneManager.GetActiveScene ().name );
My solution was to just safely destroy and clear any objects in this modifier object, like this:

public override void Initialize()
{
	if (_objects == null || _pool == null)
	{
		_objects = new Dictionary<GameObject, List<GameObject>>();
		_pool = new Queue<GameObject>();
	} else
	{
		foreach ( var pair in _objects )
		{
			foreach ( GameObject go in pair.Value )
				if ( go != null )
					Destroy ( go );
			if ( pair.Key != null )
				Destroy ( pair.Key );
		}
		_objects.Clear ();
		while ( _pool.Count > 0 )
		{
			GameObject go = _pool.Dequeue ();
			if ( go != null )
				Destroy ( go );
		}
	}
}
@jordy-isaac
Copy link
Contributor

Thanks for letting us know and for your feedback on this @demi180

@abhishektrip
Copy link
Contributor

@brnkhy Could you take a look please?
@demi180 Thanks for reporting this issue.

@brnkhy
Copy link
Contributor

brnkhy commented Apr 10, 2018

@demi180 thanks for the detailed ticket. I think we tend to look over scene change cases as we focus more on map reloads (without changing the scene).
One problem with your solution is that I believe that'll break caching for map reloads as you're clearing the pool on Initialize event. Of course if you're not doing that, it should be fine.
Actually what's happening is; we reset map when it's actually destroyed (OnDestroy) and for optimization purposes, we cache objects. Then scene reload destroys all the objects of course and we end up with missing reference exceptions.
solution, theoretically, should be easy; have two different methods for reset&destroy and handle objects different for each.
We were talking about separating initialize and reset, adding update etc for a long time so we'll add destroy to that list as well.

cc @atripathi-mb

@demi180
Copy link
Author

demi180 commented Apr 10, 2018

@brnkhy yes, you're right. Our particular case isn't moving around the map, we're just using one chunk at a time. I'd be just as happy to keep all the cached data on reload.

I could probably just add a DontDestroy and a check to our map object to circumvent this problem entirely, now that I think about it. I can even do that without modifying any of your scripts. I have no specific need to re-randomize the building textures and tree placement. In fact it'd be easier for the trees to stay the same in this case, since we've got Python scripts that have to receive all the collision information...
Anyway thanks again!

Edit: Looks like I do need minor modification for that, because AbstractMap.OnDestroy is still being called from the Destroy call inside the DDOL.

@brnkhy
Copy link
Contributor

brnkhy commented Apr 12, 2018

@demi180 glad to hear it works well for now. We'll probably look into this soon as a part of bigger review/refactor so won't rush a fix if everything works for you now.

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

5 participants