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

On Stream Code Review #2

Open
braddotcoffee opened this issue Jan 6, 2024 · 0 comments
Open

On Stream Code Review #2

braddotcoffee opened this issue Jan 6, 2024 · 0 comments

Comments

@braddotcoffee
Copy link

Magic Numbers

  • There's nothing you can do in this context to avoid magic numbers - they're things that are specific to the game implementation that you are overwriting
  • What we can do to make it easier to wrap our heads around is provide convenient names for these constants inside our code
  • You do this in a few assembly blocks and it makes the assembly block significantly more readable - it would be nice to see you take a quick pass and try to do this for all of the magic numbers

Iterative Configuration

  • A lot of your configuration options need to happen to >3 individual "components" back to back to back
  • This results in a lot of duplicated "copy/paste" code that is just "configure X, configure Y, configure Z, configure..."
  • It would be nice to see you define a struct for these configuration options that we then iterate through a list of to apply
  • Why struct?
    • It's nice for these things to have names associated with them that can provide a sense of self documentation
  • Why does it matter that we iterate through?
    • You can always move struct definitions out into a separate file if it gets really long and ugly
    • You wouldn't want to arbitrarily split up your configuration into multiple files for no reason if they are logically all the same configuration type
    • Because of this, iterating through struct definitions makes it more extensible as you want to add more of this type of configuration
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

1 participant