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

Release null objects from MemoryPoolBase #446

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

Release null objects from MemoryPoolBase #446

wants to merge 1 commit into from

Conversation

NoxMortem
Copy link

In Unity MemoryPool entries can become null through calls to Object.Destroy(entry).

When the MemoryPool returns a value, which has been destroyed, this will cause problems. This is in particular problematic for MonoMemoryPool which can hold references to objects in a scene which is unloaded. I've added a minor addition to MemoryPoolBase which will release items which are null. It now pops items until it finds one which is not null and expands the pool whenever the last item had been popped (e.g. all items are destroyed because they belonged to an unloaded scene)

…estroy(entry). When the MemoryPool returns a value, which has been destroyed, this will cause problems. This is in problematic for MonoMemoryPool which can hold references to objects in a scene which is unloaded. I've added a minor addition to MemoryPoolBase which will release items which are null. It now pops items until it finds one which is not null and expands the pool whenever all items are popped (e.g. all items are destroyed because they belonged to an unloaded scene)
@svermeulen
Copy link

If that happens isn't that a mistake? Like shouldn't the fix be to not delete objects that have been added to the pool instead of fixing it inside the memory pool class?

@NoxMortem
Copy link
Author

NoxMortem commented Jun 18, 2018

@svermeulen this usually is a problem in indirect scenarios where Unity does destroy the object and not the user via Object.Destroy , e.g. a scene change, which destroys all gameobjects part of the scene. The user could likely work around this by ensuring that pooled objects are move to the DontDestroyOnSceneLoad scene.

However - at least I believe so - it would be very difficult to keep all memory pools clean when you unload a scene. If there is e.g. a class SceneSystem which handles loading/unloading it would require a reference to all memory pools which spawn objects in any scene it potentially unloads, s.t. it can despawn all items from all pools before unloading the scene?

In other scenarios I guess it should be really the best way to keep the pool clean in the first place, e.g. spawn, delete and never despawn deleted object.

@svermeulen
Copy link

You're saying that you might want to continue using the memory pool when the scene changes? So it is bound in the project context?

@NoxMortem
Copy link
Author

NoxMortem commented Jun 18, 2018

@svermeulen ProjectContext anmd/or a SceneContext of a scene in multi-scene (additive loading, scene parenting) scenarios which is not unloaded.

@svermeulen
Copy link

In that case though, shouldn't the inactive pooled objects already be placed in the same scene where the pool is bounded? I don't understand why they are being deleted. One thing that does occur to me is that the old version of MonoMemoryPool was not resetting the parent automatically during despawn (but it is now in zenject 6), could that be related?

@NoxMortem
Copy link
Author

NoxMortem commented Jun 18, 2018

@svermeulen Yes it should be and that could have been the problem bacck then. I will investigate and report later. Might take me a few weeks, just give me 2-3 weeks before we close this.

@svermeulen
Copy link

Sure no problem

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.

None yet

2 participants