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

Address Alice's notes #203

Closed
15 tasks done
benfrankel opened this issue Jul 30, 2024 · 6 comments
Closed
15 tasks done

Address Alice's notes #203

benfrankel opened this issue Jul 30, 2024 · 6 comments
Labels
enhancement New feature or request
Milestone

Comments

@benfrankel
Copy link
Collaborator

benfrankel commented Jul 30, 2024

  • I don't think InteractionQuery is worth the cognitive complexity cost
  • InteractionPalette's naming / docs aren't clear enough (I like the pattern, but it needs to be taught better)
  • handle_title_action should be done using observers, rather than a TitleAction component
  • exit_playing and friends should be renamed to reflect a smaller scope: state transition schedules should contain multiple systems by convention
  • audio should be data-driven, rather than using enums, and needs a generalized abstraction for "random within this group"
  • I don't like the layout of the game module: IMO trying to further separate out gameplay logic from things like sound effects is important
  • WrapWithinWindow is probably clearer as ScreenWrap
  • MovementController needs docs
  • you should be able to make AppSet more robust to refactors by using the PartialOrd trait
  • "Alice - Foo" in credits screen will be clearer with a made up job title instead of Foo
  • "We hold no patent rights to anything presented in this repository." This should be clearer or probably just removed.
  • screen is kind of a weird name for that module 🤔 Totally just a nit though, and I don't really have better suggestions
  • I'm not convinced that spawning things like the player should be an observer; that seems like a pointless layer of indirection over just using a custom command
  • man we really need an actual 2D animation API <:pensive_cowboy:791012102655574086>
  • love the Known Issues page: that's a really nice compromise
@benfrankel benfrankel added the enhancement New feature or request label Jul 30, 2024
@benfrankel benfrankel added this to the Bevy Jam 6 milestone Jul 30, 2024
@benfrankel
Copy link
Collaborator Author

benfrankel commented Jul 30, 2024

A minimal rename option suggested by @alice-i-cecile: screen/ -> screens/

@janhohenheim
Copy link
Member

The animation API is resolved too, not much we can do.

@MiniaczQ
Copy link
Collaborator

@benfrankel
Copy link
Collaborator Author

See #223 for the point about spawning.

@janhohenheim
Copy link
Member

See bevyengine/bevy#14554 for how to do sounds (thanks Alice!)

@janhohenheim
Copy link
Member

All done now :D

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

No branches or pull requests

3 participants