add a new functions #48
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I added two functions such as a round turn left and right named runRight, runLeft.
kouosi
reviewed
Dec 17, 2025
Contributor
kouosi
left a comment
kouosi
left a comment
There was a problem hiding this comment.
you can achieve runRight and runLeft with turnRight and turnLeft i don't see what this pr does more.
| bool success = moveForward(numHalfSteps); | ||
| return success ? "" : CRASH; | ||
| } else if (function == "turnRight" || function == "turnRight90") { | ||
| } else if (function == "runRight" ) { |
Contributor
There was a problem hiding this comment.
Suggested change
| } else if (function == "runRight" ) { | |
| } else if (function == "runRight") { |
| } else if (function == "runRight" ) { | ||
| turn(Movement::RUN_RIGHT); | ||
| return ""; | ||
| } else if (function == "runLeft" ) { |
Contributor
There was a problem hiding this comment.
Suggested change
| } else if (function == "runLeft" ) { | |
| } else if (function == "runLeft") { |
| } else if (function == "runLeft" ) { | ||
| turn(Movement::RUN_LEFT); | ||
| return ""; | ||
| } else if (function == "turnBack" ) { |
Contributor
There was a problem hiding this comment.
Suggested change
| } else if (function == "turnBack" ) { | |
| } else if (function == "turnBack") { |
| turn(Movement::TURN_RIGHT_90); | ||
| return ""; | ||
| } else if (function == "turnLeft" || function == "turnLeft90") { | ||
| } else if (function == "turnLeft" || function == "turnLeft90" ) { |
Contributor
There was a problem hiding this comment.
Suggested change
| } else if (function == "turnLeft" || function == "turnLeft90" ) { | |
| } else if (function == "turnLeft" || function == "turnLeft90") { |
| } else if (function == "turnBack" ) { | ||
| turn(Movement::TURN_RIGHT_180); | ||
| return ""; | ||
| } else if (function == "turnRight" || function == "turnRight90" ) { |
Contributor
There was a problem hiding this comment.
Suggested change
| } else if (function == "turnRight" || function == "turnRight90" ) { | |
| } else if (function == "turnRight" || function == "turnRight90") { |
| double m_movementProgress; | ||
| double m_movementStepSize; | ||
| QSlider *m_speedSlider; | ||
| int m_sliderValue; |
Contributor
There was a problem hiding this comment.
Suggested change
| int m_sliderValue; | |
| int m_sliderValue; |
| } | ||
| if (isWall(semiPos, semiDir)) { | ||
| } | ||
| //m_runOutput->appendPlainText( QVariant(halfStepsAhead).toString() + "-(" + QVariant(semiPos.x).toString() + " , " + QVariant(semiPos.y).toString() + "):" +QVariant( (int) semiDir ).toString() ); |
Author
|
Hi!
I made a curb turn which round turning.
So it is different with turnRight or turnLeft.
Thanks for being interested!
Have a lovely Christmas time!!
…On Thu, 18 Dec 2025 at 1:00 am, kouosi ***@***.***> wrote:
***@***.**** commented on this pull request.
you can achieve runRight and runLeft with turnRight and turnLeft i don't
see what this pr does more.
------------------------------
In src/Window.cpp
<#48 (comment)>:
> @@ -1111,10 +1122,19 @@ QString Window::executeCommand(QString command) {
}
bool success = moveForward(numHalfSteps);
return success ? "" : CRASH;
- } else if (function == "turnRight" || function == "turnRight90") {
+ } else if (function == "runRight" ) {
⬇️ Suggested change
- } else if (function == "runRight" ) {
+ } else if (function == "runRight") {
------------------------------
In src/Window.cpp
<#48 (comment)>:
> @@ -1111,10 +1122,19 @@ QString Window::executeCommand(QString command) {
}
bool success = moveForward(numHalfSteps);
return success ? "" : CRASH;
- } else if (function == "turnRight" || function == "turnRight90") {
+ } else if (function == "runRight" ) {
+ turn(Movement::RUN_RIGHT);
+ return "";
+ } else if (function == "runLeft" ) {
⬇️ Suggested change
- } else if (function == "runLeft" ) {
+ } else if (function == "runLeft") {
------------------------------
In src/Window.cpp
<#48 (comment)>:
> @@ -1111,10 +1122,19 @@ QString Window::executeCommand(QString command) {
}
bool success = moveForward(numHalfSteps);
return success ? "" : CRASH;
- } else if (function == "turnRight" || function == "turnRight90") {
+ } else if (function == "runRight" ) {
+ turn(Movement::RUN_RIGHT);
+ return "";
+ } else if (function == "runLeft" ) {
+ turn(Movement::RUN_LEFT);
+ return "";
+ } else if (function == "turnBack" ) {
⬇️ Suggested change
- } else if (function == "turnBack" ) {
+ } else if (function == "turnBack") {
------------------------------
In src/Window.cpp
<#48 (comment)>:
> turn(Movement::TURN_RIGHT_90);
return "";
- } else if (function == "turnLeft" || function == "turnLeft90") {
+ } else if (function == "turnLeft" || function == "turnLeft90" ) {
⬇️ Suggested change
- } else if (function == "turnLeft" || function == "turnLeft90" ) {
+ } else if (function == "turnLeft" || function == "turnLeft90") {
------------------------------
In src/Window.cpp
<#48 (comment)>:
> @@ -1111,10 +1122,19 @@ QString Window::executeCommand(QString command) {
}
bool success = moveForward(numHalfSteps);
return success ? "" : CRASH;
- } else if (function == "turnRight" || function == "turnRight90") {
+ } else if (function == "runRight" ) {
+ turn(Movement::RUN_RIGHT);
+ return "";
+ } else if (function == "runLeft" ) {
+ turn(Movement::RUN_LEFT);
+ return "";
+ } else if (function == "turnBack" ) {
+ turn(Movement::TURN_RIGHT_180);
+ return "";
+ } else if (function == "turnRight" || function == "turnRight90" ) {
⬇️ Suggested change
- } else if (function == "turnRight" || function == "turnRight90" ) {
+ } else if (function == "turnRight" || function == "turnRight90") {
------------------------------
In src/Window.h
<#48 (comment)>:
> @@ -177,6 +180,7 @@ class Window : public QMainWindow {
double m_movementProgress;
double m_movementStepSize;
QSlider *m_speedSlider;
+ int m_sliderValue;
⬇️ Suggested change
- int m_sliderValue;
+ int m_sliderValue;
------------------------------
In src/Window.cpp
<#48 (comment)>:
> @@ -1712,9 +1818,10 @@ bool Window::isWall(SemiPosition semiPos, SemiDirection semiDir,
default:
ASSERT_NEVER_RUNS();
}
- if (isWall(semiPos, semiDir)) {
+ }
+ //m_runOutput->appendPlainText( QVariant(halfStepsAhead).toString() + "-(" + QVariant(semiPos.x).toString() + " , " + QVariant(semiPos.y).toString() + "):" +QVariant( (int) semiDir ).toString() );
Remove this comment.
—
Reply to this email directly, view it on GitHub
<#48 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANBLSKQQNHL7PYIVI34OC6D4CFOYVAVCNFSM6AAAAACPAKBLGSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTKOBXHA3DANJVGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
mackorone
requested changes
Dec 29, 2025
Owner
mackorone
left a comment
mackorone
left a comment
There was a problem hiding this comment.
Thanks @GoGoCom for submitting this PR and thanks @kouosi for doing a preliminary review! And sorry for the delay, I've been busy due to the holidays.
I left some inline comments, please address and run clang-format --style=google before re-submitting.
My main feedback is to exclude any changes that aren't required for implementing curve turns. Feel free to submit additional PRs for the other changes.
Comment on lines
+120
to
+121
| void runRight(); | ||
| void runLeft(); |
Owner
There was a problem hiding this comment.
Let's rename these to curveTurnRight and curveTurnLeft for explicitness
| * **Action:** Turn the robot forty-five degrees to the left | ||
| * **Response:** `ack` once the movement completes | ||
|
|
||
| #### `runRight` |
Owner
There was a problem hiding this comment.
Please update this name (see above)
| * **Action:** Round turn the robot ninty degrees to the right | ||
| * **Response:** `ack` once the movement completes | ||
|
|
||
| #### `runLeft` |
Owner
There was a problem hiding this comment.
Please update this name (see above)
Comment on lines
+296
to
+299
| QStringList SplitStr = path.split("/"); | ||
| QString TitleStr = SplitStr.takeLast(); | ||
| this->setWindowTitle("mms : "+TitleStr); | ||
|
|
Owner
There was a problem hiding this comment.
This looks like an unrelated change. Can you split it out into its own pull request? Same goes for similar changes below.
Comment on lines
+730
to
+732
|
|
||
| // for mouse speed control | ||
| m_sliderValue = m_speedSlider->value(); |
Owner
There was a problem hiding this comment.
We shouldn't need to redundantly store the slider value here
Comment on lines
+1132
to
+1134
| turn(Movement::TURN_RIGHT_180); | ||
| return ""; | ||
| } else if (function == "turnRight" || function == "turnRight90" ) { |
Owner
There was a problem hiding this comment.
Let's keep this PR strictly scoped to curve turns. Please split this into its own PR.
| Coordinate currentTranslation = startingTranslation * (1.0 - fraction) + destinationTranslation * fraction; | ||
| Angle currentRotation = startingRotation * (1.0 - fraction) + destinationRotation * fraction; | ||
|
|
||
| // Calculate the current translation and rotation for a curv turn |
Owner
There was a problem hiding this comment.
This function is getting a bit too big. Can you create a separate helper function for the curve turn logic?
Comment on lines
+1530
to
+1548
| // increase the speed by the distance that will be travelled | ||
| switch(numHalfSteps) { | ||
| case 1: | ||
| case 2: | ||
| m_speedSlider->setValue(m_sliderValue); | ||
| break; | ||
| case 3 : | ||
| case 4 : | ||
| m_speedSlider->setValue(m_sliderValue*1.25); | ||
| break; | ||
| case 5: | ||
| case 6: | ||
| m_speedSlider->setValue(m_sliderValue*1.5); | ||
| break; | ||
| default : | ||
| m_speedSlider->setValue(m_sliderValue*2.0); | ||
| break; | ||
| } | ||
| // TODO: upforgrabs |
Owner
There was a problem hiding this comment.
This is an interesting change but probably belongs as its own PR, because I'm not convinced programmatically setting the speed slider is wise. (There might be other ways to accomplish the goal of making the mouse acceleration more realistic and apparent.)
Author
|
Hi Mack!
How are you!
I’m glad to hear from you.
I'm trying to explain the meaning of the work I did. The runRight and
runLeft want to show that the mouse robot is trying to turn left and right
while continuing to drive at the fork in the road without repeating the go
and stopping. It's because the mouse robot can go fast while maintaining
your speed. That's why I named it run, but it doesn't matter if you change
it. The program looks a little complicated because I have to draw a curve,
but it's a C++ library that I'm not familiar with, so I'd like you to make
it more briefly.
The reason why the file name is displayed on the title is to let you know
the source of the current maze, and it is easy to rest if you express the
year and place in the maze file name.
The reason for adjusting the sliding value is to express the way two or
more blocks run fast when the mouse goes, because the speed is adjusted
according to the situation in the actual mouse game.
I'm currently making a robot and I'm doing it well thanks to the
simulation. I think it's the best simulation program. I'm going to let you
know through YouTube.
Have a good rest of 2025 and I wish you a lot of good things in the new
year.
Kind regards
David Lee
…On Mon, 29 Dec 2025 at 5:44 pm, Mack ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Thanks @GoGoCom <https://github.com/GoGoCom> for submitting this PR and
thanks @kouosi <https://github.com/kouosi> for doing a preliminary
review! And sorry for the delay, I've been busy due to the holidays.
I left some inline comments, please address and run clang-format
--style=google before re-submitting.
My main feedback is to exclude any changes that aren't required for
implementing curve turns. Feel free to submit additional PRs for the other
changes.
------------------------------
In README.md
<#48 (comment)>:
> +void runRight();
+void runLeft();
Let's rename these to curveTurnRight and curveTurnLeft for explicitness
------------------------------
In README.md
<#48 (comment)>:
> @@ -205,6 +207,16 @@ int/float getStat(string stat);
* **Action:** Turn the robot forty-five degrees to the left
* **Response:** `ack` once the movement completes
+#### `runRight`
Please update this name (see above)
------------------------------
In README.md
<#48 (comment)>:
> @@ -205,6 +207,16 @@ int/float getStat(string stat);
* **Action:** Turn the robot forty-five degrees to the left
* **Response:** `ack` once the movement completes
+#### `runRight`
+* **Args:** None
+* **Action:** Round turn the robot ninty degrees to the right
+* **Response:** `ack` once the movement completes
+
+#### `runLeft`
Please update this name (see above)
------------------------------
In src/Window.cpp
<#48 (comment)>:
> + QStringList SplitStr = path.split("/");
+ QString TitleStr = SplitStr.takeLast();
+ this->setWindowTitle("mms : "+TitleStr);
+
This looks like an unrelated change. Can you split it out into its own
pull request? Same goes for similar changes below.
------------------------------
In src/Window.cpp
<#48 (comment)>:
> +
+ // for mouse speed control
+ m_sliderValue = m_speedSlider->value();
We shouldn't need to redundantly store the slider value here
------------------------------
In src/Window.cpp
<#48 (comment)>:
> + turn(Movement::TURN_RIGHT_180);
+ return "";
+ } else if (function == "turnRight" || function == "turnRight90" ) {
Let's keep this PR strictly scoped to curve turns. Please split this into
its own PR.
------------------------------
In src/Window.cpp
<#48 (comment)>:
> Coordinate destinationTranslation = getCoordinate(destinationLocation);
+ Angle startingRotation = DIRECTION_TO_ANGLE().value(m_startingDirection);
+
+ Coordinate currentTranslation = startingTranslation * (1.0 - fraction) + destinationTranslation * fraction;
+ Angle currentRotation = startingRotation * (1.0 - fraction) + destinationRotation * fraction;
+
+ // Calculate the current translation and rotation for a curv turn
This function is getting a bit too big. Can you create a separate helper
function for the curve turn logic?
------------------------------
In src/Window.cpp
<#48 (comment)>:
> + // increase the speed by the distance that will be travelled
+ switch(numHalfSteps) {
+ case 1:
+ case 2:
+ m_speedSlider->setValue(m_sliderValue);
+ break;
+ case 3 :
+ case 4 :
+ m_speedSlider->setValue(m_sliderValue*1.25);
+ break;
+ case 5:
+ case 6:
+ m_speedSlider->setValue(m_sliderValue*1.5);
+ break;
+ default :
+ m_speedSlider->setValue(m_sliderValue*2.0);
+ break;
+ }
+ // TODO: upforgrabs
This is an interesting change but probably belongs as its own PR, because
I'm not convinced programmatically setting the speed slider is wise. (There
might be other ways to accomplish the goal of making the mouse acceleration
more realistic and apparent.)
—
Reply to this email directly, view it on GitHub
<#48 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANBLSKUI7MPXJSHCTEB6WIT4EDETXAVCNFSM6AAAAACPAKBLGSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMMJVGE2TIMBQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Owner
|
@GoGoCom thanks for the kind words! I won't have time to clean up and incorporate this PR for at least a few weeks. If you'd like to get it merged before then, do your best to address my inline comments and I can review your changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi! Machorone!
How are you!
Recently I added two functions such as a round turn left and right named runRight, runLeft.
It will be good to speed up to run.
Thanks!