Fix all the broken things [Issue #54] #56
Conversation
Man! This was so spaghetti'd I'm now Italian... ~ Removed a break from the switch statement in the constructor of CommandExecutor which meant that float values were pushed to the argument list twice (once as a float and then again as a command). ~ In Parser.parse() moved the call to nextToken() out of the else condition so that it is always executed. The intended functionality is to call parseExpression() when the argument type is int or float. After this change nextToken() was always executed regardless, then parseExpression was executed as well if the expression was an int or float, causing arguments to get skipped resulting in attempts to invoke numbers as if they were commands. ~ Renamed COMMAND_TYPES to ARGUMENT_TYPES but not everywhere causing ReferenceErrors ~ #30 Removed calls to parseInt() and parseFloat() in the CommandExecutor constructor because an argument may be an Expression object. #52 Added them back causing Expression objects to become NaN ~ Missing brace at end of parser.js ~ #50 also removed the calls to drawingBounds.reset() and drawingBounds.move(turtle.x, turtle.y) which obviously broke the autofit-drawing-to-canvas feature ~ The autofit-drawing-to-canvas feature was also broken because someone removed the call to the function ¯\_(ツ)_/¯
TheTastefulToastie
changed the title
|
This is heroic! And I created merge conflicts by merging #55 first. . mind taking one last look? |
|
done 🌈 |
|
actually hang on, some of the bugs are back after that merge |
|
@shiffman working now 🎺 |
In a previous commit changed 'str' to 'arg' when arg could be a mix of int-as-string/float-as-string/Expression #55 Changed this and now it will always be string so 'str' is more appropriate.
|
@TheTastefulToastie 😍 great job |
Looks like selecting the 'Default' item in the dropdown used to clear the textarea and the drawing until [this](https://github.com/CodingTrain/Logo/pull/51/files#diff-9f2094443505273ff51ea1d1702e4367L101) commit removed it. So now the drawing remains unchanged when selecting the default item. It then resets the camera position and calls goTurtle() to redraw which causes the view to jump off-center. Instead it should put the default Logo in the textarea and call scaleToFitBoundingBox() to redraw _and_ center the drawing. This commit makes this change.
|
@AmrBarghouthi That meant the drawing wouldn't change when selecting the default item from the dropdown. It would then reset the camera position and call goTurtle() to redraw which caused the view to jump off-center. Instead it should put the default Logo in the textarea and call scaleToFitBoundingBox() to redraw and center the drawing. 👍 |
|
Yay! Going to go ahead and merge and we can fix anything else after. |
Man! This was so spaghetti'd I'm now Italian...
These were the issues:
Someone removed a break from the switch statement in the constructor of CommandExecutor which meant that float values were pushed to the argument list twice (once as a float and then again as a command).
In Parser.parse() someone moved the call to nextToken() out of the else condition so that it is always executed.
The intended functionality is to call parseExpression() when the argument type is int or float.
After this change nextToken() was always executed regardless, then parseExpression was executed as well if the expression was an int or float, causing arguments to get skipped resulting in attempts to invoke numbers as if they were commands.
COMMAND_TYPES was renamed to ARGUMENT_TYPES but not everywhere causing ReferenceErrors
Added Expressions #30 Removed calls to parseInt() and parseFloat() in the CommandExecutor constructor because an argument may be an Expression object. Testing args OOP way #52 Added them back causing Expression objects to become NaN
Missing brace at end of parser.js
Testing args OOP way #52 also removed the calls to drawingBounds.reset() and drawingBounds.move(turtle.x, turtle.y) which obviously broke the autofit-drawing-to-canvas feature
The autofit-drawing-to-canvas feature was also broken because someone removed the call to the scaleToFitBoundingBox function ¯\_(ツ)_/¯
The commandList.js script tag got removed