Which is more readable?

[albinopapa]

Option 1:

Code: Select all

if( auto entity_iterator = parent.find_entity( _message.entity ); entity_iterator != parent.entities.end() )
{
	if( !_message.entity->has_all<Position, Shape>() )
	{
		parent.remove_entity( entity_iterator );
	}
}

Option 2:

Code: Select all

if( auto result = parent.is_compatible<Position, Shape>( _message.entity ); result.has_value() )
{
	if( auto[ entity_iterator, compatible ] = result.value(); !compatible )
	{
		parent.remove_entity( entity_iterator );
	}
}

They both do the same thing:

• search for a matching entity
• if found
  • check to see if it has all required components
  • if not all required components found
    • remove the entity

I definitely don't save myself any work with either option.

[Firepath]

Outer if of second and inner if of the first seem more human-readable. Assuming you can't have one without the other, I think the first is a little easier to read and understand in human speak.

[albinopapa]

Thanks for the feedback.

I see what you are saying, and now that I look at it again, there's nothing telling you result in the second one is going to be an std::optional of std::pair, nor that the bool in the std::pair is not associated with the succession of the first 'if' statement.

The issue with mixing ( not that you were suggesting to do so ) is if I have a valid iterator, then all I need to do is check if the entity has_all components (which I've renamed the function to has_all_components for clarity and aesthetics.

Was really wanting an excuse to use std::optional, I suppose I shouldn't force it. I still have to check if the optional object is valid here, same with checking the iterator so nothing gained by using it.

I have other functions that could throw exceptions if I didn't use std::optional, because they return a reference to possibly a non-valid value ). Since I'm using std::variant, it's possible that the object being held isn't the object being requested, and would cause an exception to be thrown. I could instead return an std::optional<std::reference_wrapper<T>> if the value is valid, or std::nullopt if the types do not match.

[albinopapa]

I'm on like my 4th rewrite of this ECS engine. I think I have finally come up with something simple, yet flexible. Instead of getting the underlying type all the time, I'm relying on the std::visit function to dispatch accordingly.

So, the above code has now turned into:

Code: Select all

// Pass a list of components to have verified that the entity has.
template<typename...ComponentList>
struct VerifyComponents
{
	// Every "entity" inherits the has_all_components() function so 
	// as long as I don't do something stupid like pass a system object, this overload
	// will be chosen.  
	template<typename T>
	auto operator()( const T& _entity )->
		std::enable_if_t<!std::is_same_v<T, std::monostate>, bool>
	{
		return _entity.has_all_components<ComponentList...>();
	}

	// If the type is std::monostate specifically, this overload is chosen.  All other types
	// SHOULD be an annoying compile time error.
	bool operator()( const std::monostate& _entity )
	{
		return false;
	}
};


if( auto it = parent.find_entity( _message.entity );
	it != parent.entities.end() )
{
	if( std::visit( VerifyComponents<Position, Shape>(), **it ) )
	{
		parent.add_entity( std::move( _message.entity ) );
	}
}

As mentioned in one of my other posts ( maybe ) std::visit can return values...as long as all visitors return the same value ( hopefully that's by design and not just a bug or misinterpretation on Microsoft's behalf ), so all the visitor functions return a bool and all is well.

I'm guessing nested templates are not very welcomed either in the language or just the VS compiler or just my installation of the VS compiler, because I tried putting the VerifyComponents struct nested in another templated class, VS shit the bed.

[chili]

You could have all of your visitor function return std::any if you need that kind of flexibility.

[albinopapa]

Well, the problem with that might be still not knowing which visitor returned a value, then I'm stuck figuring out how to any_cast.

In some places, I usually get the underlying object by searching the vector, then using std::get inside a reference captured lambda or check if the variant holds the current type using std::holds_alternative. In the case of entities, I search the component vector for a variant that is holding the component I'm looking for and then use std::get if I need the component, like changing the data for instance.

std::get<Position>( *pos_iter ) += ( std::get<Velocity>( *vel_iter ) * dt );

Unfortunately, this means I loop over the vector twice ( at least partially anyway ).

It would probably be faster if I used a vector of tuple and a bitset, the tuple would store an instance of each component that an Entity could possibly have, and the bitset would be a flag for enabling or disabling the component, but I think I want to try to variant approach.

[chili]

hmmm, i would have thought that any also had a visitor system implemented... Then again, i've barely looked at it

[albinopapa]

Haven't looked that far ahead at std::any either, so am not sure if it does or not. That being said, I guess it wouldn't be too difficult to come up with some function overloads to work that out...maybe?

I'm not very confident after what I've gone through with this damned variant ECS system. You really have to understand how the compiler sees things when dealing with concrete types and templates. For instance:

Code: Select all

using world_t = std::variant<  // World variant
	screws::ECS_World<
	std::variant<  // System variant
		screws::ECS_System< // Movable system
			MovableDispatcher,
			MovableMessageHandler,
			std::variant<  // Entity variant
				screws::ECS_Entity<player_tag, component_t>,
				screws::ECS_Entity<enemy_tag, component_t>>,
			std::variant<  // Message variant
				screws::ECS_Message<componentAdded_tag>,
				screws::ECS_Message<componentRemoved_tag>,
				screws::ECS_Message<entityAdded_tag>,
				screws::ECS_Message<entityRemoved_tag>,
				screws::ECS_Message<systemAdded_tag>,
				screws::ECS_Message<systemRemoved_tag>>,
			SystemMessageFilter>,
		screws::ECS_System<  // Draw system
			DrawableDispatcher,
			DrawableMessageHandler,
			std::variant<  // Entity variant
				screws::ECS_Entity<player_tag, component_t>,
				screws::ECS_Entity<enemy_tag, component_t>>,
			std::variant<  // Message variant
				screws::ECS_Message<componentAdded_tag>,
				screws::ECS_Message<componentRemoved_tag>,
				screws::ECS_Message<entityAdded_tag>,
				screws::ECS_Message<entityRemoved_tag>,
				screws::ECS_Message<systemAdded_tag>,
				screws::ECS_Message<systemRemoved_tag>>,
				SystemMessageFilter>>,
			std::variant<  // Message variant
				screws::ECS_Message<componentAdded_tag>,
				screws::ECS_Message<componentRemoved_tag>,
				screws::ECS_Message<entityAdded_tag>,
				screws::ECS_Message<entityRemoved_tag>,
				screws::ECS_Message<systemAdded_tag>,
				screws::ECS_Message<systemRemoved_tag>>,
	WorldMessageFilter>>;

So far, this is the only way I've gotten the project to compile. Using aliases was a nightmare to get the compiler to understand what I wanted. The issues I am having are the order in which things are seen and instantiated by the compiler.

I'd define an alias, but the type being aliased needed other long templated types, so I tried aliasing those types and found they needed to be defined somewhere before that because instantiating a template requires the type to be fully formed first, and so on.

In the end, I'm not finding variant to be a viable option for an ECS system even if ECS systems are suppose to be modular and not coupled, the fact that variant requires fully defined types before hand makes this decoupling a very difficult chore.