Green-Sky/imgui_entt_entity_editor

Question about entt::component

GnikDroy opened this issue ยท 12 comments

I don't seem to be able to compile my project with recent entt versions and your library.
Surprisingly, the demo code works perfectly. This is more of a question than an issue. Where is the entt::component type that your code uses internally? I cannot find it in entt's source.

Edit: Okay, so it seems to be present on the latest release version but not included in the current master branch.
So, entt has decided to not include the entt::component in newer versions I assume.

Well, entt::component got introduced in EnTTv3.1.0 and replaced Registry::component_type.
The current master seems to have changed that again to ENTT_ID_TYPE, which is a define to std::uint32_t (https://github.com/skypjack/entt/blob/8aacd4d02252e1a3af8cc84a1c58acde52b004e3/src/entt/config/config.h#L30).
I will look into it more later.

Edit: This commit clarifies more: skypjack/entt@ed8eea1

So, I had a need for the entity-editor for the latest EnTT. Also, skypjack expects a new release within a month so it will get updated soon. Therefore, I decided to rewrite this for my engine. I also wanted a clearer API.
https://gist.github.com/GnikDroy/f15446f23963d3800d28f0d0bb9bae63
This is a snippet what I have so far. This is also heavily influenced from how you wrote your code, yet has some major API breaking changes.

If you like the API I will make a fork and send you a pull request. Or, maybe we can work together to improve this.

  • the usage
    k2::EntityEditor<entt::registry::entity_type> editor;
    editor.registerComponent<TransformComponent>("Transform Component");
    editor.registerComponent<LightComponent>("Light Component", [] (auto, auto) {});
    editor.registerComponent<LightComponent>("Sprite Component", <Your widget function>);
    editor.registerComponent<LightComponent>(<ComponentInfoObject>);
    editor.render(registry, <entity>);
  • You can also specialize these function templates instead of providing a custom callback (a custom callback also works).
template <class Component>
inline void ComponentEditorWidget(entt::registry& registry, entt::registry::entity_type entity);

template <class Component>
inline void ComponentAddAction(entt::registry& registry, entt::registry::entity_type entity);

template <class Component>
inline void ComponentRemoveAction(entt::registry& registry, entt::registry::entity_type entity);
  • The internal architecture has changed so we are no longer storing different std::map<> and std::set<> but instead a single std::map<>

  • Some protected members + a virtual render() method ensures that a user can subclass this and override the render() method to completely change how everything is rendered.

  • Some things are done at compile time (not a big deal of stuff is done at compile time though since we are using runtime_views)

The code is of almost equivalent size. There are still things I do not like about the current implementation, but we can hopefully solve them.

  • The idea of default functionality is not new, but your approach is a very nice one.
  • Why did you decide to change the template parameter from registry to entity?
  • The idea of the struct packing all the info is nice, since you no longer need access to the editor for registration.

But please make a PR, and put yourself in the copyright notice ๐Ÿ˜„

I decided to use the EntityType as the template parameter since most of the functionality in the class is dependent on a registry that has an interface similar to entt::basic_registry<>. While we can theoretically make this work for any registry, stuff like

inline bool entity_has_component(Registry& ecs, typename Registry::entity_type& e, component_type ct) {
        component_type type[] = { ct };
	auto rv = ecs.runtime_view(std::cbegin(type), std::cend(type));
        return rv.contains(e);
}

is highly dependent on ECS from EnTT. So it is unlikely to be used in any other context.
Additionally, this delegates the task of checking if a type works to entt.

It is not a big deal though. We can just change it from

template <class EntityType>
class EntityEditor {
public:
    using Registry = entt::basic_registry<EntityType>;
}

to

template <class Registry>
class EntityEditor {
public:
}

Ultimately it boils down to if we like EntityEditor<entt::registry> or EntityEditor<entt::registry::entity_type> better, I guess.

I will send a PR when I get some time to write the demo code and the documentation for this. Hopefully, I will be able to do this within a week.

Also, do you think it is a good idea to allow users to select the entity as well?
If so, we will need to change
void renderImGui(Registry& ecs, typename Registry::entity_type& e)
to something else I guess.

I do not know if this is a good idea though.

Also, do you think it is a good idea to allow users to select the entity as well?

I currently do this with a "entity widget" which is a imgui drop-zone, for drag'n'drop.
And have a dedicated entity list-by-component window.

Here its in action, in a rather humorous way...

Any progress on this?
EnTT 3.3.0 has now been released and i'd like to get this done soon.
If there has not been done anything, i will create the PR and start working on the refactor.

as a sidenote, the new visit method seems to replace the internal helper, which gets the componets IDs.

Sorry, I completely forgot about this since I was busy with college stuff. I will send you a pull request later this day.

Also, I took a look at the registry::visit method you mentioned.
We have two options:

  • Use registry.visit(entity, [](const auto componentTypeID){}and then check if the componentTypeID is registered.
registry.visit(entity, [&componentTypeInfos](const auto componentTypeID){
        if (componentTypeInfos.find(componentTypeID != componentTypeInfos.end()) {
            ...
        }
});
  • Iterate over the registered components, and check if the entity contains the component ( the current method).
for (auto& [component_type_id, ci] : component_infos) {
        if (entity_has_component(registry, e, component_type_id)) {
                ...
        }
}

I do not know which method is better. But, assuming the entity has hundreds of components and only a few are registered, the first method will visit all components, while the second will visit only the registered components.

This is not of concern; it will take little resource to iterate through a few more components. What are your thoughts about this?

Edit: So, I checked entt's source and it turns out both entt::basic_registry::visit and runtime_view::contains use the same sparse_set::has. So, I assume our current method has marginally better performance. Although, it's still inconclusive. Ultimately, it depends on how you want to write this library. Either method is good enough for the task.

college stuff

same ๐Ÿ˜‰

So, I assume our current method has marginally better performance. Although, it's still inconclusive. Ultimately, it depends on how you want to write this library. Either method is good enough for the task.

Yes, took a look too. So we either move more stuff into the visit-lambda or we leave it as is, which is less work and therefore better ๐Ÿ˜ .