librelist archives

« back to archive

Assigning components to entities crashes if assignment would replace an existing component

Assigning components to entities crashes if assignment would replace an existing component

From:
David Wicks
Date:
2015-01-14 @ 17:41
I’m curious about the decision to disallow assigning a component to an 
entity that already has a component of that type. My expectation is that 
the newly assigned component would simply overwrite the previous 
component. Instead, entityx throws an assertion.

I would like to be able to simply write the assignment, and am happy to 
pay the minor price of destroying the previous component for this 
simplicity. In cases where I really cared about the performance of the 
assignment, I could avoid creating new components.

```
// I would expect the following to overwrite any existing Style component
e.assign<Style>( vec4( 1, 0, 1, 1 ) );
```

Instead, I need to write to avoid the assertion at present:
```
auto style = e.component<Style>();
if( style ) {
  style->color = vec4( 1, 0, 1, 1 );
}
else {
  e.assign<Style>( vec4( 1, 0, 1, 1 ) );
}
```

Is there a problem with entityx not calling the destructor of a previously
instantiated component? If so, perhaps there could be another method on 
entity that conditionally removes and assigns.

```
e.replace<C>( args ) {
  if( e.has_component<C> ) {
    e.remove<C>();
  }
  e.assign<C>( forward( args ) );
}
```

Thanks,
David
______________
David Wicks
sansumbrella.com

Re: [entityx] Assigning components to entities crashes if assignment would replace an existing component

From:
Alec Thomas
Date:
2015-01-15 @ 05:44
Hi David,  

This is the case because it’s not obvious that assigning a component can 
(sometimes) cause data to be destroyed. There are no surprises. I think 
adding a replace<C>() method is probably better (which makes it very clear
what the intent is):

entity.replace<Style>(vec4(1, 0, 1, 1));

get_or_assign<C>() seems to be another common pattern.  The latter could 
be handy for modifying individual fields while leaving any other fields 
intact, though it should probably only call the default constructor...

entity.get_or_assign<Style>()->color = vec4(1, 0, 1, 1);


Though maybe these should be helper functions. I’m not sure.

Alec

PS. I merged your PR :)  


On Thursday, 15 January 2015 at 4:41 am, David Wicks wrote:

> I’m curious about the decision to disallow assigning a component to an 
entity that already has a component of that type. My expectation is that 
the newly assigned component would simply overwrite the previous 
component. Instead, entityx throws an assertion.
>  
> I would like to be able to simply write the assignment, and am happy to 
pay the minor price of destroying the previous component for this 
simplicity. In cases where I really cared about the performance of the 
assignment, I could avoid creating new components.
>  
> ```
> // I would expect the following to overwrite any existing Style component
> e.assign<Style>( vec4( 1, 0, 1, 1 ) );
> ```
>  
> Instead, I need to write to avoid the assertion at present :
> ```
> auto style = e.component<Style>();
> if( style ) {
>   style->color = vec4( 1, 0, 1, 1 );
> }
> else {
>   e.assign<Style>( vec4( 1, 0, 1, 1 ) );
> }
> ```
>  
> Is there a problem with entityx not calling the destructor of a 
previously instantiated component? If so, perhaps there could be another 
method on entity that conditionally removes and assigns.
>  
> ```
> e.replace<C>( args ) {
>   if( e.has_component<C> ) {
>     e.remove<C>();
>   }
>   e.assign<C>( forward( args ) );
> }
> ```
>  
> Thanks,
> David
> ______________
> David Wicks
> sansumbrella.com (http://sansumbrella.com)
>  
>  
>  
>