Skip to content
This repository has been archived by the owner on Aug 30, 2018. It is now read-only.

OnEntityRemoved/Removing called when Component is removed doesn't allow accessing the removed component. #21

Open
acoppes opened this issue Jan 16, 2018 · 4 comments

Comments

@acoppes
Copy link

acoppes commented Jan 16, 2018

When you remove a Component calling the manager.RemoveComponent(), the entity removing and remove are called to all groups interested in that component, however, they are called without that component.

I believe the system may want to react to the component being removed to remove data related to that component, in some cases with a reference to that data stored in the component itself or being used as identifier, etc.

To test it I am using the following code:

	class ListenerMock : IEntityAddedEventListener, IEntityRemovedEventListener, IEntityRemovingEventListener
	{
		public int addedCalls;

		public int removedCalls;

		public int removingCalls;

		public event Action<Entity> RemovedEvent;

		public event Action<Entity> RemovingEvent;

		#region IEntityAddedEventListener implementation
		public void OnEntityAdded (object sender, Entity entity)
		{
			addedCalls++;
		}

        public void OnEntityRemoved(object sender, Entity entity)
        {
            removedCalls++;
			if (RemovedEvent != null)
				RemovedEvent(entity);
        }

        public void OnEntityRemoving(object sender, Entity entity)
        {
            removingCalls++;
			if (RemovingEvent != null)
				RemovingEvent(entity);
        }
        #endregion

    }


	[Test]
	public void EntityRemovedCallbackShouldBeCalledWithComponents() {
		var entityManager = new EntityManager ();
		
		var group = entityManager.GetComponentGroup (typeof(Component1), typeof(Component2));

		var listenerMock = new ListenerMock ();
		group.SubscribeOnEntityAdded(listenerMock);
		group.SubscribeOnEntityRemoved (listenerMock);
		group.SubscribeOnEntityRemoving(listenerMock);

		var e = entityManager.CreateEntity ();

		entityManager.AddComponent (e, new Component1());
		entityManager.AddComponent (e, new Component2());

		Assert.That (listenerMock.addedCalls, Is.EqualTo (1));

		listenerMock.RemovingEvent += delegate (Entity e1) {
			Assert.True(entityManager.HasComponent<Component1>(e1));
			Assert.True(entityManager.HasComponent<Component2>(e1));
		};


		entityManager.RemoveComponentImmediate<Component1>(e);

		Assert.That (listenerMock.removingCalls, Is.EqualTo (1));
	}

It fails also using the normal remove:

	[Test]
	public void EntityRemovedCallbackShouldBeCalledWithComponents2() {
		var entityManager = new EntityManager ();

		var systemRoot = new SystemRoot<EntityManager>(entityManager);

		var group = entityManager.GetComponentGroup (typeof(Component1), typeof(Component2));

		var listenerMock = new ListenerMock ();
		// group.SubscribeOnEntityAdded(listenerMock);
		// group.SubscribeOnEntityRemoved (listenerMock);
		group.SubscribeOnEntityRemoving(listenerMock);

		// group.component

		var e = entityManager.CreateEntity ();

		entityManager.AddComponent (e, new Component1());
		entityManager.AddComponent (e, new Component2());

		// Assert.That (listenerMock.addedCalls, Is.EqualTo (1));

		// listenerMock.RemovedEvent += delegate (Entity e1) {
		// 	Assert.True(entityManager.HasComponent<Component1>(e1));
		// 	Assert.True(entityManager.HasComponent<Component2>(e1));
		// };

		listenerMock.RemovingEvent += delegate (Entity e1) {
			Assert.True(entityManager.HasComponent<Component1>(e1));
			Assert.True(entityManager.HasComponent<Component2>(e1));
		};


		entityManager.RemoveComponent<Component1>(e);
		// entityManager.RemoveComponentImmediate<Component1>(e);

		systemRoot.Update();

		Assert.That (listenerMock.removingCalls, Is.EqualTo (1));
	}
@Spy-Shifty
Copy link
Owner

Thank you I'll fix this

@Gotcab
Copy link

Gotcab commented Apr 12, 2018

Is it fixed yet?

@dfraska
Copy link

dfraska commented Apr 20, 2018

I was able to address this by changing ComponentWrapper.cs lines 82 and 149 from:
gameObjectEntity.EntityManager.RemoveComponent(gameObjectEntity.Entity);
to:
gameObjectEntity.EntityManager.RemoveComponentImmediate(gameObjectEntity.Entity);

I'm not 100% sure if this will have any other effects, but so far it has worked for me.

@dfraska
Copy link

dfraska commented May 9, 2018

The above fix causes issues with setting GameObjects inactive. I'm no longer seeing the original issue in my code... but I've made other bug fixes in my own non-public branch.

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

No branches or pull requests

4 participants