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

Move intake scoring states to separate classes #56

Merged

Conversation

IanTapply22
Copy link
Member

Description

Moved the scoring states for the intake to separate classes to improve organization and to remove the large conditional block

Fixes #51

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

N/A

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation, if any
  • My changes generate no new warnings
  • I have performed tests that prove my fix is effective or that my feature works, if necessary
  • New and existing unit tests pass locally with my changes

@IanTapply22 IanTapply22 added the enhancement New feature or request label Dec 19, 2023
@IanTapply22 IanTapply22 self-assigned this Dec 19, 2023
Copy link

Thank you for making a pull request! Please make sure to follow the pull request guidelines for this PR. This applies to your commit messages, pull request code, and more!

Copy link
Member

@colemacphail colemacphail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks fine to me, just very confused about why we have 2 different approaches to scoring states

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for my information, why is low different from mid and high?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The speed of the intake seems to be higher when scoring low🤷. I just transferred over what was in the switch statement over to classes

* @param outtakeSpeed The speed that should be used to outtake the gamepiece
* @param stateName The name of the scoring state that should be used for logging
*/
protected ScoringState(double outtakeSpeed, String stateName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This interface also seems like it could be simplified if we made all the strategies handle things the same way

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, that was the approach I was trying to make in the beginning. But I found that it would just overcomplicate it if a state had different speeds for cones and cubes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any suggestions on how I could maybe improve it so every state could be handled the same?

@kaleb-dodd kaleb-dodd merged commit 8ea1b85 into 2024-beta Jan 6, 2024
2 checks passed
@kaleb-dodd kaleb-dodd deleted the 51/move-intake-scoring-states-to-separate-classes branch January 6, 2024 01:16
@IanTapply22 IanTapply22 linked an issue Jan 6, 2024 that may be closed by this pull request
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

Successfully merging this pull request may close these issues.

Rename "IntakeGamepieces" enum to "IntakeGamepiece"
3 participants