alecthomas / entityx

EntityX - A fast, type-safe C++ Entity-Component system
MIT License
2.21k stars 295 forks source link

Update to brace initialisation to allow for easy aggregate use #213

Closed radman0x closed 6 years ago

radman0x commented 6 years ago

As discussed in #212, update relevant initialisation locations to use {} instead of (). Also updated tests to pass the right types to avoid narrowing conversion errors.

radman0x commented 6 years ago

@alecthomas, looks like the travis builds are broken due to linux apt errors. I think this is due to the use of 14.04 as the compile platform which has gone EOL and had all apt repositories removed...

alecthomas commented 6 years ago

I don't think this works correctly. If you remove the constructors from the components, this won't compile, which is the intent correct?

alecthomas commented 6 years ago

After a bit more reading (which I should have done earlier), I don't think it is possible to make this work correctly as desired.

radman0x commented 6 years ago

Definitely does work if the components are aggregates. To qualify as an aggregate a class needs to have: No user defined constructors No defaulted members No virtual functions

Check out aggregate initialisation on cppreference https://en.cppreference.com/w/cpp/language/aggregate_initialization

alecthomas commented 6 years ago

Try it with the Tag class.

radman0x commented 6 years ago

I looked at TagComponent and that probably doesn't qualify as an aggregate due to the inheritance structure. The case that I've been using thus for is defining my own structs and using them as components without inheriting from any entity base. Adding brace init allows these simple classes not to have constructors defined. However it won't remove the requirement for known simple aggregate classes to need constructors defined.

alecthomas commented 6 years ago

Ah right, sorry, I totally forgot about the inheritance. I'm going to remove Component as well, as it's completely redundant now.