Conversation
mmp
left a comment
There was a problem hiding this comment.
Looks great, this is a huge step forward.
Couple of comments; for all of them feel free to leave them as TODOs for future work (though I'll wait for responses before merging.)
(Looks like the build is failing due to some stt test merge issues; we can clean those up post merge if needed.)
| if _, ok := pos[id]; ok { | ||
| e.ErrorString("%s: TCP / sector_id used for multiple \"control_positions\"", id) | ||
| if existing, ok := pos[id]; ok { | ||
| // Allow aliases: the same controller stored under multiple keys |
There was a problem hiding this comment.
This feels a little fragile; what's it needed for?
| // BOS, MCO, etc. that aren't in the TRACONs database. | ||
| resourcesFS := util.GetResourcesFS() | ||
| filename := facility + ".json" | ||
| _ = fs.WalkDir(resourcesFS, "configurations", func(p string, d fs.DirEntry, err error) error { |
There was a problem hiding this comment.
I don't know how often this path runs, but it could be expensive to keep walking the directory tree versus doing it once and maintaining a map from facility to paths.
| if !(ctrl.SectorID[0] >= 'A' && ctrl.SectorID[0] <= 'Z') { | ||
| e.ErrorString("first character of center controller \"sector_id\" must be a letter") | ||
| } | ||
| if _, err := strconv.Atoi(ctrl.SectorID[1:]); err != nil { |
There was a problem hiding this comment.
Confirming, removing this was intended?
| // aircraft parameters. Returns the index into the FixPairs slice and true | ||
| // if a match is found, or -1 and false otherwise. | ||
| func MatchFixPair(fixPairs []FixPairDefinition, entryFix, exitFix string, flightType av.TypeOfFlight, altitude int) (int, bool) { | ||
| // Sort by priority (lower = higher priority) |
There was a problem hiding this comment.
It might be worth doing this sort once in PostDeserialize and then relying on it here.
This should be safe in practice due to the Sim holding a mutex when an RPC comes in, but if two goroutines ran MatchFixPair at the same time with the same underlying fixPairs, it could end up scrambled / with duplicate entries and missing entries if both sorted it at the same time.
| @@ -0,0 +1,121 @@ | |||
| // sim/facility_config.go | |||
There was a problem hiding this comment.
meta-comment: does it make sense moving some of the PostDeserialize code for these into this file to keep it closer to where things are defined? (That may not be doable / may not be worth the trouble depending on how much external info is needed to validate.)
| if sg.Airports == nil { | ||
| sg.Airports = make(map[string]*av.Airport) | ||
| } | ||
| sg.Airports[airport] = &av.Airport{} |
There was a problem hiding this comment.
This initially made me nervous since Airport is left totally uninitialized. I guess it's ok since it's not an airport in the local facility (so we don't care about departure procedures, etc.)? Maybe add a comment explaining a bit?
stars_configfrom scenario files into each facilities' own dedicated configuration files inresources/configurations/<artcc>/<facility>.stars_config.airspace_awareness/coordination_fixes.control_positionsfor that facility.controllersfrom scenarios. All controllers in that facility and neighboring facilities are loaded.<Facility> <##> <Name>for all enroute positions (eg,ZNY 56 Kennedy).<XXX>_<X>_APP/<XXX>_<X>_DEPfor all terminal positions (eg,EWR_P_APP).initial_facility,departure_facility, orreceiving_facilitymust be specified.index.htmlto reflect new definition changes.