From 7ed001c4c1b2a90632e758865fb722b97b934562 Mon Sep 17 00:00:00 2001 From: JetSetIlly Date: Tue, 16 Apr 2024 22:37:16 +0100 Subject: [PATCH] changed when cartridgeloader is created for log/video regression creating the cartridgeloader later in the regression process gives a clearer indication of when and if the cartridgeloader is closed properly --- cartridgeloader/naming.go | 10 ++++++++-- debugger/debugger.go | 5 ++--- gopher2600.go | 18 ++++-------------- recorder/fileformat.go | 1 - regression/log.go | 31 ++++++++++++++++++------------- regression/video.go | 27 +++++++++++++++++---------- 6 files changed, 49 insertions(+), 43 deletions(-) diff --git a/cartridgeloader/naming.go b/cartridgeloader/naming.go index 74b446b2..ca554ddf 100644 --- a/cartridgeloader/naming.go +++ b/cartridgeloader/naming.go @@ -32,8 +32,14 @@ func decideOnName(ld Loader) string { return "" } - name := filepath.Base(ld.Filename) - name = strings.TrimSuffix(name, filepath.Ext(ld.Filename)) + return NameFromFilename(ld.Filename) +} +// NameFromFilename converts a filename to a shortened version suitable for +// display. Useful in some contexts where creating a cartridge loader instance +// is inconvenient. +func NameFromFilename(filename string) string { + name := filepath.Base(filename) + name = strings.TrimSuffix(name, filepath.Ext(filename)) return name } diff --git a/debugger/debugger.go b/debugger/debugger.go index 08aad27b..c6286b7f 100644 --- a/debugger/debugger.go +++ b/debugger/debugger.go @@ -1337,14 +1337,13 @@ func (dbg *Debugger) startComparison(comparisonROM string, comparisonPrefs strin return err } - // cartload is passed to comparision.CreateFromLoader(). closure will be - // handled from there cartload, err := cartridgeloader.NewLoaderFromFilename(comparisonROM, "AUTO") if err != nil { return err } - // comparision emulation handles closure of cartridgeloader + // cartload is passed to comparision.CreateFromLoader(). closure will be + // handled from there when comparision emulation ends dbg.comparison.CreateFromLoader(cartload) // check use of comparison prefs diff --git a/gopher2600.go b/gopher2600.go index a7808fc1..d5edfdf4 100644 --- a/gopher2600.go +++ b/gopher2600.go @@ -785,19 +785,14 @@ with the LOG mode. Note that asking for log output will suppress regression prog switch strings.ToUpper(regressMode) { case "VIDEO": - cartload, err := cartridgeloader.NewLoaderFromFilename(args[0], mapping) - if err != nil { - return err - } - defer cartload.Close() - statetype, err := regression.NewStateType(state) if err != nil { return err } regressor = ®ression.VideoRegression{ - CartLoad: cartload, + Cartridge: args[0], + Mapping: mapping, TVtype: strings.ToUpper(spec), NumFrames: numFrames, State: statetype, @@ -811,14 +806,9 @@ with the LOG mode. Note that asking for log output will suppress regression prog Notes: notes, } case "LOG": - cartload, err := cartridgeloader.NewLoaderFromFilename(args[0], mapping) - if err != nil { - return err - } - defer cartload.Close() - regressor = ®ression.LogRegression{ - CartLoad: cartload, + Cartridge: args[0], + Mapping: mapping, TVtype: strings.ToUpper(spec), NumFrames: numFrames, Notes: notes, diff --git a/recorder/fileformat.go b/recorder/fileformat.go index 7ff2c701..cb24f61e 100644 --- a/recorder/fileformat.go +++ b/recorder/fileformat.go @@ -111,7 +111,6 @@ func (plb *Playback) readHeader(lines []string, checkROM bool) error { var err error - // read header plb.CartLoad, err = cartridgeloader.NewLoaderFromFilename(lines[lineCartName], "AUTO") if err != nil { return fmt.Errorf("playback: %w", err) diff --git a/regression/log.go b/regression/log.go index 2057d81d..af00cb39 100644 --- a/regression/log.go +++ b/regression/log.go @@ -48,7 +48,8 @@ const ( // the run. Regression passes if the subsequent runs produce the same // log/digest. type LogRegression struct { - CartLoad cartridgeloader.Loader + Cartridge string + Mapping string TVtype string NumFrames int Notes string @@ -66,18 +67,14 @@ func deserialiseLogEntry(fields database.SerialisedEntry) (database.Entry, error return nil, fmt.Errorf("log: too few fields") } - var err error - - // string fields need no conversion - reg.CartLoad, err = cartridgeloader.NewLoaderFromFilename(fields[videoFieldCartName], fields[videoFieldCartMapping]) - if err != nil { - return nil, fmt.Errorf("log: %w", err) - } + reg.Cartridge = fields[videoFieldCartName] + reg.Mapping = fields[videoFieldCartMapping] reg.TVtype = fields[logFieldTVtype] reg.digest = fields[logFieldDigest] reg.Notes = fields[logFieldNotes] - // convert number of frames field + var err error + reg.NumFrames, err = strconv.Atoi(fields[logFieldNumFrames]) if err != nil { msg := fmt.Sprintf("invalid numFrames field [%s]", fields[logFieldNumFrames]) @@ -95,8 +92,8 @@ func (reg LogRegression) EntryType() string { // Serialise implements the database.Entry interface. func (reg *LogRegression) Serialise() (database.SerialisedEntry, error) { return database.SerialisedEntry{ - reg.CartLoad.Filename, - reg.CartLoad.Mapping, + reg.Cartridge, + reg.Mapping, reg.TVtype, strconv.Itoa(reg.NumFrames), reg.digest, @@ -114,7 +111,9 @@ func (reg LogRegression) CleanUp() error { func (reg LogRegression) String() string { s := strings.Builder{} - s.WriteString(fmt.Sprintf("[%s] %s [%s] frames=%d", reg.EntryType(), reg.CartLoad.Name, reg.TVtype, reg.NumFrames)) + s.WriteString(fmt.Sprintf("[%s] %s [%s] frames=%d", reg.EntryType(), + cartridgeloader.NameFromFilename(reg.Cartridge), + reg.TVtype, reg.NumFrames)) if reg.Notes != "" { s.WriteString(fmt.Sprintf(" [%s]", reg.Notes)) } @@ -146,7 +145,13 @@ func (reg *LogRegression) regress(newRegression bool, output io.Writer, msg stri // default the hardware preferences vcs.Env.Normalise() - err = setup.AttachCartridge(vcs, reg.CartLoad, true) + cartload, err := cartridgeloader.NewLoaderFromFilename(reg.Cartridge, reg.Mapping) + if err != nil { + return false, "", fmt.Errorf("log: %w", err) + } + defer cartload.Close() + + err = setup.AttachCartridge(vcs, cartload, true) if err != nil { return false, "", fmt.Errorf("log: %w", err) } diff --git a/regression/video.go b/regression/video.go index 8d5fc4c8..2ab00ca3 100644 --- a/regression/video.go +++ b/regression/video.go @@ -52,7 +52,8 @@ const ( // emulation for N frames and the video recorded at that point. Regression // passes if subsequenct runs produce the same video value. type VideoRegression struct { - CartLoad cartridgeloader.Loader + Cartridge string + Mapping string TVtype string NumFrames int State StateType @@ -76,10 +77,8 @@ func deserialiseVideoEntry(fields database.SerialisedEntry) (database.Entry, err var err error // string fields need no conversion - reg.CartLoad, err = cartridgeloader.NewLoaderFromFilename(fields[videoFieldCartName], fields[videoFieldCartMapping]) - if err != nil { - return nil, fmt.Errorf("video: %w", err) - } + reg.Cartridge = fields[videoFieldCartName] + reg.Mapping = fields[videoFieldCartMapping] reg.TVtype = fields[videoFieldTVtype] reg.digest = fields[videoFieldDigest] reg.Notes = fields[videoFieldNotes] @@ -128,8 +127,8 @@ func (reg VideoRegression) EntryType() string { // Serialise implements the database.Entry interface. func (reg *VideoRegression) Serialise() (database.SerialisedEntry, error) { return database.SerialisedEntry{ - reg.CartLoad.Filename, - reg.CartLoad.Mapping, + reg.Cartridge, + reg.Mapping, reg.TVtype, strconv.Itoa(reg.NumFrames), reg.State.String(), @@ -170,7 +169,9 @@ func (reg VideoRegression) String() string { state = " [with state]" } - s.WriteString(fmt.Sprintf("[%s] %s [%s] frames=%d%s", reg.EntryType(), reg.CartLoad.Name, reg.TVtype, reg.NumFrames, state)) + s.WriteString(fmt.Sprintf("[%s] %s [%s] frames=%d%s", reg.EntryType(), + cartridgeloader.NameFromFilename(reg.Cartridge), + reg.TVtype, reg.NumFrames, state)) if reg.Notes != "" { s.WriteString(fmt.Sprintf(" [%s]", reg.Notes)) } @@ -204,7 +205,13 @@ func (reg *VideoRegression) regress(newRegression bool, output io.Writer, msg st // default the hardware preferences vcs.Env.Normalise() - err = setup.AttachCartridge(vcs, reg.CartLoad, true) + cartload, err := cartridgeloader.NewLoaderFromFilename(reg.Cartridge, reg.Mapping) + if err != nil { + return false, "", fmt.Errorf("log: %w", err) + } + defer cartload.Close() + + err = setup.AttachCartridge(vcs, cartload, true) if err != nil { return false, "", fmt.Errorf("video: %w", err) } @@ -275,7 +282,7 @@ func (reg *VideoRegression) regress(newRegression bool, output io.Writer, msg st if reg.State != StateNone { // create a unique filename - reg.stateFile, err = uniqueFilename("state", reg.CartLoad.Name) + reg.stateFile, err = uniqueFilename("state", reg.Cartridge) if err != nil { return false, "", fmt.Errorf("video: %w", err) }