clarified some preferences concepts

set defaults for revisions, rewind and ARM tabs of the prefs window (in
addition to the CRT tab). set defaults button labelled appropriately

fixes some race conditions caused by pre/post hooks on prefs values -
prefs values are generally goroutine safe, except when pre/post hook is
defined

debugger/preferences.go

@@ -35,7 +35,6 @@ func (p *Preferences) String() string {
 func newPreferences() (*Preferences, error) {
 	p := &Preferences{}
 
-	// save server using the prefs package
 	pth, err := resources.JoinPath(prefs.DefaultPrefsFile)
 	if err != nil {
 		return nil, err

disassembly/preferences.go

@@ -44,12 +44,18 @@ func (p *Preferences) String() string {
 func newPreferences(dsm *Disassembly) (*Preferences, error) {
 	p := &Preferences{dsm: dsm}
 
-	// set defaults
-	p.FxxxMirror.Set(true)
-	p.Symbols.Set(true)
-	p.mirrorOrigin = memorymap.OriginCartFxxxMirror
+	p.FxxxMirror.SetHookPost(func(v prefs.Value) error {
+		if v.(bool) {
+			p.mirrorOrigin = memorymap.OriginCartFxxxMirror
+		} else {
+			p.mirrorOrigin = memorymap.OriginCart
+		}
+		dsm.setCartMirror()
+		return nil
+	})
+
+	p.SetDefaults()
 
-	// save server using the prefs package
 	pth, err := resources.JoinPath(prefs.DefaultPrefsFile)
 	if err != nil {
 		return nil, err
@@ -69,16 +75,6 @@ func newPreferences(dsm *Disassembly) (*Preferences, error) {
 		return nil, err
 	}
 
-	p.FxxxMirror.SetHookPost(func(v prefs.Value) error {
-		if v.(bool) {
-			p.mirrorOrigin = memorymap.OriginCartFxxxMirror
-		} else {
-			p.mirrorOrigin = memorymap.OriginCart
-		}
-		dsm.setCartMirror()
-		return nil
-	})
-
 	err = p.dsk.Load(true)
 	if err != nil {
 		return nil, err
@@ -87,6 +83,13 @@ func newPreferences(dsm *Disassembly) (*Preferences, error) {
 	return p, nil
 }
 
+// SetDefaults reverts all settings to default values.
+func (p *Preferences) SetDefaults() {
+	p.FxxxMirror.Set(true)
+	p.Symbols.Set(true)
+	p.mirrorOrigin = memorymap.OriginCartFxxxMirror
+}
+
 // Load disassembly preferences and apply to the current disassembly.
 func (p *Preferences) Load() error {
 	return p.dsk.Load(false)

gui/crt/preferences.go

@@ -89,7 +89,6 @@ func NewPreferences() (*Preferences, error) {
 	p := &Preferences{}
 	p.SetDefaults()
 
-	// save server using the prefs package
 	pth, err := resources.JoinPath(prefs.DefaultPrefsFile)
 	if err != nil {
 		return nil, err

gui/sdlaudio/preferences.go

@@ -35,7 +35,6 @@ func NewPreferences() (*Preferences, error) {
 	p := &Preferences{}
 	p.SetDefaults()
 
-	// save server using the prefs package
 	pth, err := resources.JoinPath(prefs.DefaultPrefsFile)
 	if err != nil {
 		return nil, err

gui/sdlimgui/win_prefs.go

@@ -66,6 +66,7 @@ func (win *winPrefs) draw() {
 	}
 
 	var setDef setDefaultPrefs
+	var setDefLabel = ""
 
 	if win.img.isPlaymode() {
 		imgui.SetNextWindowPosV(imgui.Vec2{100, 40}, imgui.ConditionAppearing, imgui.Vec2{0, 0})
@@ -88,8 +89,10 @@ func (win *winPrefs) draw() {
 	}
 
 	if imgui.BeginTabItem("CRT") {
-		setDef = win.drawCRT()
+		win.drawCRT()
 		imgui.EndTabItem()
+		setDef = win.img.crtPrefs
+		setDefLabel = "CRT"
 	}
 
 	if imgui.BeginTabItem("Playmode") {
@@ -107,11 +110,15 @@ func (win *winPrefs) draw() {
 	if imgui.BeginTabItem("Rewind") {
 		win.drawRewind()
 		imgui.EndTabItem()
+		setDef = win.img.dbg.Rewind.Prefs
+		setDefLabel = "Rewind"
 	}
 
 	if imgui.BeginTabItem("ARM") {
 		win.drawARM()
 		imgui.EndTabItem()
+		setDef = win.img.vcs.Instance.Prefs.ARM
+		setDefLabel = "ARM"
 	}
 
 	if imgui.BeginTabItem("PlusROM") {
@@ -127,8 +134,10 @@ func (win *winPrefs) draw() {
 	// draw "Set Defaults" button
 	if setDef != nil {
 		imgui.SameLine()
-		if imgui.Button("Set Defaults") {
-			setDef.SetDefaults()
+		if imgui.Button(fmt.Sprintf("Set %s Defaults", setDefLabel)) {
+			// some preferences are sensitive to the goroutine SetDefaults() is
+			// called within
+			win.img.dbg.PushRawEvent(setDef.SetDefaults)
 		}
 	}
 
@@ -193,7 +202,9 @@ func (win *winPrefs) drawRewind() {
 
 	rewindMaxEntries := int32(win.img.dbg.Rewind.Prefs.MaxEntries.Get().(int))
 	if imgui.SliderIntV("Max Entries##maxentries", &rewindMaxEntries, 10, 500, fmt.Sprintf("%d", rewindMaxEntries), imgui.SliderFlagsNone) {
-		win.img.dbg.Rewind.Prefs.MaxEntries.Set(rewindMaxEntries)
+		win.img.dbg.PushRawEvent(func() {
+			win.img.dbg.Rewind.Prefs.MaxEntries.Set(rewindMaxEntries)
+		})
 	}
 
 	imgui.Spacing()
@@ -205,7 +216,9 @@ func (win *winPrefs) drawRewind() {
 
 	rewindFreq := int32(win.img.dbg.Rewind.Prefs.Freq.Get().(int))
 	if imgui.SliderIntV("Frequency##freq", &rewindFreq, 1, 5, fmt.Sprintf("%d", rewindFreq), imgui.SliderFlagsNone) {
-		win.img.dbg.Rewind.Prefs.Freq.Set(rewindFreq)
+		win.img.dbg.PushRawEvent(func() {
+			win.img.dbg.Rewind.Prefs.Freq.Set(rewindFreq)
+		})
 	}
 
 	imgui.Spacing()
@@ -350,7 +363,7 @@ func (win *winPrefs) drawPlusROM() {
 }
 
 func (win *winPrefs) drawDiskButtons() {
-	if imgui.Button("Save") {
+	if imgui.Button("Save All") {
 		win.img.dbg.PushRawEvent(func() {
 			err := win.img.prefs.save()
 			if err != nil {
@@ -395,7 +408,7 @@ func (win *winPrefs) drawDiskButtons() {
 	}
 
 	imgui.SameLine()
-	if imgui.Button("Restore") {
+	if imgui.Button("Restore All") {
 		win.img.dbg.PushRawEvent(func() {
 			err := win.img.prefs.load()
 			if err != nil {

gui/sdlimgui/win_prefs_crt.go

@@ -22,7 +22,7 @@ import (
 	"github.com/jetsetilly/gopher2600/hardware/television/specification"
 )
 
-func (win *winPrefs) drawCRT() setDefaultPrefs {
+func (win *winPrefs) drawCRT() {
 	imgui.Spacing()
 
 	// disable all CRT effect options if pixel-perfect is on
@@ -85,8 +85,6 @@ func (win *winPrefs) drawCRT() setDefaultPrefs {
 	imgui.PushItemWidth(-1)
 	win.drawSyncSpeed()
 	imgui.PopItemWidth()
-
-	return win.img.crtPrefs
 }
 
 func (win *winPrefs) drawCurve() {

gui/sdlimgui/win_prefs_tia_revisions.go

@@ -52,13 +52,17 @@ func (win *winPrefs) drawLateGRPx() {
 	imgui.Spacing()
 	a := win.img.vcs.Instance.Prefs.Revision.DskLateVDELGRP0.Get().(bool)
 	if imgui.Checkbox("GRP0", &a) {
-		win.img.vcs.Instance.Prefs.Revision.DskLateVDELGRP0.Set(a)
+		win.img.dbg.PushRawEvent(func() {
+			win.img.vcs.Instance.Prefs.Revision.DskLateVDELGRP0.Set(a)
+		})
 	}
 	win.drawTIARevTooltip(revision.LateVDELGRP0)
 
 	b := win.img.vcs.Instance.Prefs.Revision.DskLateVDELGRP1.Get().(bool)
 	if imgui.Checkbox("GRP1", &b) {
-		win.img.vcs.Instance.Prefs.Revision.DskLateVDELGRP1.Set(b)
+		win.img.dbg.PushRawEvent(func() {
+			win.img.vcs.Instance.Prefs.Revision.DskLateVDELGRP1.Set(b)
+		})
 	}
 	win.drawTIARevTooltip(revision.LateVDELGRP1)
 }
@@ -69,13 +73,17 @@ func (win *winPrefs) drawRESPxUnderHMOVE() {
 	imgui.Spacing()
 	a := win.img.vcs.Instance.Prefs.Revision.DskLateRESPx.Get().(bool)
 	if imgui.Checkbox("Late RESPx", &a) {
-		win.img.vcs.Instance.Prefs.Revision.DskLateRESPx.Set(a)
+		win.img.dbg.PushRawEvent(func() {
+			win.img.vcs.Instance.Prefs.Revision.DskLateRESPx.Set(a)
+		})
 	}
 	win.drawTIARevTooltip(revision.LateRESPx)
 
 	b := win.img.vcs.Instance.Prefs.Revision.DskEarlyScancounter.Get().(bool)
 	if imgui.Checkbox("Early Scancounter", &b) {
-		win.img.vcs.Instance.Prefs.Revision.DskEarlyScancounter.Set(b)
+		win.img.dbg.PushRawEvent(func() {
+			win.img.vcs.Instance.Prefs.Revision.DskEarlyScancounter.Set(b)
+		})
 	}
 	win.drawTIARevTooltip(revision.EarlyScancounter)
 }
@@ -86,13 +94,17 @@ func (win *winPrefs) drawLatePlayfield() {
 	imgui.Spacing()
 	a := win.img.vcs.Instance.Prefs.Revision.DskLatePFx.Get().(bool)
 	if imgui.Checkbox("PFx", &a) {
-		win.img.vcs.Instance.Prefs.Revision.DskLatePFx.Set(a)
+		win.img.dbg.PushRawEvent(func() {
+			win.img.vcs.Instance.Prefs.Revision.DskLatePFx.Set(a)
+		})
 	}
 	win.drawTIARevTooltip(revision.LatePFx)
 
 	b := win.img.vcs.Instance.Prefs.Revision.DskLateCOLUPF.Get().(bool)
 	if imgui.Checkbox("COLUPF", &b) {
-		win.img.vcs.Instance.Prefs.Revision.DskLateCOLUPF.Set(b)
+		win.img.dbg.PushRawEvent(func() {
+			win.img.vcs.Instance.Prefs.Revision.DskLateCOLUPF.Set(b)
+		})
 	}
 	win.drawTIARevTooltip(revision.LateCOLUPF)
 }
@@ -103,7 +115,9 @@ func (win *winPrefs) drawLostMOTCK() {
 	imgui.Spacing()
 	a := win.img.vcs.Instance.Prefs.Revision.DskLostMOTCK.Get().(bool)
 	if imgui.Checkbox("Players/Missiles/Ball", &a) {
-		win.img.vcs.Instance.Prefs.Revision.DskLostMOTCK.Set(a)
+		win.img.dbg.PushRawEvent(func() {
+			win.img.vcs.Instance.Prefs.Revision.DskLostMOTCK.Set(a)
+		})
 	}
 	win.drawTIARevTooltip(revision.LostMOTCK)
 }
@@ -114,7 +128,9 @@ func (win *winPrefs) drawLateRESPx() {
 	imgui.Spacing()
 	a := win.img.vcs.Instance.Prefs.Revision.DskRESPxHBLANK.Get().(bool)
 	if imgui.Checkbox("HBLANK threshold", &a) {
-		win.img.vcs.Instance.Prefs.Revision.DskRESPxHBLANK.Set(a)
+		win.img.dbg.PushRawEvent(func() {
+			win.img.vcs.Instance.Prefs.Revision.DskRESPxHBLANK.Set(a)
+		})
 	}
 	win.drawTIARevTooltip(revision.RESPxHBLANK)
 }

hardware/preferences/arm_preferences.go

@@ -55,7 +55,6 @@ func newARMprefrences() (*ARMPreferences, error) {
 	p := &ARMPreferences{}
 	p.SetDefaults()
 
-	// setup preferences and load from disk
 	pth, err := resources.JoinPath(prefs.DefaultPrefsFile)
 	if err != nil {
 		return nil, err

hardware/preferences/plusrom_preferences.go

@@ -38,7 +38,6 @@ func newPlusROMpreferences() (*PlusROMPreferences, error) {
 	p := &PlusROMPreferences{}
 	p.SetDefaults()
 
-	// save server using the prefs package
 	pth, err := resources.JoinPath(prefs.DefaultPrefsFile)
 	if err != nil {
 		return nil, err

hardware/preferences/preferences.go

@@ -53,7 +53,6 @@ func NewPreferences() (*Preferences, error) {
 	p := &Preferences{}
 	p.SetDefaults()
 
-	// setup preferences and load from disk
 	pth, err := resources.JoinPath(prefs.DefaultPrefsFile)
 	if err != nil {
 		return nil, err

hardware/preferences/revision_preferences.go

@@ -57,7 +57,6 @@ type RevisionPreferences struct {
 
 func newRevisionPreferences() (*RevisionPreferences, error) {
 	p := &RevisionPreferences{}
-	p.SetDefaults()
 
 	// register callbacks to update the "live" values from the disk value
 	p.DskLateVDELGRP0.SetHookPost(func(v prefs.Value) error {
@@ -93,7 +92,8 @@ func newRevisionPreferences() (*RevisionPreferences, error) {
 		return nil
 	})
 
-	// save server using the prefs package
+	p.SetDefaults()
+
 	pth, err := resources.JoinPath(prefs.DefaultPrefsFile)
 	if err != nil {
 		return nil, curated.Errorf("revision: %v", err)

hiscore/preferences.go

@@ -36,7 +36,6 @@ func (p *Preferences) String() string {
 func newPreferences() (*Preferences, error) {
 	p := &Preferences{}
 
-	// save server using the prefs package
 	pth, err := resources.JoinPath(prefs.DefaultPrefsFile)
 	if err != nil {
 		return nil, curated.Errorf("hiscore: %v", err)

prefs/doc.go

@@ -42,6 +42,12 @@
 // program in the normal way. Changes can be committed to disk wit the
 // Disk.Save() function and restoried with Disk.Load().
 //
+//Concurrency
+//
+// Generally, it is safe to access a prefs value from any goroutine. However,
+// Set() should be used with care *if* SetHookPost() or SetHookPre() has been
+// set for that value.
+//
 //Note
 //
 // While saved preference files are stored in UTF-8 it is not a good idea for

rewind/preferences.go

@@ -45,11 +45,8 @@ const snapshotFreq = 1
 // newPreferences is the preferred method of initialisation for the Preferences type.
 func newPreferences(r *Rewind) (*Preferences, error) {
 	p := &Preferences{r: r}
+	p.SetDefaults()
 
-	p.MaxEntries.Set(maxEntries)
-	p.Freq.Set(snapshotFreq)
-
-	// save server using the prefs package
 	pth, err := resources.JoinPath(prefs.DefaultPrefsFile)
 	if err != nil {
 		return nil, err
@@ -87,6 +84,12 @@ func newPreferences(r *Rewind) (*Preferences, error) {
 	return p, nil
 }
 
+// SetDefaults reverts all settings to default values.
+func (p *Preferences) SetDefaults() {
+	p.MaxEntries.Set(maxEntries)
+	p.Freq.Set(snapshotFreq)
+}
+
 // Load disassembly preferences and apply to the current disassembly.
 func (p *Preferences) Load() error {
 	return p.dsk.Load(false)