fix: address review feedback for sweatbox engine skeleton

Reject NaN/Inf geometry on add; clear Ident on ss; combined runway
name defaults to RwyA; simplify rules switch; test SetMaxAircraft
below current count.
This commit is contained in:
Reese Norris
2026-07-17 15:54:39 -04:00
parent 400facfacb
commit a397d6ef6d
3 changed files with 137 additions and 30 deletions

View File

@@ -2,6 +2,7 @@ package sweatbox
import (
"fmt"
"math"
"strconv"
"strings"
)
@@ -129,6 +130,10 @@ func (e *Engine) cmdXPDRLocked(target, mode string) CommandResult {
return CommandResult{OK: false, Message: errMsg}
}
ac.XPDRMode = mode
// Standby and ident are contradictory on the wire; clear flash on ss.
if mode == XPDRModeStandby {
ac.Ident = false
}
return CommandResult{OK: true}
}
@@ -177,19 +182,9 @@ func (e *Engine) cmdAddLocked(args []string) CommandResult {
switch rules {
case RulesVFR, RulesIFR, RulesDVFR, RulesSVFR:
// ok — also accept lowercase already uppercased
// ok (args already upper-cased)
default:
// accept single-letter v/i/d/s
if len(rules) == 1 {
switch rules {
case "V", "I", "D", "S":
// ok
default:
return CommandResult{OK: false, Message: addUsage}
}
} else {
return CommandResult{OK: false, Message: addUsage}
}
return CommandResult{OK: false, Message: addUsage}
}
switch weight {
case WeightSmall, WeightSmallP, WeightLarge, WeightHeavy:
@@ -231,14 +226,10 @@ func (e *Engine) cmdAddLocked(args []string) CommandResult {
if len(args) < 6 {
return CommandResult{OK: false, Message: addUsage}
}
bearingStr := loc[1:]
bearing, err := strconv.ParseFloat(bearingStr, 64)
if err != nil {
return CommandResult{OK: false, Message: addUsage}
}
distNM, err1 := strconv.ParseFloat(args[4], 64)
alt, err2 := strconv.ParseFloat(args[5], 64)
if err1 != nil || err2 != nil || distNM < 0 {
bearing, ok1 := parseFiniteFloat(loc[1:])
distNM, ok2 := parseFiniteFloat(args[4])
alt, ok3 := parseFiniteFloat(args[5])
if !ok1 || !ok2 || !ok3 || distNM < 0 {
return CommandResult{OK: false, Message: addUsage}
}
typeTok, errMsg := optionalType(args[6:])
@@ -254,8 +245,8 @@ func (e *Engine) cmdAddLocked(args []string) CommandResult {
return CommandResult{OK: false, Message: addUsage}
}
rwy := strings.ToUpper(loc)
distNM, err := strconv.ParseFloat(args[4], 64)
if err != nil || distNM < 0 {
distNM, ok := parseFiniteFloat(args[4])
if !ok || distNM < 0 {
return CommandResult{OK: false, Message: addUsage}
}
typeTok, errMsg := optionalType(args[5:])
@@ -286,6 +277,15 @@ func (e *Engine) cmdAddLocked(args []string) CommandResult {
return CommandResult{OK: true, Added: []AircraftSnapshot{ac.snapshot()}}
}
// parseFiniteFloat parses a float that must be finite (rejects NaN/±Inf).
func parseFiniteFloat(s string) (float64, bool) {
v, err := strconv.ParseFloat(strings.TrimSpace(s), 64)
if err != nil || math.IsNaN(v) || math.IsInf(v, 0) {
return 0, false
}
return v, true
}
func optionalType(args []string) (string, string) {
if len(args) == 0 {
return "", ""
@@ -367,6 +367,11 @@ func (e *Engine) placeOnApproachLocked(ac *SimAircraft, rwy string, distNM float
if !ok {
return "Runway/taxiway not found in airport file."
}
// Record the resolved end designator (combined "33/15" → RwyA).
landEnd := rwy
if rwy == s.Name || rwy == s.RwyA+"/"+s.RwyB {
landEnd = s.RwyA
}
// Place along final: opposite of landing heading from threshold.
finalBearing := normalizeHeading(hdg + 180)
lat, lon := destinationPoint(thr.Lat, thr.Lon, finalBearing, distNM*metersPerNM)
@@ -376,7 +381,7 @@ func (e *Engine) placeOnApproachLocked(ac *SimAircraft, rwy string, distNM float
ac.Heading = hdg
ac.Speed = defaultApproachSpeed(ac.Engine)
ac.Status = StatusOnApproach
ac.LandingRunway = rwy
ac.Instruction = "Approach runway " + rwy
ac.LandingRunway = landEnd
ac.Instruction = "Approach runway " + landEnd
return ""
}

View File

@@ -460,13 +460,18 @@ func initialBearingDeg(lat1, lon1, lat2, lon2 float64) float64 {
}
// runwayThreshold returns the threshold point and landing heading for a runway
// end designator (e.g. "19" or "33"). Landing heading is along the runway
// centerline from the threshold toward the far end.
// end designator (e.g. "19" or "33"). Combined names ("19/1") default to RwyA.
// Landing heading is along the runway centerline from the threshold toward the
// far end.
func runwayThreshold(s *Surface, end string) (threshold Point, hdg float64, ok bool) {
if s == nil || s.Kind != SurfaceRunway || len(s.Points) < 2 {
return Point{}, 0, false
}
end = strings.ToUpper(strings.TrimSpace(end))
// Combined "A/B" (or full surface Name) → approach end A by default.
if end == s.Name || end == s.RwyA+"/"+s.RwyB {
end = s.RwyA
}
// Points are ordered RwyA → RwyB in .apt files.
var thr, far Point
var dispFt float64

View File

@@ -430,6 +430,14 @@ func TestCommand_AddValidation(t *testing.T) {
{"add i l j -270 10", false, "Missing parameters"}, // bearing needs alt
{"add i l j 33 abc", false, "Missing parameters"},
{"add i l j 33 10 EXTRA JUNK", false, "Missing parameters"},
// Non-finite geometry args (Issue 1)
{"add i l j 33 NaN", false, "Missing parameters"},
{"add i l j 33 +Inf", false, "Missing parameters"},
{"add i l j 33 -Inf", false, "Missing parameters"},
{"add v s p -270 10 NaN", false, "Missing parameters"},
{"add v s p -NaN 10 2500", false, "Missing parameters"},
{"add v s p -270 Inf 2500", false, "Missing parameters"},
{"add v s p -+Inf 10 2500", false, "Missing parameters"},
}
for _, tc := range cases {
r := e.CommandLine(tc.line)
@@ -469,6 +477,90 @@ func TestCommand_AddMaxAircraft(t *testing.T) {
}
}
// Issue 5: SetMaxAircraft reduced below current count keeps existing AC
// and rejects further adds until under the new cap.
func TestSetMaxAircraft_BelowCurrentCount(t *testing.T) {
e := loadKBTVEngine(t)
var css []string
for i := 0; i < 3; i++ {
r := e.CommandLine("add v s p @GA1")
if !r.OK {
t.Fatalf("add %d: %s", i, r.Message)
}
css = append(css, r.Added[0].Callsign)
}
if e.Count() != 3 {
t.Fatalf("count = %d", e.Count())
}
e.SetMaxAircraft(1)
if e.Settings().MaxAircraft != 1 {
t.Fatalf("max = %d", e.Settings().MaxAircraft)
}
if e.Count() != 3 {
t.Fatalf("existing aircraft must be kept: count=%d", e.Count())
}
r := e.CommandLine("add v s p @GA2")
if r.OK || !contains(r.Message, "Maximum") {
t.Fatalf("add while over cap: %+v", r)
}
// Free slots until under cap, then add succeeds.
if !e.Delete(css[0]) || !e.Delete(css[1]) {
t.Fatal("delete")
}
if e.Count() != 1 {
t.Fatalf("count after del = %d", e.Count())
}
r = e.CommandLine("add v s p @GA2")
if r.OK {
t.Fatal("still at cap (1); further add should fail")
}
e.Delete(css[2])
r = e.CommandLine("add v s p @GA2")
if !r.OK {
t.Fatalf("add under cap: %s", r.Message)
}
if e.Count() != 1 {
t.Fatalf("count = %d", e.Count())
}
}
func (e *Engine) mustGet(t *testing.T, cs string) AircraftSnapshot {
t.Helper()
ac, ok := e.Get(cs)
if !ok {
t.Fatalf("missing %s", cs)
}
return ac
}
func TestCommand_AddCombinedRunwayName(t *testing.T) {
// Issue 3: combined designator "33/15" should place on RwyA approach.
e := loadKBTVEngine(t)
r := e.CommandLine("add i l j 33/15 8")
if !r.OK {
t.Fatalf("add combined: %s", r.Message)
}
ac := r.Added[0]
if ac.Status != StatusOnApproach {
t.Errorf("status = %s", ac.Status)
}
if ac.LandingRunway != "33" {
t.Errorf("LandingRunway = %s, want 33 (RwyA)", ac.LandingRunway)
}
// Same geometry as end designator "33".
r2 := e.CommandLine("add i l j 33 8")
if !r2.OK {
t.Fatal(r2.Message)
}
ac2 := r2.Added[0]
if math.Abs(ac.Lat-ac2.Lat) > 1e-6 || math.Abs(ac.Lon-ac2.Lon) > 1e-6 {
t.Errorf("combined vs end mismatch: (%v,%v) vs (%v,%v)", ac.Lat, ac.Lon, ac2.Lat, ac2.Lon)
}
if math.Abs(ac.Heading-ac2.Heading) > 1e-6 {
t.Errorf("hdg %v vs %v", ac.Heading, ac2.Heading)
}
}
func TestCommand_DelPosSqId(t *testing.T) {
e := loadKBTVEngine(t)
r := e.CommandLine("add v s p @GA1")
@@ -511,6 +603,7 @@ func TestCommand_DelPosSqId(t *testing.T) {
t.Errorf("sqi: %+v", ac)
}
// ss must clear Ident after id/sqi (Issue 2).
r = e.Command(cs, "ss")
if !r.OK {
t.Fatal(r.Message)
@@ -519,6 +612,9 @@ func TestCommand_DelPosSqId(t *testing.T) {
if ac.XPDRMode != XPDRModeStandby {
t.Errorf("mode = %s", ac.XPDRMode)
}
if ac.Ident {
t.Error("ss should clear Ident")
}
r = e.Command(cs, "sn")
if !r.OK {
t.Fatal(r.Message)
@@ -528,10 +624,6 @@ func TestCommand_DelPosSqId(t *testing.T) {
t.Errorf("mode = %s", ac.XPDRMode)
}
// Clear ident then set via id.
e.mu.Lock()
e.aircraft[cs].Ident = false
e.mu.Unlock()
r = e.Command(cs, "id")
if !r.OK {
t.Fatal(r.Message)
@@ -540,6 +632,11 @@ func TestCommand_DelPosSqId(t *testing.T) {
if !ac.Ident {
t.Error("ident not set")
}
// ss after id clears ident again
r = e.Command(cs, "ss")
if !r.OK || e.mustGet(t, cs).Ident {
t.Fatalf("ss after id: ok=%v ident=%v", r.OK, e.mustGet(t, cs).Ident)
}
// Bad squawk.
r = e.Command(cs, "sq abcd")