-
Notifications
You must be signed in to change notification settings - Fork 185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bugfix/906 #912
bugfix/906 #912
Conversation
…nce an aircraft has left controlled airspace * updates aircraft to aircraftModel in AircraftController.update() * adds .departureExit() method to AircraftModel and consolidates .setIsRemovable() calls
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.
It's still not working as desired. In fact, it's even more broken now. Strips don't disappear at all, even after the aircraft disappears from the scope.
@Fechulo be sure to clear your cache. I tested this locally with a single departure spawnPattern and it was working just fine for me. I’ll give it a shot again once I get home, but it was working. |
💩 But I didn’t test arrivals. Whoops. |
@n8rzz It doesn't work with departures either on my side, clearing the cache didn't change anything. |
Well shit |
// an aircraft is exiting controlled airspace but the aircraft strip | ||
// is still in the DOM. this where the strip gets removed from the DOM | ||
if (!aircraftModel.inside_ctr && aircraftModel.isRemovable) { | ||
this.removeStripView(aircraftModel); |
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.
Wouldn't it make more sense to remove the strip upon AircraftModel.onAirspaceExit()
and AircraftController.aircraft_remove()
instead of using conditional logic within the StripViewModel itself?
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.
The AircraftModel
has no knowledge of the StripView
, only the AircraftController
does and .aircraft_remove()
only fires once an aircraft lands or is way outside controlled airspace.
The removing logic should be consolidated and updated first before this PR does its thing.
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.
We could also perhaps use event bus callbacks? EVENT.EXIT_AIRSPACE
and EVENT.REMOVE_AIRCRAFT
or similar, which call the appropriate methods? Just spitballing anyway.
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.
We could do that, but we'd still have some spaghetti code. see: #916 for future work on all that
@n8rzz Okay to delete this branch? |
@erikquinn yes, I'll do that |
Resolves #906
Updates
StripViewController
to remove a flight strip once an aircraft has left controlled airspace