BrokenBricksECS icon indicating copy to clipboard operation
BrokenBricksECS copied to clipboard

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

Open acoppes opened this issue 6 years ago • 4 comments

When you remove a Component calling the manager.RemoveComponent<SpecificComponent>(), 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));
	}

acoppes avatar Jan 16 '18 00:01 acoppes

Thank you I'll fix this

Spy-Shifty avatar Jan 16 '18 09:01 Spy-Shifty

Is it fixed yet?

Gotcab avatar Apr 12 '18 23:04 Gotcab

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

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

dfraska avatar Apr 20 '18 20:04 dfraska

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.

dfraska avatar May 09 '18 18:05 dfraska