zlsa / atc

https://openscope.co/
342 stars 107 forks source link

Command Parsing Issues #700

Closed kaitsu71 closed 7 years ago

kaitsu71 commented 7 years ago

f wetor command gives "command not understood".

joespeed52 commented 7 years ago

I also get this. I have no idea how to fix it.

erikquinn commented 7 years ago

Weird... any chance it also does this with other fixes that start with a "W" or a "T"? Maybe the peg parser thingy is confusing the first letter of the fix as a command...

Fechulo commented 7 years ago

@erikquinn That would make a lot of sense. In my experience it only happens with fixes that start with 'W' but I never noticed the relation between them.

Alpi-no commented 7 years ago

Could that be the reason that in LOWW I can't use the direct command for like 99% of the fixes? :/

erikquinn commented 7 years ago

@jakobeng1303 Probably! Do you know if it was always this way, or just since the "replace hand rolled command parsing" pull request?

Alpi-no commented 7 years ago

I haven't been able to play the game a lot lately but when I tried today it didn't work which, when I played last, just about when the new LOWW was merged, worked

Fechulo commented 7 years ago

@erikquinn It definitely hasn't been like this always, it's started happening to me quite recently, although I can't recall exactly when.

doofus commented 7 years ago

All the fixes that start with 'W' in KORD also have this problem. For some reason the "TONIE" fix at KORD does this as well. Other fixes that start with 'T' at KORD do not exhibit this problem.

erikquinn commented 7 years ago

Okay, that must be it then. to and w are being recognized as commands within WETOR and TONIE, and it doesn't know what to make of w [space] ETOR or to [space] NIE. This could be a tough fix, as I haven't been able to make that much sense out of how the pegparser works.

doofus commented 7 years ago

I wanted to test out some other fix names to see how they behaved. I made a testing branch and renamed some of the KORD fixes to be prefixed with some of the commands listed in parser.js. The following did not work as prefixes to fix names:

They were as prefixes to fix names: e.g CTOSS, WAITS, CVSBB, TAXID, CAFES, DVSDD. They produced the same bug as reported. It only seems to happen when these commands prefix a fix name. This list is not exhaustive, as I don't have time to try them all. However, there does seem to be a trend as most of these bogus fix/commands seems to be related to stuff that happens before the aircraft is in the air (expect for dvs and maybe abort). Maybe this indicates some sort of aircraft state issue that impacts the parser?

There is also something else. Using the ROUTE or RR with the bogus fix names DOES work. I can do KLM214 ROUTE TONIE and the command will be accepted and the aircraft will indeed fly to the TONIE fix. Works with all the other bogus fix names I listed above as well.

I hope this info helps with tracking down this bug.

erikquinn commented 7 years ago

The command hold lozit (KSFO) also is not understood; presumably related to the same issue with the command parsing.

Fechulo commented 7 years ago

Here's a few others I found: -hold LAM -dct WHAMY -dct TOU -dct TOR

n8rzz commented 7 years ago

adding https://github.com/n8rzz/atc/pull/179 and #755 for tracking purposes

n8rzz commented 7 years ago

@Fechulo just for fun, I've added tests to the new CommandParser for the specific use cases you mentioned. All green!

Fechulo commented 7 years ago

@n8rzz Great news, thanks!

n8rzz commented 7 years ago

@Fechulo I forgot, if you'd like to try it out, you can play with it here: https://atc-dev-pr-179.herokuapp.com/

There is one major change with how this works from the current version (there will be doc updates to go along with this): every command must be followed by a space. in most cases, that was the existing syntax. But in the case of the fly heading alias,fh270 is now fh 270.

Also, if you run into any issues, please log them under the PR

eliuuk commented 7 years ago

For me, the best thing is that fh is now separated with a space! Thanks for that :P