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

Require deriving explicitly ActionBuilder or ScorerBuilder #62

Closed
cBournhonesque opened this issue Jan 5, 2023 · 2 comments
Closed
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@cBournhonesque
Copy link
Contributor

cBournhonesque commented Jan 5, 2023

I think it would be helpful to require users to derive explicitly ActionBuilder and ScorerBuilder (and to change all the impl ActionBuilder into ActionBuilder, so that users need to provide explicitly a ActionBuilder).

  1. Too much flexibility is kind of confusing, and not that helpful. When i first read the docs I was confused as to what a ScorerBuilder was, and how it was related to me just creating a struct with Component and Clone.
    I think it would be much clearer for users to have their scorers-builders/action-builders with explicit #[derive(ScorerBuilder)]

This is the exact same argument as: bevyengine/bevy#1843
In theory, it's more flexible to accept any std::fmt::Debug + Sync + Send as a ScorerBuilder (by allowing pre-existing types to be ScorerBuilders, but in practice it just makes it less clear, and i believe 99% of users won't use a pre-existing type as a scorer builder)

  1. Easy labels. I think right now a user needs to explicitly implement ScorerBuilder to have a Label.
    By forcing all ScorerBuilders to have the explicit type, users could easily add the label via
 `#[derive(ScorerBuilder, label="MyScorer")]`
 pub struct MyScorer;
@zkat zkat added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Jan 5, 2023
@zkat
Copy link
Owner

zkat commented Jan 5, 2023

This sounds good to me.

@zkat
Copy link
Owner

zkat commented Jan 30, 2023

This is done.

@zkat zkat closed this as completed Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants