Skip to content

Conversation

@nAmKcAz
Copy link
Contributor

@nAmKcAz nAmKcAz commented Dec 17, 2024

  • Creates a Playback Status Manager Behaviour that holds/manages the current status of segment/sequence playback
  • Moves all on screen display and logging from the playback controller into this new Behaviour
  • Updates the exploration logic to be far less log spammy and flicker the screen less

Find the pull request instructions here

Every reviewer and the owner of the PR should consider these points in their request (feel free to copy this checklist so you can fill it out yourself in the overall PR comment)

  • The code is extensible and backward compatible
  • New public interfaces are extensible and open to backward compatibility in the future
  • If preparing to remove a field in the future (i.e. this PR removes an argument), the argument stays but is no longer functional, and attaches a deprecation warning. A linear task is also created to track this deletion task.
  • Non-critical or potentially modifiable arguments are optional
  • Breaking changes and the approach to handling them have been verified with the team (in the Linear task, design doc, or PR itself)
  • The code is easy to read
  • Unit tests are added for expected and edge cases
  • Integration tests are added for expected and edge cases
  • Functions and classes are documented
  • Migrations for both up and down operations are completed
  • A documentation PR is created and being reviewed for anything in this PR that requires knowledge to use
  • Implications on other dependent code (i.e. sample games and sample bots) is considered, mentioned, and properly handled
  • Style changes and other non-blocking changes are marked as non-blocking from reviewers

@nAmKcAz nAmKcAz marked this pull request as ready for review December 17, 2024 20:42
@nAmKcAz nAmKcAz requested a review from vontell as a code owner December 17, 2024 20:42
@nAmKcAz
Copy link
Contributor Author

nAmKcAz commented Dec 17, 2024

@abeizer This one is really targeted at your work, but it also helped me cleanup some of the mess in the playback controller.

There is now a new Behaviour BotSegmentPlaybackStatusManager that manages the status of playback to make it easy to see where we are and what the current error states are.

Copy link
Contributor

@abeizer abeizer left a comment

Choose a reason for hiding this comment

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

It'll be great to have all this info easily available for the UI. Maybe after these initial layouts for CV segments we can rework the "current play status" in the dashboard to look more like Addison's past mockups

Copy link
Collaborator

@vontell vontell left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for putting this together to make Abby's work easier. I also think encapsulating like this could be helpful in making more debug features in the future

@nAmKcAz nAmKcAz merged commit 6746439 into main Dec 19, 2024
2 checks passed
@nAmKcAz nAmKcAz deleted the zack/reg-2213-2 branch December 19, 2024 13:39
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

Successfully merging this pull request may close these issues.

4 participants