feature/ATC-114 #179

n8rzz · 2016-12-06T01:38:41Z

completes #114 and #183

Custom command parser. This represents a complete re-write of the command parser.

…. Adds CommandParser inside console.log to InputController

* Modifies how CommandModel objects are built * Updates tests

…ds (excluding transmit) * Adds InputController.processCOmmand() method and simplifies branching logic by using switch

* Adds new getter to CommandModel * Adds additional test cases

…sers * COMMAND_DEFINITION objects the define a validator and a parser to validated and parse each command * Simplifies some of the run functions within AircraftInstanceModel

* Adds ERROR_MESSAGE enum for argumentValidators error messaging * Adds convertToThousands() and convertStringToNumber() to unitConverters

* Abstracts errors to ERROR_MESSAGE * Adds more doc blocks

…p to 100%

…s failing test for holdParser.

…r files

…l. Adds encapsulated boolean method for Parser

n8rzz · 2016-12-09T04:33:37Z

src/assets/scripts/commandParser/argumentParsers.js

+export const altitudeParser = (args) => {
+    const altitude = convertToThousands(args[0]);
+    // the validator will have already caught an invalid value here. if one exists, it is assumed to be valid and
+    // thus we return true. other wise its false


spelling other wise -> otherwise

n8rzz · 2016-12-09T04:34:32Z

src/assets/scripts/commandParser/argumentValidators.js

+ * @type {Object}
+ * @final
+ */
+const ERROR_MESSAGE = {


move to different file

…TC-114 Conflicts: - src/assets/scripts/constants/globalConstants.js

…TC-114

n8rzz · 2016-12-10T00:29:38Z

src/assets/scripts/commandParser/CommandParser.js

+     * @method _buildSystemCommandModel
+     * @private
+     */
+    _buildSystemCommandModel(commandArgSegments) {


system commands should still be validated and parsed

n8rzz · 2016-12-10T00:34:01Z

src/assets/scripts/commandParser/argumentParsers.js

+ * @param args {array}
+ * @return {array<string, number, boolean>}
+ */
+export const headingParser = (args) => {


this should be able to handle two or three digit radials:

030 -> { heading: 30, incremental: false } 30 -> { heading: currentHeading + 30, incremental: true }

- Abstracts ERROR_MESSAGES to new file - Adds _parseUserCommand() method to InputController to encapsulate actual parsing/errors of the CommandParser - Adds validators and parsers to System commands

…forces a single argument for airport command

n8rzz · 2016-12-15T04:42:30Z

@erikquinn ready for your eyes again. feel free to use the review app, too ;-) Let me know if you find more stuff. I definitely don't want to rush this one out the door. Lets get this right the first time!

@Dabea feel free to add feedback or ask questions on this stuff. I'd like to get you in on some of these PRs just to get used to the process an see more of the codebase.

erikquinn

I dug in pretty deep with this one... found a lot of good unrelated issues actually (as your newsfeed likely indicates 😉)...

Things are definitely better, there are just a few things I called out that should be addressed. But I believe I was quite thorough, so it should pretty much be that what you see here is what you get; there should be no other issues lurking around beyond what is called out here or recorded in another issue. So hopefully there will be nothing broken when it is to be merged! 👍

erikquinn · 2016-12-15T05:51:12Z

src/assets/scripts/commandParser/commandMap.js

+    '\\u2B61': 'altitude',
+    '\\u2B63': 'altitude',
+    '\\u2BA2': 'heading',
+    '\\u2BA3': 'heading'


@n8rzz It theoretically wouldn't break my heart to have these out of commission for a short while, but the left/right functionality of the arrows is crucial; without it, they really don't provide nearly as much of a benefit.

I think we talked about this briefly before... is there any way to easily get it to recognize that one should pass the direction during the call of runHeading? And again, even if pressing the keys inserts t l instead of the unicode arrows, that would be an acceptable resolution.

Please add '\\u2B50': 'land' to this list to re-activate the asterisk shortcut for approach clearances.

Thanks fair. to be honest, I think inserting t l or t r on keypress will be the cleanest/easiest solution. I thought of one other way to do it, but it's gross and very brittle. If we programmatically add t l or t r, once the user presses enter it will get validated and parsed just like it would if the user typed it in. I think this is going to be the best solution here.

and I totally missed the land command, thanks for that.

heh, I didn't miss the land command. I just included it as: '*': 'land' in the commandMap file. I'll move that up to the unicodes.

erikquinn · 2016-12-15T06:15:00Z

src/assets/scripts/InputController.js

@@ -436,7 +440,14 @@ export default class InputController {
    onCommandInputKeydownHandler(e) {
        const currentCommandInputValue = this.$commandInput.val();

+        // TODO: this swtich can be simplified, there is a lot of repetition here
        switch (e.which) {


I know it's not actually part of this PR, but since you're here, could you take care of the following small change?

The following need to have spaces at the end of that which is appended to the currentCommandInputValue:

case KEY_CODES.MULTIPLY

case KEY_CODES.ADD

case KEY_CODES.EQUALS

case KEY_CODES.SUBTRACT

case KEY_CODES.DASH

case KEY_CODES.DIVIDE

I'll be in there anyway. this is a simple change.

erikquinn · 2016-12-15T07:02:48Z

src/assets/scripts/commandParser/CommandParser.js

+     * @method _validateCommandArguments
+     * @private
+     */
+    _validateCommandArguments() {


the hold command throws an error when I try to feed it only two arguments, as it explains it expected either one or three. The command of hold coinz 2min would ideally be considered sufficient. The user can provide the constraints they want, and if not specified, the default behaviors of [1min legs] and [right turns] will be assumed.

So really, the expected number of arguments should be zero, one, two, or three.

See this section of the Command Reference within the wiki for the intended functionality.

totally missed that part as well. I remember reading through it, I just didn't implement it. It'll add a little more logic to the validator and parser, but shouldn't be too bad.

…TC-114

- Adds asterisk unicode to unicode commands

erikquinn · 2016-12-16T02:47:51Z

src/assets/scripts/InputController.js

-                if (window.gameController.game.option.get('controlMethod') === 'arrows') {
-                    this.$commandInput.val(`${currentCommandInputValue} \u2BA2`);
+                if (this._isArrowControlMethod()) {
+                    this.$commandInput.val(`${currentCommandInputValue}t l `);


@n8rzz Not to be high maintenance, but could you add back the spaces before the command as well? The idea is to make them chainable, eg [right arrow] [3] [5] [0] [down arrow] [5] [0] [numpad -] [1] [7] [0], and get AAL 123 > 350 v 50 - 170, so you never have to press space.

not high maintenance at all, I'm glad you called that out. We were actually getting two spaces between callsign and first command, so I just fixed the glitch here. Wrong idea.

I just looked through it and found where the callsign is added to the command line and removed the space there. these commands will have the space on the left in my next commit.

Good eye! never be shy about these kinds of things!!

I just looked through it and found where the callsign is added to the command line and removed the space there.

As long as you can still click to select an aircraft and type [C] [A] [F] [enter] without needing to add a leading space, that'll do, but it sounds like you're making it so selecting the aircraft inserts AAL123 instead of AAL123[space] in the currentCommandInputValue.

we really need a way to make this consistent, and you're exactly right. I'm glad you're keeping an eye on things!

I'll get it right one of these times 😉

…that be received in any order - Adjusts spacing in InputController

n8rzz · 2016-12-16T04:57:08Z

@erikquinn ready for review. Check those spaces and let me know if you find anything else.

erikquinn · 2016-12-16T05:28:31Z

src/assets/scripts/InputController.js

                e.preventDefault();
                this.onCommandInputChangeHandler();

                break;

            case KEY_CODES.SUBTRACT:
-                this.$commandInput.val(`${currentCommandInputValue} -`);
+                this.$commandInput.val(`${currentCommandInputValue}- `);


this.$commandInput.val(`${currentCommandInputValue} - `);

erikquinn · 2016-12-16T05:31:19Z

src/assets/scripts/commandParser/CommandParser.js

+     * @method _validateCommandArguments
+     * @private
+     */
+    _validateCommandArguments() {


Just one thing not working with the hold command still: the command alone with no arguments. This would be a standard holding pattern at the aircraft's present position (unrelated to a fix). All other combinations work properly.

whoops 🤕

done

* Adds more documentation

n8rzz · 2016-12-16T14:37:20Z

@erikquinn ready for review

we're making this right the first time, so be brutal!

erikquinn · 2016-12-16T22:58:55Z

@n8rzz My "brutal" conclusion is that everything works exactly as it should! I'm practically jumping for joy about this one!!! 🎉

n8rzz · 2016-12-16T23:37:54Z

n8rzz added 5 commits December 1, 2016 15:35

feature/ATC-114 - Adds CommandParser and CommandModel and basic tests…

14916a1

…. Adds CommandParser inside console.log to InputController

feature/ATC-114 - Adds support for unicode character commands

9b5cd51

* Modifies how CommandModel objects are built * Updates tests

feature/ATC-114 - Begins switch to CommandParser for top level comman…

946c9da

…ds (excluding transmit) * Adds InputController.processCOmmand() method and simplifies branching logic by using switch

feature/ATC-114 - Continues CommandParser integration

ded0ca3

* Adds new getter to CommandModel * Adds additional test cases

feature/ATC-114 - Adds commandMap, argumentValidators and argumentPar…

fa4ac70

…sers * COMMAND_DEFINITION objects the define a validator and a parser to validated and parse each command * Simplifies some of the run functions within AircraftInstanceModel

n8rzz added the feature label Dec 6, 2016

n8rzz added this to the v3.2.0 milestone Dec 6, 2016

n8rzz self-assigned this Dec 6, 2016

n8rzz changed the title ~~feature/ATC-114~~ [WIP] feature/ATC-114 Dec 6, 2016

n8rzz added the WIP label Dec 6, 2016

n8rzz added 9 commits December 5, 2016 23:10

feature/ATC-114 Adds headingValidator and headingParser

58b2a3b

* Adds ERROR_MESSAGE enum for argumentValidators error messaging * Adds convertToThousands() and convertStringToNumber() to unitConverters

feature/ATC-114 - Adds WAYPOINT_NAV_MODE constants to runHeading()

f348e85

feature/ATC-114 - Finishes headingParser

6920d68

* Abstracts errors to ERROR_MESSAGE * Adds more doc blocks

feature/ATC-114 - Adds holdValidator and tests

31454d1

feature/ATC-114 - adds holdParser and brings parser/validator tests u…

d559a4d

…p to 100%

feature/ATC-114 - Adds _commandDefinition as a class property. Update…

7b9f05c

…s failing test for holdParser.

feature/ATC-114 - moves .unicodeToString() and UNICODE regex to prope…

bab0b0d

…r files

feature/ATC-114 - Updates tests for CommandParser

a0995da

feature/ATC-114 - Adds documentation to CammandParser and CommandMode…

53d2a9f

…l. Adds encapsulated boolean method for Parser

n8rzz commented Dec 9, 2016

View reviewed changes

Merge branch 'develop' of https://github.com/n8rzz/atc into feature/A…

4fa9c75

…TC-114 Conflicts: - src/assets/scripts/constants/globalConstants.js

n8rzz requested a review from erikquinn December 9, 2016 19:57

Merge branch 'develop' of https://github.com/n8rzz/atc into feature/A…

cdd1793

…TC-114

n8rzz commented Dec 10, 2016

View reviewed changes

n8rzz temporarily deployed to atc-dev-pr-179 December 10, 2016 02:41 Inactive

n8rzz added 2 commits December 9, 2016 22:32

feature/ATC-114 - Finishes major documentation

2933994

- Abstracts ERROR_MESSAGES to new file - Adds _parseUserCommand() method to InputController to encapsulate actual parsing/errors of the CommandParser - Adds validators and parsers to System commands

feature/ATC-114 - Updates failing test

7292ec7

n8rzz temporarily deployed to atc-dev-pr-179 December 10, 2016 04:54 Inactive

n8rzz temporarily deployed to atc-dev-pr-179 December 13, 2016 23:18 Inactive

feature/ATC-114 - removes dead code for airport system command and en…

9888008

…forces a single argument for airport command

n8rzz temporarily deployed to atc-dev-pr-179 December 14, 2016 23:27 Inactive

feature/ATC-114 - Adds custom validator for fix command

eb582dd

n8rzz temporarily deployed to atc-dev-pr-179 December 15, 2016 04:39 Inactive

n8rzz removed the WIP label Dec 15, 2016

n8rzz requested a review from Dabea December 15, 2016 04:39

erikquinn requested changes Dec 15, 2016

View reviewed changes

n8rzz added 2 commits December 15, 2016 19:41

Merge branch 'develop' of https://github.com/n8rzz/atc into feature/A…

4c56a4a

…TC-114

feature/ATC-114 - Adjusts spacing in keydown case blocks

cd2b71e

- Adds asterisk unicode to unicode commands

n8rzz temporarily deployed to atc-dev-pr-179 December 16, 2016 02:17 Inactive

erikquinn reviewed Dec 16, 2016

View reviewed changes

feature/ATC-114 - Adds new functions to support 1 to 3 args for hold …

0bd87b5

…that be received in any order - Adjusts spacing in InputController

n8rzz temporarily deployed to atc-dev-pr-179 December 16, 2016 04:55 Inactive

erikquinn requested changes Dec 16, 2016

View reviewed changes

feature/ATC-114 - Adjusts hold command to accept 0 to 3 arguments.

9419563

* Adds more documentation

n8rzz temporarily deployed to atc-dev-pr-179 December 16, 2016 13:44 Inactive

feature/ATC-114 - Adds missing space in InputController to subtract case

8f9c45e

n8rzz temporarily deployed to atc-dev-pr-179 December 16, 2016 13:47 Inactive

feature/ATC-114 - Adds updates to the docs for new CommandParser

fee93b2

n8rzz deployed to atc-dev-pr-179 December 16, 2016 19:56 View deployment

erikquinn approved these changes Dec 16, 2016

View reviewed changes

n8rzz merged commit 607d840 into develop Dec 16, 2016

n8rzz deleted the feature/ATC-114 branch December 16, 2016 23:36

erikquinn mentioned this pull request Jan 16, 2017

Command parser insensitive to context zlsa/atc#729

Closed

feature/ATC-114 #179

feature/ATC-114 #179

Conversation

n8rzz commented Dec 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n8rzz Dec 10, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n8rzz commented Dec 15, 2016

erikquinn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikquinn Dec 15, 2016 • edited by n8rzz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikquinn Dec 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n8rzz commented Dec 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n8rzz commented Dec 16, 2016

erikquinn commented Dec 16, 2016

n8rzz commented Dec 16, 2016

n8rzz commented Dec 6, 2016 •

edited

Loading

n8rzz Dec 10, 2016 •

edited

Loading

erikquinn Dec 15, 2016 •

edited by n8rzz

Loading

erikquinn Dec 15, 2016 •

edited

Loading