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

CustomComponentLookups not working with structs? #19

Open
GetFunctional opened this issue Apr 21, 2020 · 5 comments
Open

CustomComponentLookups not working with structs? #19

GetFunctional opened this issue Apr 21, 2020 · 5 comments

Comments

@GetFunctional
Copy link

GetFunctional commented Apr 21, 2020

Hi,

I'm new to your framework and followed your guide from this page ( https://ecsrx.gitbook.io/project/performance/component-type-lookups ).

I created a Module for the Customlookups and added it to my application like this:

 protected override void LoadModules()
  {
        base.LoadModules();
        this.Container.LoadModule(new CustomComponentLookupsModule());
   }

When adding a component, that is a struct and was registered in the dictionary, to an entity via Blueprint I get an exception:

System.InvalidCastException
HResult=0x80004002
Message=Unable to cast object of type 'EcsRx.Components.ComponentPool1[GF.FriendsInSpace.Core.Components.EntityIdComponent]' to type 'EcsRx.Components.IComponentPool1[EcsRx.Components.IComponent]'.
Source=EcsRx
StackTrace:
at EcsRx.Components.Database.ComponentDatabase.GetPoolFor[T](Int32 componentTypeId)
at EcsRx.Components.Database.ComponentDatabase.Set[T](Int32 componentTypeId, Int32 allocationIndex, T component)
at EcsRx.Entities.Entity.AddComponents(IReadOnlyList`1 components)
at EcsRx.Extensions.IEntityExtensions.AddComponents(IEntity entity, IComponent[] components)

I can work around this when replacing the struct with a class. But it would be great if you could make it work again or if I made a mistake maybe you can offer a solution :-)

@grofit
Copy link
Member

grofit commented Apr 21, 2020

Can you show what your registration logic looks like and what the component looks like?

It seems odd, as I know lots of bits (especially the Batching plugin) get the components and run with structs, but maybe they are not getting the pool, but either way lots of bits set components as structs in examples.

Are you able to make a simple repro that I can just clone and look at directly? my time is limited at the moment or I would go dig a bit deeper. I have kids running around trying to injure each other until late at night, and then I have a raft of other stuff to catch up on.

Also in most cases you never need to manually register stuff, its only if you want to save the lookup cost incurred if you know ahead of time the type you want, so if you are doing this for performance reasons then carry on as you are, if you are doing this for any other reasons it may be best to just use the default component type lookup system which does all the heavy lifting for you.

@GetFunctional
Copy link
Author

Hey grofit

I'm pretty busy as well at the moment but I will try to provide a small reproduceable project soon :-)

@GetFunctional
Copy link
Author

GetFunctional commented Apr 24, 2020

I think i found the issue. I think it has to do with the extensionmethods I used from EcsRx.Extensions. I created a small reproduceable console project for you to have a look.
Issue19.zip

Making the EntityIdComponent a class "resolves" the issue

@grofit
Copy link
Member

grofit commented Apr 24, 2020

Right having a look over and I think there could do with being a few changes, as currently there is only one method to add a struct component, and you dont pass in the component you pass in the generic and the id for it, then it provides you the default component. There is a workaround where with structs you can actually just call GetComponent<T>(tyoeId) and get back an empty/current component for that entity.

So the other simpler methods for adding components are all for class based ones.

In hindsight this seems rubbish developer experience, so I think this could do with some changes, and hopefully it shouldn't be a case of moving mountains, but will need to look into it some more to see what the ramifications are for it, as struct and classes have slightly different flows through the underlying parts of the system due to the way they are handled.

Anyway to get you up and running if you change your code to this:

        public void Apply(IEntity entity)
        {
            var gameTimeComponent = new GameTimeComponent  {CurrentTime = this._currentGameTime};
            entity.AddComponents(gameTimeComponent);
            
            ref var gameEntityIdComponent = ref entity.AddComponent<EntityIdComponent>(ComponentLookupTypes.EntityIdComponentId);
            gameEntityIdComponent.EntityId = _currentGameEntity;
        }

It should do what you expect, however as I say this probably needs better documentation and potentially an improvement to the way you can create and add components.

@grofit
Copy link
Member

grofit commented Apr 24, 2020

I started a conv on the discord server about this topic as it touches on a few problem areas and I dont think I can easily wrangle the existing API to be able to let classes/structs mix nicely in the same method.

There was a lot of chat a while ago about moving entities to be structs, and some research was done into it, but it may be that rather than spending more time on allowing variations of adding classes/structs together look more into the entity changes then revisit this topic, as then the storage/interaction will be in 2 separate classes (well class and struct).

So feel free to drop in if you have any strong feelings on stuff.

Also worth pointing out, originally you never really bulk added components you did them one at a time, but the performance hit on group changes was quite large, so we added the AddComponents as a sort of performance improvement there as you could batch together multiple additions. So one thing that was originally going to be possible (potentially) with the newer entity separation was better batching/transaction-like handling, so you could just do lots of operations then just "commit" them (which would basically remove these pre-batched methods).

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

No branches or pull requests

2 participants