-
Notifications
You must be signed in to change notification settings - Fork 0
Add boiler point for state machine #31
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
base: main
Are you sure you want to change the base?
Conversation
3e30222 to
536a194
Compare
PhazonicRidley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall good! One small design question, is there a reason you are using raw pointers for your states? Do they need to stay alive after exiting? If so, is there a reason you are using raw pointers over smart pointers
| @@ -0,0 +1,52 @@ | |||
| #pragma once | |||
| #include <print> | |||
| #include "./state.hpp" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: include/ should be in the include path (this functionally doesn't matter but imo its cleaner without relative paths, also if we ever change our file structure then it doesn't need to be updated)
| * SAFE will be stored here and shared between the states. | ||
| * | ||
| */ | ||
| class StateContext { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add doxygen style documentation (our good friend chaudexityilot is great at this)
|
|
||
| class State; | ||
|
|
||
| class StateMachine { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs
| void StateMachine::run_state() { | ||
| current_state->enter(context); | ||
| current_state->handle(context); | ||
| transition_state (current_state->exit(context)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prev state is dangling and is never freed
|
|
||
| class StateMachine { | ||
| public: | ||
| StateMachine(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: No need to declare a default ctor or dtor
Fixes #33