Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom Default impls for various components #32

Closed
rparrett opened this issue Mar 12, 2024 · 4 comments
Closed

Add custom Default impls for various components #32

rparrett opened this issue Mar 12, 2024 · 4 comments

Comments

@rparrett
Copy link

(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.

@rparrett rparrett changed the title Add custom Default impls for various types Add custom Default impls for various components Mar 12, 2024
@komadori
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants