feature/ATC-151 #158
feature/ATC-151 #158
Conversation
…ethod to add new callsigns to the existing list.
…sion for when a flight number already exists
| aircraft_auto_toggle() { | ||
| prop.aircraft.auto.enabled = !this.aircraft.auto.enabled; | ||
| isCallsignInList(callsign) { | ||
| return this.aircraft.callsigns.indexOf(callsign) !== -1; |
There was a problem hiding this comment.
good thought but, unfortunately, _has only works with objects. indexOf is [I'm sure] quicker here anyway. If it 'aint broke don't fix it.
There was a problem hiding this comment.
Oh, okay, I figured it was useful because it would also check an array so you could use the same function since the two concepts are similar. But I know what happens when I assume...
| this.aircraft.callsigns.push(callsign); | ||
| addCallsignToList(callsign) { | ||
| if (this.isCallsignInList(callsign)) { | ||
| return; |
There was a problem hiding this comment.
It might be useful here to do a console.warn or something, as if we end up here with a callsign that's already in use, something has gone wrong, as it should be prevented before reaching the step of adding it to the callsign list.
There was a problem hiding this comment.
see my other comment re: generateFlightNumber + recursion. I think you're right, though, and I'll throw a console.warn here. It we've made it within this block something has gone very wrong.
| @@ -359,6 +347,7 @@ export default class AircraftController { | |||
| return this.aircraft.models[icao]; | |||
| } | |||
|
|
|||
| // TODO: what is an `eid` and why would it beed to be updated? | |||
There was a problem hiding this comment.
It's my moronic way to try to create unique IDs for individual aircraft so they can be distinguished from each other when talking back and forth between Aircraft and AircraftFlightManagamentSystem without the need for the existing circular dependencies. (points for trying to remove them, despite the lack of a good solution 😉)
So it's like the _id that all aircraft will inherit from BaseModel. It was meant as a way to distinguish, and if replaced with another method to do this, the EIDs can be removed.
Fun fact: it's for "entity ID", the verbage used in the simulation software I helped with when I worked for BGI on their MFOQA product for the US Navy.
There was a problem hiding this comment.
those circular dependencies are wicked. eid, ahh, yes that makes sense. So they aren't used for anything and can be removed? I'll add an issue for that (#161).
One thing to remember with an id is that once it assigned it should never change. because then you need to know if the id you are looking for is updated or a previous version, etc. That gets messy in a hurry, as I'm sure you can imagine.
| // TODO: Why is the `AircraftController` repsonsible for generating a callsign, but this is responsible for | ||
| // generating the flight number? callsign generation logi should live in this class. | ||
| options.callsign = window.aircraftController.aircraft_callsign_new(options.airline); | ||
| options.callsign = this.generateFlightNumber(); |
There was a problem hiding this comment.
@erikquinn options.callsign will always have a unique flightNumber. I've updated the generateFlightNumber method to only return a value if it doesn't already in the callsigns list. It's hidden, and it took me a few minutes to find it in the delta:
if (window.aircraftController.isCallsignInList(flightNumber)) {
return this.generateFlightNumber();
}
yay recursion!
…r.addCallsignToList()
|
Are you sure there isn't like a |
|
To the best of my knowledge, the only clone type operation is the I couldn't track down how a duplicate would even be possible before. With this, I see it even less likely, I think this a little bit more defensive than the last version. |
fixes #151
I simplified the current callsign generation process a little. I could not find any reason why there would ever be allowed to be duplicate callsigns, though. So I'm going to call this complete with the simplifications made.