feature/ATC-53 #89

n8rzz · 2016-11-14T03:20:43Z

I have questions for some of this stuff...

… feature work

…ature/ATC-53

* Updates refs for new getters * Begins abstracting waypoint creation to individual methods * Adds _get for setting properties from passed in object in Leg and Waypoint models

…TC-53

…ile.

…t to RouteModel.

* Adds aircraftConstants to Leg * Adds new method to get waypoint models for sid in AirportModel

StandardRouteCollections when dealing with sids and stars. * Removes inline string transformation logic from Leg

…he passed fixName before searching.

…eModel within the AircraftInstanceModel - Adds doc blocks to Leg - Adds new methods to Leg to reset the current waypoint array and for adding a new Waypoint to the waypoint array.

…TC-53

* Removes fms.clearedAsFiled() method * Consolidates runSID() and climbViaSid() logic

…TC-53

n8rzz

@erikquinn a few questions for you on this feature

n8rzz · 2016-11-14T03:21:22Z

src/assets/scripts/aircraft/AircraftFlightManagementSystem.js

        // set initial
        this.fp.altitude = clamp(1000, options.model.ceiling, 60000);

        if (options.aircraft.category === 'arrival') {
+            // TODO: why KDBG?


Is this just an arbitrary icao? or is there some meaning here?

correct. back in the good old days, "KDBG", aka "Debug Airport" was the first airport in the game. When needing to fill the origin and destination airports in, "KDBG" was an arbitrary choice so it wasn't empty.

I feel like this is confusing. Perhaps we could update this to be a little clearer like UNASSIGNED or something. I'll add an issue for future consideration #102.

Yeah it's misleading. Something like that would be perfectly fine.

n8rzz · 2016-11-14T03:24:35Z

src/assets/scripts/aircraft/AircraftFlightManagementSystem.js

                    // arrival airport
-                    r.push(this.legs[l].route.split('.')[2]);
+                    flightPlanRoute.push(leg.route.exit);

                    break;
                case FP_LEG_TYPE.IAP:
                    // no need to include these in flightplan (because wouldn't happen in real life)
                    break;
                case FP_LEG_TYPE.AWY:


Are airways in use in the app yet or is that a future enhancement?

Yes and no; they just have to be added to the airport files. They should work fine once airports have airways added in.

n8rzz · 2016-11-14T03:27:51Z

src/assets/scripts/aircraft/AircraftFlightManagementSystem.js

-            // add speeds to wp
-            wp[i].speed = spd;
+            waypoint.setAltitude(ctr_ceiling, cruise_alt);
+            waypoint.setSpeed(cruise_spd);


so now I have model methods doing this logic, but I feel they should be run when the waypoints are set not here after the fact.

Probably not. Here's an example:

You can see there are altitude restrictions obviously, but aircraft not having been told to "descend via the KORRY3 arrival" would have last been told to "climb and maintain FL370" (or whatever their cruise altitude is. So they have to maintain that until being told to cvs or dvs.

So the setAltitude and setSpeed methods actually put the restrictions in the altitude property of the given waypoints, and this should not be done until they are are cleared to descend via STAR or climb via SID. Currently, the former is done automatically on spawn, but the actions of storing the restrictions and instructing to comply with the restrictions should remain separate.

What you're saying makes sense.

I guess what I was getting at here is that once we know an aircraft is going to fly the sid/star, we should set the restrictions then when we create the Waypoints. The fms doesnt care about future restrictions, only the restrictions for the current Waypoint. Maybe that isn't practical in this case. ehh :-/

The issue is that the restrictions are only ro be followed after they have been cleared to "descend via the GSLIN8 arrival" or "climb via SID".

If they aren't told to do this, they aren't allowed to do anything regarding those posted altitudes; they must just remain at the altitude last assigned.

So assigning the altitude waypoint based on the fixRestrictions should not be done until that cvs / dvs command is given.

Is that more clear hopefully? I don't think I made much sense in the last comment.

* Adds .getStarLegWaypoints() * Removes dead code

n8rzz · 2016-11-14T13:23:54Z

src/assets/scripts/aircraft/Waypoint.js

+        } else if (firstRouteSegment === 'KDBG' && this.heading === null) {
+            // FIXME: radial is not defined or set anywhere in this class
+            // aim arrival @ middle of airspace
+            this.heading = this.radial + Math.PI;


where is radial coming from? This is the only spot it exists within this class?

This block seems to be for arrivals that somehow got messed up and have no waypoint to fly to or no assigned heading. this.radial is one of those old properties of the aircraft model that I had mentioned a while ago that should eventually be removed. Nonetheless, this.radial is the bearing (radians) from the airport to the aircraft. Thus, line 160 is just telling the aircraft to fly on a heading to the airport so they don't get stuck out in the abyss outside the airspace boundary.

except this.radial is never used in Waypoint. This is the only spot its mentioned, which leads me to believe that when the game is being played, this section never gets hit. This would fail if it did or at least cause some weirdness

The truth is, this.radial predates the entire fms, and is no longer used for anything important. It is one of the many things I intend to clean out of the aircraft object at some point here.

any qualms with me removing it from this block entirely? I'd like to try and remove stale code whenever possible :-)

There might be some airports that use it. Maybe MSP off the top of my head? But I'd have to check. In general, I have no qualms though; I'd love to see it gone.

n8rzz · 2016-11-15T18:45:10Z

src/assets/scripts/aircraft/Waypoint.js

+            const minAlt = parseInt(altitudeRestriction.replace(ABOVE_SYMBOL, ''), DECIMAL_RADIX);
+            const minimumAltitudeWithoutSymbol = minAlt * FL_TO_THOUSANDS_MULTIPLIER;
+
+            this.altitude = Math.min(minimumAltitudeWithoutSymbol, cruiseAltitude);


@erikquinn is Math.min correct here? I would thin it should be cruiseAltitude unless minimumAltitudeWithoutSymbol is higher.

You're right; this should be max not min.

…ypoint * Adds tests and supporting fixutes and mocks for Waypoint

…TC-53 Conflicts: - src/assets/scripts/canvas/CanvasController.js - test/airport/fix/FixCollection.spec.js

…s, adds additional test case.

merged incorrectly causing failing tests.

….setAltitude() and .setSpeed(). removes ava.skip from skipped tests in Waypoint.spec

- Replaces KDGB with UNASSIGNED

…TC-53

n8rzz · 2016-11-21T01:45:50Z

src/assets/scripts/aircraft/AircraftFlightManagementSystem.js


        // TODO: simplify this
        // FIXME: is this.legs an array?
-        for (const l in this.legs) {


remove comments

n8rzz · 2016-11-21T01:47:39Z

src/assets/scripts/aircraft/AircraftFlightManagementSystem.js

        const legs = [];
-        const curr = this.currentWaypoint(); // save the current waypoint
+        // const legs = this._generateLegsForFixOrStandardRoute(route);


remove comment

n8rzz · 2016-11-21T01:47:59Z

src/assets/scripts/aircraft/AircraftFlightManagementSystem.js


-                if (Object.keys(window.airportController.airport_get().sids).indexOf(pieces[1]) > -1) {
+                if (typeof currentAirport.sidCollection.findRouteByIcao(routeModel.procedure) !== 'undefined') {


replace with _isNil()

n8rzz · 2016-11-21T01:49:22Z

src/assets/scripts/aircraft/AircraftFlightManagementSystem.js

@@ -975,7 +921,8 @@ export default class AircraftFlightManagementSystem {
            return null;
        }

-        return `${this.following.sid}.${this.currentLeg().route.split('.')[2]}`;
+        // TODO: use the `RouteModel` for this


remove comment

…TC-53

* Refactors StandardRouteModel to use _entryCollection, _bodySegmentModel and _exitCollection for both SIDs and STARs * Updates tests * Adds istanbul-ignore to constructors inheriting one of the Base classes

n8rzz · 2016-11-22T04:25:44Z

@erikquinn ready for you.

I had to modify how the StandardRouteModel was working a little. Its a good thing I have tests for those files, that made it much easier to do. Its also more generalized now. Take a peak, run it through its paces and let me know what you think!

…TC-53

erikquinn · 2016-11-24T00:04:22Z

src/assets/scripts/aircraft/AircraftFlightManagementSystem.js

        this.legs = [];
+
        this.current = [0, 0]; // [current_Leg, current_Waypoint_within_that_Leg]


What are the nerdery conventions on whitespacing between properties like this?

usually, not cool. But there should be doc blocks for each property which is why, I think, I left the spacing here.

I'll add those now.

…TC-53

erikquinn · 2016-11-24T00:10:59Z

src/assets/scripts/aircraft/AircraftFlightManagementSystem.js

@@ -180,15 +206,17 @@ export default class AircraftFlightManagementSystem {
     *  Insert a waypoint after the *current* waypoint
     */
    appendWaypoint(data) {
-        this.currentLeg().waypoints.splice(this.current[WAYPOINT_WITHIN_LEG] + 1, 0, new Waypoint(data, this));
+        const airport = window.airportController.airport_get();


At some point, we might want to change airport_get() to a getter. Just a thought.

added #150 for this

…ass properties

…TC-53

n8rzz added 20 commits October 31, 2016 07:21

feature/ATC-53 - Adds TODO and FIXME comments to fms file in prep for…

d54a136

… feature work

Merge branch 'feature/ATC-18' of https://github.com/n8rzz/atc into fe…

89aa66e

…ature/ATC-53

feature/ATC-53 - Creates getters for currentLeg and currentWaypoint

767c973

* Updates refs for new getters * Begins abstracting waypoint creation to individual methods * Adds _get for setting properties from passed in object in Leg and Waypoint models

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

0e28eb0

…TC-53

feature/ATC-53 - adds .getFixPositionCoordinates() to FixCollection.

c2c17b5

feature/ATC-53 - Adds aircraftConstants and updates refs to use new f…

b93a087

…ile.

feature/ATC-53 - Cleans and streamlines Waypoint, adds example commen…

2c21396

…t to RouteModel.

feature/ATC-53 - Adds docblocs to Leg

f93adc3

* Adds aircraftConstants to Leg * Adds new method to get waypoint models for sid in AirportModel

feature/ATC-53 - Adds restriction parsing to StandardRouteWaypointModel

cf98892

feature/ATC-53 - Adds .gemerateFmsWaypoint() to StandardWaypointModel.

c2a945f

feature/ATC-53 - Updates Leg to use the RouteModel and

f756539

StandardRouteCollections when dealing with sids and stars. * Removes inline string transformation logic from Leg

feature/ATC-53 - Updates FixCollection.findFixByName() to uppercase t…

2d7349c

…he passed fixName before searching.

feature/ATC-53 - Begins implementing sid and starCollection with Rout…

26a1b4d

…eModel within the AircraftInstanceModel - Adds doc blocks to Leg - Adds new methods to Leg to reset the current waypoint array and for adding a new Waypoint to the waypoint array.

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

941811e

…TC-53

feature/ATC-53 - Updates comments and removes old TODOs

0809408

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

6c1442e

…TC-53

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

403fc37

…TC-53

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

33db91c

…TC-53

feature/ATC-53 - Abstracts restrictions logic to Waypoint

0109343

* Removes fms.clearedAsFiled() method * Consolidates runSID() and climbViaSid() logic

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

42fc97c

…TC-53

n8rzz added feature refactor labels Nov 14, 2016

n8rzz added this to the v3.1.0 milestone Nov 14, 2016

n8rzz commented Nov 14, 2016

View reviewed changes

feature/ATC-53 - Simplifies .descendViaSTAR()

733547d

* Adds .getStarLegWaypoints() * Removes dead code

n8rzz commented Nov 14, 2016

View reviewed changes

n8rzz commented Nov 15, 2016

View reviewed changes

n8rzz added 3 commits November 15, 2016 15:09

feature/ATC-53 - Replaces fms ref with airportInstanceModel ref in Wa…

20041e4

…ypoint * Adds tests and supporting fixutes and mocks for Waypoint

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

d755738

…TC-53 Conflicts: - src/assets/scripts/canvas/CanvasController.js - test/airport/fix/FixCollection.spec.js

feature/ATC-53 - Adds _get to Waypoint for populating class propertie…

0a3900f

…s, adds additional test case.

n8rzz added 4 commits November 19, 2016 18:11

feature/ATC-53 - Fixes merge conflict wonkiness, several items were

3643da1

merged incorrectly causing failing tests.

feature/ATC-53 - Updates Waypoint to use ternay assignment inside of …

29931de

….setAltitude() and .setSpeed(). removes ava.skip from skipped tests in Waypoint.spec

feature/ATC-53 - updates _generateManualWaypoint() to pass route not fix

fa548e2

feature/ATC-53 - Adds enum for flight category

a4ee77d

- Replaces KDGB with UNASSIGNED

n8rzz changed the title ~~[WIP] feature/ATC-53~~ feature/ATC-53 Nov 20, 2016

n8rzz removed the WIP label Nov 20, 2016

erikquinn modified the milestones: v3.2.0, v3.1.0 Nov 20, 2016

n8rzz assigned erikquinn Nov 20, 2016

n8rzz added 2 commits November 20, 2016 11:54

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

767c40c

…TC-53

feature/ATC-53 - Uncomments ga block in index.html

4776baa

n8rzz commented Nov 21, 2016

View reviewed changes

n8rzz added 4 commits November 20, 2016 20:43

feature/ATC-53 - adds _isNil to fms

d0fb9f9

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

ce8690c

…TC-53

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

5814e67

…TC-53

feature/ATC-53 - Removes _runwayCollection from StandardRouteModel

b8fda4b

* Refactors StandardRouteModel to use _entryCollection, _bodySegmentModel and _exitCollection for both SIDs and STARs * Updates tests * Adds istanbul-ignore to constructors inheriting one of the Base classes

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

d9346e9

…TC-53

This was referenced Nov 22, 2016

feature/ATC-125 #141

Merged

Move fms files to new directory #128

Closed

rename controller files that are not controllers to collection #83

Closed

erikquinn reviewed Nov 24, 2016

View reviewed changes

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

defce8d

…TC-53

erikquinn reviewed Nov 24, 2016

View reviewed changes

feature/ATC-53 - Adds doc blocks to AircraftFlightManagementSystem cl…

2a92c41

…ass properties

erikquinn approved these changes Nov 24, 2016

View reviewed changes

n8rzz added 2 commits November 23, 2016 21:08

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

4eefce9

…TC-53

feature/ATC-53 - Updates changelog

cbfd296

n8rzz merged commit 840e073 into develop Nov 24, 2016

n8rzz deleted the feature/ATC-53 branch November 24, 2016 04:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature/ATC-53 #89

feature/ATC-53 #89

n8rzz commented Nov 14, 2016 •

edited

Loading

n8rzz left a comment

n8rzz Nov 14, 2016

erikquinn Nov 14, 2016

n8rzz Nov 16, 2016

erikquinn Nov 17, 2016

n8rzz Nov 14, 2016

erikquinn Nov 14, 2016

n8rzz Nov 14, 2016

erikquinn Nov 14, 2016

n8rzz Nov 16, 2016

erikquinn Nov 17, 2016

n8rzz Nov 14, 2016

erikquinn Nov 14, 2016

n8rzz Nov 16, 2016

erikquinn Nov 17, 2016

n8rzz Nov 17, 2016

erikquinn Nov 17, 2016

n8rzz Nov 15, 2016

erikquinn Nov 17, 2016

n8rzz Nov 21, 2016

n8rzz Nov 21, 2016

n8rzz Nov 21, 2016

n8rzz Nov 21, 2016

n8rzz commented Nov 22, 2016

erikquinn Nov 24, 2016

n8rzz Nov 24, 2016

n8rzz Nov 24, 2016

erikquinn Nov 24, 2016

n8rzz Nov 24, 2016


		if (Object.keys(window.airportController.airport_get().sids).indexOf(pieces[1]) > -1) {
		if (typeof currentAirport.sidCollection.findRouteByIcao(routeModel.procedure) !== 'undefined') {

		this.legs = [];

		this.current = [0, 0]; // [current_Leg, current_Waypoint_within_that_Leg]

feature/ATC-53 #89

feature/ATC-53 #89

Conversation

n8rzz commented Nov 14, 2016 • edited Loading

n8rzz 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

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

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 Nov 22, 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

Choose a reason for hiding this comment

n8rzz commented Nov 14, 2016 •

edited

Loading