feature/ATC-134 #209
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1382,7 +1382,7 @@ export default class Aircraft { | |
| * @param data | ||
| */ | ||
| runReroute(data) { | ||
| // TODO: capitalize everything? | ||
| data = data[0].toUpperCase(); | ||
| let worked = true; | ||
| const route = this.fms.formatRoute(data); | ||
|
|
@@ -1473,7 +1473,7 @@ export default class Aircraft { | |
| return ['fail', 'inbound']; | ||
| } | ||
|
|
||
| if (!this.isOnGround()) { | ||
| return ['fail', 'already airborne']; | ||
| } | ||
| if (this.mode === FLIGHT_MODES.APRON) { | ||
|
|
@@ -1704,6 +1704,41 @@ export default class Aircraft { | |
| return this.approachOffset <= 0.048; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the aircraft is inside the airspace of a specified airport | ||
| * | ||
| * @for AircraftInstanceModel | ||
| * @method isInsideAirspace | ||
| * @param {airport} airport the airport whose airspace we are checking | ||
| * @return {Boolean} | ||
| * @private | ||
| */ | ||
| isInsideAirspace(airport) { | ||
|
There was a problem hiding this comment. you don't need to do this, but in cases like this where you only need a few properties from a massive object you could destructure them: isInsideAirspace(airport) { }
// becomes
isInsideAirspace({ perimeter, ctr_radius, ctr_ceiling }) { }There was a problem hiding this comment. 👍 Good information; wouldn't have thought of that. Didn't implement here... thought it looked more clean just as There was a problem hiding this comment. totally cool. I just wanted to call that out in case you ever need it in the future 😉 |
||
| let withinAirspaceLateralBoundaries = this.distance <= airport.ctr_radius; | ||
| const withinAirspaceAltitudeRange = this.altitude <= airport.ctr_ceiling; | ||
|
|
||
| if (!_isNil(airport.perimeter)) { // polygonal airspace boundary | ||
| withinAirspaceLateralBoundaries = point_in_area(this.position, airport.perimeter); | ||
| } | ||
|
|
||
| return withinAirspaceAltitudeRange && withinAirspaceLateralBoundaries; | ||
|
There was a problem hiding this comment. yes!!! 👍 one thing to remember and I don't think we've gone over this yet, so I'll let this go as is this time. When a method returns a boolean or a constant is a boolean, it should be prepended with return isWithinAirspaceAltitudeRange && isWithinAirspaceLateralBoundaries;this reads a little better and you can immediately tell that those two things are booleans. |
||
| } | ||
|
|
||
| /** | ||
| * Aircraft has "weight-on-wheels" (on the ground) | ||
| * @for AircraftInstanceModel | ||
| * @method isOnGround | ||
| */ | ||
| isOnGround() { | ||
| const error_allowance_ft = 5; | ||
| const airport = window.airportController.airport_get(); | ||
| const runway = airport.getRunway(this.rwy_dep || this.rwy_arr); | ||
|
There was a problem hiding this comment. is this assuming that one of There was a problem hiding this comment. get activeRunway() {
return this.rwy_dep
? this.rwy_dep
: this.rwy_arr;
}There was a problem hiding this comment. this is the one instance where I am ok with ternaries, by the way. In a case like this I think it makes sense and is just as readable as the alternative There was a problem hiding this comment. that getter simply doesn't make sense though... it's not in relation to the airport, it has to do with each individual aircraft. In the current way the game works, each aircraft will only have one of There was a problem hiding this comment. yeah, I understand. and you're right, the getter would be in the Essentially whats happening here, from what I understand, is 'give me the runway you used/or are going to use'. This is where the getter would be helpful because it can do that logic on its own. Thus, anything calling the getter simply says 'give me the runway info' and the getter figures out which one it should send back. and then everything is right in the world. But, if this isn't used all over the place, it probably isn't really worth the effort. There was a problem hiding this comment. added #225 so if we ever decide to fix it, we have an issue to refer to :-) There was a problem hiding this comment. it's not like it's a lot of effort, but this is literally the only place it would be used. Adding it would increase readability, but shouldn't single-use methods be avoided if possible? If not, I could just add another method to the pile-- no biggie. |
||
| const nearRunwayAltitude = abs(this.altitude - runway.elevation) < error_allowance_ft; | ||
| const nearAirportAltitude = abs(this.altitude - airport.position.elevation) < error_allowance_ft; | ||
|
|
||
| return nearRunwayAltitude || nearAirportAltitude; | ||
| } | ||
|
|
||
| /** | ||
| * Aircraft is actively following an instrument approach and is elegible for reduced separation | ||
| * | ||
|
|
@@ -1725,7 +1760,7 @@ export default class Aircraft { | |
| */ | ||
| isStopped() { | ||
| // TODO: enumerate the magic number. | ||
| return this.isOnGround() && this.speed < 5; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1980,7 +2015,7 @@ export default class Aircraft { | |
| this.fms.setCurrent({ start_speed: this.fms.currentWaypoint.speed }); | ||
| } | ||
|
|
||
| if (this.isOnGround()) { | ||
| this.target.altitude = runway.elevation; | ||
| this.target.speed = 0; | ||
| } else { | ||
|
|
@@ -2136,7 +2171,7 @@ export default class Aircraft { | |
| } | ||
|
|
||
| // If stalling, make like a meteorite and fall to the earth! | ||
| if (this.speed < this.model.speed.min && !this.isOnGround()) { | ||
| this.target.altitude = Math.min(0, this.target.altitude); | ||
| } | ||
|
|
||
|
|
@@ -2235,7 +2270,7 @@ export default class Aircraft { | |
|
|
||
| // TURNING | ||
| // this.target.heading = radians_normalize(this.target.heading); | ||
| if (!this.isOnGround() && this.heading !== this.target.heading) { | ||
| // Perform standard turns 3 deg/s or 25 deg bank, whichever | ||
| // requires less bank angle. | ||
| // Formula based on http://aviation.stackexchange.com/a/8013 | ||
|
|
@@ -2290,7 +2325,7 @@ export default class Aircraft { | |
| } | ||
| } | ||
|
|
||
| if (this.isOnGround()) { | ||
| this.trend = 0; | ||
| } | ||
|
|
||
|
|
@@ -2300,7 +2335,7 @@ export default class Aircraft { | |
| if (this.target.speed < this.speed - 0.01) { | ||
| difference = -this.model.rate.decelerate * window.gameController.game_delta() / 2; | ||
|
|
||
| if (this.isOnGround()) { | ||
| difference *= 3.5; | ||
| } | ||
| } else if (this.target.speed > this.speed + 0.01) { | ||
|
|
@@ -2348,7 +2383,7 @@ export default class Aircraft { | |
| const wind = window.airportController.airport_get().wind; | ||
| let vector; | ||
|
|
||
| if (this.isOnGround()) { | ||
| vector = vscale([sin(angle), cos(angle)], scaleSpeed); | ||
| } else { | ||
| let crab_angle = 0; | ||
|
|
@@ -2387,22 +2422,10 @@ export default class Aircraft { | |
| this.radial += tau(); | ||
| } | ||
|
|
||
| const isInsideAirspace = this.isInsideAirspace(window.airportController.airport_get()); | ||
|
|
||
| if (isInsideAirspace !== this.inside_ctr) { | ||
|
There was a problem hiding this comment. remember to add a line between constants and conditional blocks |
||
| this.crossBoundary(isInsideAirspace); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -2481,7 +2504,7 @@ export default class Aircraft { | |
| }); | ||
| } | ||
|
|
||
| if (this.terrain_ranges && !this.isOnGround()) { | ||
| const terrain = prop.airport.current.terrain; | ||
| const prev_level = this.terrain_ranges[this.terrain_level]; | ||
| const ele = Math.ceil(this.altitude, 1000); | ||
|
|
@@ -2664,17 +2687,4 @@ export default class Aircraft { | |
| removeConflict(other) { | ||
| delete this.conflicts[other.getCallsign()]; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I like this name a lot better. doesn't leave anything to the imagination about what its for.