zlsa / atc

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

Assigning SID or executing "caf" before taxi breaks game #761

Closed szabodabo closed 7 years ago

szabodabo commented 7 years ago

If a newly-spawned departure is assigned a SID (either with the sid command or via the caf command) while it is at status apron (has not taxi'd, has not been assigned a runway), the game is unusable and spams the console with these logs:

RouteModel.js:46 Invalid data type passed to RouteModel. Expected a string but received [object Object]RouteModel @ RouteModel.js:46generateWaypoints @ Leg.js:97parse @ Leg.js:75Leg @ Leg.js:57prependLeg @ AircraftFlightManagementSystem.js:151followSID @ AircraftFlightManagementSystem.js:526setDepartureRunway @ AircraftInstanceModel.js:346runTaxi @ AircraftInstanceModel.js:1444run @ AircraftInstanceModel.js:717runCommands @ AircraftInstanceModel.js:627input_run @ InputController.js:810onCommandInputKeydownHandler @ InputController.js:443(anonymous function) @ InputController.js:136dispatch @ jquery.js:5201elemData.handle @ jquery.js:5009
Leg.js:190 Uncaught TypeError: Cannot read property 'length' of undefined(…)_generateWaypointsForSid @ Leg.js:190generateWaypoints @ Leg.js:98parse @ Leg.js:75Leg @ Leg.js:57prependLeg @ AircraftFlightManagementSystem.js:151followSID @ AircraftFlightManagementSystem.js:526setDepartureRunway @ AircraftInstanceModel.js:346runTaxi @ AircraftInstanceModel.js:1444run @ AircraftInstanceModel.js:717runCommands @ AircraftInstanceModel.js:627input_run @ InputController.js:810onCommandInputKeydownHandler @ InputController.js:443(anonymous function) @ InputController.js:136dispatch @ jquery.js:5201elemData.handle @ jquery.js:5009
AircraftFlightManagementSystem.js:1003 Uncaught TypeError: Cannot read property 'altitude' of null(…)altitudeForCurrentWaypoint @ AircraftFlightManagementSystem.js:1003updateTarget @ AircraftInstanceModel.js:1938update @ AircraftInstanceModel.js:2612aircraft_update @ AircraftController.js:155recalculate @ AirportController.js:78update @ App.js:326(anonymous function) @ App.js:323
AircraftFlightManagementSystem.js:1003 Uncaught TypeError: Cannot read property 'altitude' of null(…)altitudeForCurrentWaypoint @ AircraftFlightManagementSystem.js:1003updateTarget @ AircraftInstanceModel.js:1938update @ AircraftInstanceModel.js:2612aircraft_update @ AircraftController.js:155recalculate @ AirportController.js:78update @ App.js:326(anonymous function) @ App.js:323
AircraftFlightManagementSystem.js:1003 Uncaught TypeError: Cannot read property 'altitude' of null(…)altitudeForCurrentWaypoint @ AircraftFlightManagementSystem.js:1003updateTarget @ AircraftInstanceModel.js:1938update @ AircraftInstanceModel.js:2612aircraft_update @ AircraftController.js:155recalculate @ AirportController.js:78update @ App.js:326(anonymous function) @ App.js:323
AircraftFlightManagementSystem.js:1003 Uncaught TypeError: Cannot read property 'altitude' of null(…)altitudeForCurrentWaypoint @ AircraftFlightManagementSystem.js:1003updateTarget @ AircraftInstanceModel.js:1938update @ AircraftInstanceModel.js:2612aircraft_update @ AircraftController.js:155recalculate @ AirportController.js:78update @ App.js:326(anonymous function) @ App.js:323
AircraftFlightManagementSystem.js:1003 Uncaught TypeError: Cannot read property 'altitude' of null(…)altitudeForCurrentWaypoint @ AircraftFlightManagementSystem.js:1003updateTarget @ AircraftInstanceModel.js:1938update @ AircraftInstanceModel.js:2612aircraft_update @ AircraftController.js:155recalculate @ AirportController.js:78update @ App.js:326(anonymous function) @ App.js:323
AircraftFlightManagementSystem.js:1003 Uncaught TypeError: Cannot read property 'altitude' of null(…)altitudeForCurrentWaypoint @ AircraftFlightManagementSystem.js:1003updateTarget @ AircraftInstanceModel.js:1938update @ AircraftInstanceModel.js:2612aircraft_update @ AircraftController.js:155recalculate @ AirportController.js:78update @ App.js:326(anonymous function) @ App.js:323
AircraftFlightManagementSystem.js:1003 Uncaught TypeError: Cannot read property 'altitude' of null(…)

This is with the current version running on https://atc-release.herokuapp.com (not sure which, but log says [ INFO ] Version v3.0.0, though this is not broken at http://zlsa.github.io/atc/, which also logs [ INFO ] Version v3.0.0).

panther2 commented 7 years ago

Interesting. I tried it out at https://atc-release.herokuapp.com and it works for me. No problems when taxi after caf on apron - no messages in the console, either.

I encountered a different problem, though: #760

n8rzz commented 7 years ago

@panther2 @szabodabo do you guys see the same thing on https://atc-dev.herokuapp.com? This is the env for my fork's develop branch. so this is the cutting edge and also not fully tested, so keep that in mind.

I'll dig in on my end, too, and see what I can find.

szabodabo commented 7 years ago

ok, I ran a local server with n8rzz/atc@ac05e5d6b69, which is the latest commit in your develop branch, and I could reproduce it... maybe this issue should be moved over to https://github.com/n8rzz/atc?

do you have a pointer to the development process for the project? why does your development clone have its own issue tracker? where do contributors usually send pull requests?

(and thanks for making this awesome simulator, btw)

erikquinn commented 7 years ago

It's fine to leave the issue here. As for the second part, yrs it is confusing. ;) That will be changing some time soon here.

n8rzz commented 7 years ago

@szabodabo you're working too hard 😉, the heroku link is the most recent commit to develop.

My fork started as a complete refactor of the JS code when we converted from ES5 to ES2016 and added tooling (https://github.com/zlsa/atc/pull/702). Since then, it has assumed the role of javascript improvements. The issues we have there are mostly for continuous development, but there are also some bugs that are being worked on as part of our release cycle.

you can take a look at the tooling readme. We don't have any other ramp up docs, yet.

szabodabo commented 7 years ago

@n8rzz thanks for the info. seems like community contributions in zlsa/atc might get annoying when you want to merge n8rzz/atc back in at release time?

I was playing on https://atc-release.herokuapp.com because I was running into #747 or a similar bug (fixed in your develop branch, apparently).

that will be changing some time soon here.

@erikquinn which part?

n8rzz commented 7 years ago

@szabodabo not at all. That's why we do it this way. We can work on the JS unencumbered by new airports and merge in a finial set of changes. New airports/aircraft/airlines work pretty much independently of the JS. so contributors can make changes without having to know anything about the js.

erikquinn commented 7 years ago

@szabodabo I just mean the separate stacks of issues will eventually be all in one place.

n8rzz commented 7 years ago

@szabodabo try it on this one https://atc-dev-pr-179.herokuapp.com/

This branch contains code for the new CommandParser but, as part of that feature work, I re-worked the method I believe to be causing your issue. I, however, have not been able to reproduce the problems you were seeing.

Which airport were you using? Is there anything else that you did that could help me try to reproduce your error?


RE merges and such: we do a PR process for all the code in my fork just like we do here in the main repo. Mine has a few more bells and whistles, (like CI that runs test on every branch and heroku release apps we can spin up for PRs) but the process is the same. We continue to add new features to develop and address bugs on release/x.x.x. eventually, a week or so before release is planned to merge into this fork, I open a PR and post links for everybody so they can test it out and uncover bugs we missed. Once we are satisfied all the major bugs are squashed, my current release branch gets merged into the gh-pages branch here. Then we repeat the process.

Basically new JS in my fork, everything else here.

erikquinn commented 7 years ago

@szabodabo Are you still seeing this problem on the current version at https://zlsa.github.io/atc? The linked PR above has made its way into the production version of the sim.

If it cannot be reproduced, I'd like to close this and we can re-investigate if it ever does resurface, but I would agree with @n8rzz that the changes to the CommandParser would very, very likely have resolved whatever was causing the problem.

erikquinn commented 7 years ago

Assuming this to be resolved.

Closing.