komadori / bevy_mod_outline

Apache License 2.0
119 stars 10 forks source link

Add custom `Default` impls for various components #32

Closed rparrett closed 6 months ago

rparrett commented 6 months ago

(Thanks for the plugin, it seems to work really well)

While checking out the plugin, I naively added an outline to an entity with:

commands.entity(entity).insert(OutlineBundle::default());

And had a few minutes of headscratching until I realized that OutlineVolume has a derived Default impl of:

OutlineVolume {
    width: 0.,
    visible: false,
    colour: Color::default() // white, from Bevy
}

It would be nice if the default OutlineBundle resulted in a visible outline with non-zero width.

komadori commented 6 months ago

While I can appreciate this may be counter-intuitive when getting started, the expectation is that the outline components are added to all entities that might need to be outlined and then the properties are set as needed. Mutating a component is cheaper and more ergonomic than inserting and removing them. For this reason, I prefer the default state to be off.

Maybe there's a case for a helper function like OutlineBundle::new(colour: &Colour, width: f32) if you think that would improve discoverability?

rparrett commented 6 months ago

That is a surprising optimization to me. In my case, there are many outlinable things, and only one thing will be outlined at any given time. The outline would change at most once per frame. So it seems like I would be paying greatly in memory usage and perhaps time iterating over entities with outline components for a small speedup in something that happens once per frame.

Still, if this is an intentional design decision then there's probably nothing to be done here. Is this philosophy explained in some documentation somewhere that I might have missed?

komadori commented 6 months ago

When I first developed this plugin I also added and removed the outline components as needed, but I found this was not very ergonomic for me and added the enabled field to toggle them on and off. I would say that ethos of the Bevy community as I perceive it seems wary of "archetype fragmentation" and this is consistent with avoiding that, but I can't bring any specific example to mind right now. In any case, you are more than welcome to add and remove the outline components if you feel that best suits your use case. However, as with #34, you may find the experience slightly less polished since that's not how I use it.

rparrett commented 6 months ago

Thanks for the explanation. The current defaults make sense in that context.