Skip to content

Commit 0151eb4

Browse files
highbytehighbyte
authored
Feature/refactor systemlist (#135)
* Remove unnecessary caching of ISystem objects in SystemList class. * Remove unnecessary caching in SystemList. Remove unnecessary extra creation of system object in SystemList. Change ISystemConfigurer method signatures to more appropriate match which type of config object is expected. * Change some state managment in HostApp to not create new System objects unnecessary.
1 parent b639b5d commit 0151eb4

12 files changed

Lines changed: 130 additions & 163 deletions

File tree

‎src/apps/Highbyte.DotNet6502.App.SadConsole/MenuConsole.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ private void DrawUIItems()
5454
selectSystemComboBox.SelectedItemChanged += async (s, e) =>
5555
{
5656
await _sadConsoleHostApp.SelectSystem(selectSystemComboBox.SelectedItem.ToString());
57-
await _sadConsoleHostApp.SelectSystemConfigurationVariant(_sadConsoleHostApp.AllSelectedSystemConfigurationVariants.First());
5857

5958
var selectSystemVariantComboBox = Controls["selectSystemVariantComboBox"] as ComboBox;
6059
selectSystemVariantComboBox.SetItems(_sadConsoleHostApp.AllSelectedSystemConfigurationVariants.ToArray());

‎src/apps/Highbyte.DotNet6502.App.SadConsole/SystemSetup/C64Setup.cs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public class C64Setup : ISystemConfigurer<SadConsoleRenderContext, SadConsoleInp
1818
{
1919
public string SystemName => C64.SystemName;
2020

21-
public Task<List<string>> GetConfigurationVariants(IHostSystemConfig hostSystemConfig) => Task.FromResult(s_systemVariants);
21+
public Task<List<string>> GetConfigurationVariants(ISystemConfig systemConfig) => Task.FromResult(s_systemVariants);
2222
public List<string> ConfigurationVariants => s_systemVariants;
2323

2424
private static readonly List<string> s_systemVariants = C64ModelInventory.C64Models.Keys.ToList();
@@ -55,18 +55,18 @@ public Task PersistHostSystemConfig(IHostSystemConfig hostSystemConfig)
5555
return Task.CompletedTask;
5656
}
5757

58-
public Task<ISystem> BuildSystem(string configurationVariant, IHostSystemConfig hostSystemConfig)
58+
public Task<ISystem> BuildSystem(string configurationVariant, ISystemConfig systemConfig)
5959
{
60-
var c64HostSystemConfig = (C64HostConfig)hostSystemConfig;
60+
var c64SystemConfig = (C64SystemConfig)systemConfig;
6161
var c64Config = new C64Config
6262
{
6363
C64Model = configurationVariant,
6464
Vic2Model = C64ModelInventory.C64Models[configurationVariant].Vic2Models.First().Name, // NTSC, NTSC_old, PAL
65-
AudioEnabled = c64HostSystemConfig.SystemConfig.AudioEnabled,
66-
KeyboardJoystickEnabled = c64HostSystemConfig.SystemConfig.KeyboardJoystickEnabled,
67-
KeyboardJoystick = c64HostSystemConfig.SystemConfig.KeyboardJoystick,
68-
ROMs = c64HostSystemConfig.SystemConfig.ROMs,
69-
ROMDirectory = c64HostSystemConfig.SystemConfig.ROMDirectory,
65+
AudioEnabled = c64SystemConfig.AudioEnabled,
66+
KeyboardJoystickEnabled = c64SystemConfig.KeyboardJoystickEnabled,
67+
KeyboardJoystick = c64SystemConfig.KeyboardJoystick,
68+
ROMs = c64SystemConfig.ROMs,
69+
ROMDirectory = c64SystemConfig.ROMDirectory,
7070
};
7171

7272
var c64 = C64.BuildC64(c64Config, _loggerFactory);

‎src/apps/Highbyte.DotNet6502.App.SadConsole/SystemSetup/GenericComputerSetup.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ public class GenericComputerSetup : ISystemConfigurer<SadConsoleRenderContext, S
1414
{
1515
public string SystemName => GenericComputer.SystemName;
1616

17-
public Task<List<string>> GetConfigurationVariants(IHostSystemConfig hostSystemConfig)
17+
public Task<List<string>> GetConfigurationVariants(ISystemConfig systemConfig)
1818
{
19-
var examplePrograms = ((GenericComputerHostConfig)hostSystemConfig).SystemConfig.ExamplePrograms.Keys.OrderByDescending(x => x).ToList();
19+
var examplePrograms = ((GenericComputerSystemConfig)systemConfig).ExamplePrograms.Keys.OrderByDescending(x => x).ToList();
2020
return Task.FromResult(examplePrograms);
2121
}
2222

@@ -42,10 +42,10 @@ public Task PersistHostSystemConfig(IHostSystemConfig hostSystemConfig)
4242
return Task.CompletedTask;
4343
}
4444

45-
public Task<ISystem> BuildSystem(string configurationVariant, IHostSystemConfig hostSystemConfig)
45+
public Task<ISystem> BuildSystem(string configurationVariant, ISystemConfig systemConfig)
4646
{
47-
var genericComputerHostConfig = (GenericComputerHostConfig)hostSystemConfig;
48-
var exampleProgramPath = genericComputerHostConfig.SystemConfig.ExamplePrograms[configurationVariant];
47+
var genericComputerSystemConfig = (GenericComputerSystemConfig)systemConfig;
48+
var exampleProgramPath = genericComputerSystemConfig.ExamplePrograms[configurationVariant];
4949
var genericComputerConfig = GenericComputerExampleConfigs.GetExampleConfig(configurationVariant, exampleProgramPath);
5050

5151
return Task.FromResult<ISystem>(

‎src/apps/Highbyte.DotNet6502.App.SilkNetNative/SystemSetup/C64HostConfig.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
using Highbyte.DotNet6502.Impl.SilkNet.Commodore64.Video;
44
using Highbyte.DotNet6502.Systems;
55
using Highbyte.DotNet6502.Systems.Commodore64.Config;
6-
using Highbyte.DotNet6502.Systems.Generic.Config;
76

87
namespace Highbyte.DotNet6502.App.SilkNetNative.SystemSetup;
98

‎src/apps/Highbyte.DotNet6502.App.SilkNetNative/SystemSetup/C64Setup.cs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ namespace Highbyte.DotNet6502.App.SilkNetNative.SystemSetup;
1818
public class C64Setup : ISystemConfigurer<SilkNetRenderContextContainer, SilkNetInputHandlerContext, NAudioAudioHandlerContext>
1919
{
2020
public string SystemName => C64.SystemName;
21-
public Task<List<string>> GetConfigurationVariants(IHostSystemConfig hostSystemConfig) => Task.FromResult(s_systemVariants);
21+
public Task<List<string>> GetConfigurationVariants(ISystemConfig systemConfig) => Task.FromResult(s_systemVariants);
2222

2323
private static readonly List<string> s_systemVariants = C64ModelInventory.C64Models.Keys.ToList();
2424

@@ -43,18 +43,18 @@ public Task PersistHostSystemConfig(IHostSystemConfig hostSystemConfig)
4343
// TODO: Should user settings be persisted? If so method GetNewHostSystemConfig() also needs to be updated to read from there instead of appsettings.json.
4444
return Task.CompletedTask;
4545
}
46-
public Task<ISystem> BuildSystem(string configurationVariant, IHostSystemConfig hostSystemConfig)
46+
public Task<ISystem> BuildSystem(string configurationVariant, ISystemConfig systemConfig)
4747
{
48-
var c64HostSystemConfig = (C64HostConfig)hostSystemConfig;
48+
var c64SystemConfig = (C64SystemConfig)systemConfig;
4949
var c64Config = new C64Config
5050
{
5151
C64Model = configurationVariant,
5252
Vic2Model = C64ModelInventory.C64Models[configurationVariant].Vic2Models.First().Name, // NTSC, NTSC_old, PAL
53-
AudioEnabled = c64HostSystemConfig.SystemConfig.AudioEnabled,
54-
KeyboardJoystickEnabled = c64HostSystemConfig.SystemConfig.KeyboardJoystickEnabled,
55-
KeyboardJoystick = c64HostSystemConfig.SystemConfig.KeyboardJoystick,
56-
ROMs = c64HostSystemConfig.SystemConfig.ROMs,
57-
ROMDirectory = c64HostSystemConfig.SystemConfig.ROMDirectory,
53+
AudioEnabled = c64SystemConfig.AudioEnabled,
54+
KeyboardJoystickEnabled = c64SystemConfig.KeyboardJoystickEnabled,
55+
KeyboardJoystick = c64SystemConfig.KeyboardJoystick,
56+
ROMs = c64SystemConfig.ROMs,
57+
ROMDirectory = c64SystemConfig.ROMDirectory,
5858
};
5959

6060
var c64 = C64.BuildC64(c64Config, _loggerFactory);

‎src/apps/Highbyte.DotNet6502.App.SilkNetNative/SystemSetup/GenericComputerSetup.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ public class GenericComputerSetup : ISystemConfigurer<SilkNetRenderContextContai
1414
{
1515
public string SystemName => GenericComputer.SystemName;
1616

17-
public Task<List<string>> GetConfigurationVariants(IHostSystemConfig hostSystemConfig)
17+
public Task<List<string>> GetConfigurationVariants(ISystemConfig systemConfig)
1818
{
19-
var examplePrograms = ((GenericComputerHostConfig)hostSystemConfig).SystemConfig.ExamplePrograms.Keys.OrderByDescending(x => x).ToList();
19+
var examplePrograms = ((GenericComputerSystemConfig)systemConfig).ExamplePrograms.Keys.OrderByDescending(x => x).ToList();
2020
return Task.FromResult(examplePrograms);
2121
}
2222

@@ -42,10 +42,10 @@ public Task PersistHostSystemConfig(IHostSystemConfig hostSystemConfig)
4242
return Task.CompletedTask;
4343
}
4444

45-
public Task<ISystem> BuildSystem(string configurationVariant, IHostSystemConfig hostSystemConfig)
45+
public Task<ISystem> BuildSystem(string configurationVariant, ISystemConfig systemConfig)
4646
{
47-
var genericComputerHostConfig = (GenericComputerHostConfig)hostSystemConfig;
48-
var exampleProgramPath = genericComputerHostConfig.SystemConfig.ExamplePrograms[configurationVariant];
47+
var genericComputerSystemConfig = (GenericComputerSystemConfig)systemConfig;
48+
var exampleProgramPath = genericComputerSystemConfig.ExamplePrograms[configurationVariant];
4949
var genericComputerConfig = GenericComputerExampleConfigs.GetExampleConfig(configurationVariant, exampleProgramPath);
5050

5151
return Task.FromResult<ISystem>(

‎src/apps/Highbyte.DotNet6502.App.WASM/Emulator/SystemSetup/C64Setup.cs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ namespace Highbyte.DotNet6502.App.WASM.Emulator.SystemSetup;
2121
public class C64Setup : ISystemConfigurer<SkiaRenderContext, AspNetInputHandlerContext, WASMAudioHandlerContext>
2222
{
2323
public string SystemName => C64.SystemName;
24-
public Task<List<string>> GetConfigurationVariants(IHostSystemConfig hostSystemConfig) => Task.FromResult(s_systemVariants);
24+
public Task<List<string>> GetConfigurationVariants(ISystemConfig systemConfig) => Task.FromResult(s_systemVariants);
2525

2626
private static readonly List<string> s_systemVariants = C64ModelInventory.C64Models.Keys.ToList();
2727

@@ -90,18 +90,18 @@ public async Task PersistHostSystemConfig(IHostSystemConfig hostSystemConfig)
9090
await _browserContext.LocalStorage.SetItemAsStringAsync($"{C64HostConfig.ConfigSectionName}", JsonSerializer.Serialize(c64HostConfig));
9191
}
9292

93-
public Task<ISystem> BuildSystem(string configurationVariant, IHostSystemConfig hostSystemConfig)
93+
public Task<ISystem> BuildSystem(string configurationVariant, ISystemConfig systemConfig)
9494
{
95-
var c64HostSystemConfig = (C64HostConfig)hostSystemConfig;
95+
var c64SystemConfig = (C64SystemConfig)systemConfig;
9696
var c64Config = new C64Config
9797
{
9898
C64Model = configurationVariant,
9999
Vic2Model = C64ModelInventory.C64Models[configurationVariant].Vic2Models.First().Name, // NTSC, NTSC_old, PAL
100-
AudioEnabled = c64HostSystemConfig.SystemConfig.AudioEnabled,
101-
KeyboardJoystickEnabled = c64HostSystemConfig.SystemConfig.KeyboardJoystickEnabled,
102-
KeyboardJoystick = c64HostSystemConfig.SystemConfig.KeyboardJoystick,
103-
ROMs = c64HostSystemConfig.SystemConfig.ROMs,
104-
ROMDirectory = c64HostSystemConfig.SystemConfig.ROMDirectory,
100+
AudioEnabled = c64SystemConfig.AudioEnabled,
101+
KeyboardJoystickEnabled = c64SystemConfig.KeyboardJoystickEnabled,
102+
KeyboardJoystick = c64SystemConfig.KeyboardJoystick,
103+
ROMs = c64SystemConfig.ROMs,
104+
ROMDirectory = c64SystemConfig.ROMDirectory,
105105
};
106106

107107
var c64 = C64.BuildC64(c64Config, _loggerFactory);

‎src/apps/Highbyte.DotNet6502.App.WASM/Emulator/SystemSetup/GenericComputerSetup.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ public class GenericComputerSetup : ISystemConfigurer<SkiaRenderContext, AspNetI
1313
{
1414
public string SystemName => GenericComputer.SystemName;
1515

16-
public Task<List<string>> GetConfigurationVariants(IHostSystemConfig hostSystemConfig)
16+
public Task<List<string>> GetConfigurationVariants(ISystemConfig systemConfig)
1717
{
18-
var examplePrograms = ((GenericComputerHostConfig)hostSystemConfig).SystemConfig.ExamplePrograms.Keys.OrderByDescending(x => x).ToList();
18+
var examplePrograms = ((GenericComputerSystemConfig)systemConfig).ExamplePrograms.Keys.OrderByDescending(x => x).ToList();
1919
return Task.FromResult(examplePrograms);
2020
}
2121

@@ -71,10 +71,10 @@ public async Task PersistHostSystemConfig(IHostSystemConfig hostSystemConfig)
7171
await _browserContext.LocalStorage.SetItemAsStringAsync($"{GenericComputerHostConfig.ConfigSectionName}", JsonSerializer.Serialize(cenericComputerHostConfig));
7272
}
7373

74-
public async Task<ISystem> BuildSystem(string configurationVariant, IHostSystemConfig hostSystemConfig)
74+
public async Task<ISystem> BuildSystem(string configurationVariant, ISystemConfig systemConfig)
7575
{
76-
var genericComputerHostConfig = (GenericComputerHostConfig)hostSystemConfig;
77-
var exampleProgramPath = genericComputerHostConfig.SystemConfig.ExamplePrograms[configurationVariant];
76+
var genericComputerSystemConfig = (GenericComputerSystemConfig)systemConfig;
77+
var exampleProgramPath = genericComputerSystemConfig.ExamplePrograms[configurationVariant];
7878
var exampleProgramBytes = await _browserContext.HttpClient.GetByteArrayAsync(exampleProgramPath);
7979
var genericComputerConfig = GenericComputerExampleConfigs.GetExampleConfig(configurationVariant, exampleProgramBytes);
8080

‎src/libraries/Highbyte.DotNet6502.Systems/HostApp.cs‎

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,16 @@ public class HostApp<TRenderContext, TInputHandlerContext, TAudioHandlerContext>
2727
// Other variables
2828
private string _selectedSystemName;
2929
public string SelectedSystemName => _selectedSystemName;
30+
private ISystem? _selectedSystemTemporary; // A temporary storage of the selected system if asked for, and system has not been started yet.
31+
3032
public HashSet<string> AvailableSystemNames => _systemList.Systems;
3133

3234
private string _selectedSystemConfigurationVariant;
3335
public string SelectedSystemConfigurationVariant => _selectedSystemConfigurationVariant;
3436
private List<string> _allSelectedSystemConfigurationVariants = new();
3537
public List<string> AllSelectedSystemConfigurationVariants => _allSelectedSystemConfigurationVariants;
3638

39+
3740
private SystemRunner? _systemRunner = null;
3841
public SystemRunner? CurrentSystemRunner => _systemRunner;
3942
public ISystem? CurrentRunningSystem => _systemRunner?.System;
@@ -141,6 +144,9 @@ public async Task SelectSystem(string systemName)
141144
_selectedSystemConfigurationVariant = _allSelectedSystemConfigurationVariants.First();
142145
}
143146

147+
// Make sure any state regarding the system variant is also in sync
148+
await SelectSystemConfigurationVariant(_selectedSystemConfigurationVariant);
149+
144150
OnAfterSelectSystem();
145151
}
146152

@@ -154,7 +160,16 @@ public async Task SelectSystemConfigurationVariant(string configurationVariant)
154160

155161
_selectedSystemConfigurationVariant = configurationVariant;
156162

157-
return;
163+
// Pre-create a temporary variable to contain the system if it is valid.
164+
// This is useful if the system has not been started yet, but client requests the system object.
165+
if (CurrentHostSystemConfig.IsValid(out _))
166+
{
167+
_selectedSystemTemporary = await _systemList.BuildSystem(_selectedSystemName, _selectedSystemConfigurationVariant);
168+
}
169+
else
170+
{
171+
_selectedSystemTemporary = null;
172+
}
158173
}
159174

160175
public virtual void OnAfterSelectSystem() { }
@@ -169,18 +184,34 @@ public async Task Start()
169184
if (EmulatorState == EmulatorState.Running)
170185
throw new DotNet6502Exception("Cannot start emulator if emulator is running.");
171186

172-
if (!await _systemList.IsValidConfig(_selectedSystemName, _selectedSystemConfigurationVariant))
187+
if (!await _systemList.IsValidConfig(_selectedSystemName))
173188
throw new DotNet6502Exception("Cannot start emulator if current system config is invalid.");
174189

175-
var systemAboutToBeStarted = await _systemList.GetSystem(_selectedSystemName, _selectedSystemConfigurationVariant);
176-
bool shouldStart = OnBeforeStart(systemAboutToBeStarted);
190+
ISystem systemToBeStarted;
191+
if (EmulatorState == EmulatorState.Uninitialized)
192+
{
193+
if (_selectedSystemTemporary == null)
194+
{
195+
// If we don't have a temporary system, build it.
196+
_selectedSystemTemporary = await _systemList.BuildSystem(_selectedSystemName, _selectedSystemConfigurationVariant);
197+
}
198+
systemToBeStarted = _selectedSystemTemporary;
199+
}
200+
else
201+
{
202+
// We already have a system runner, so use the system from that.
203+
systemToBeStarted = _systemRunner!.System;
204+
}
205+
206+
bool shouldStart = OnBeforeStart(systemToBeStarted);
177207
if (!shouldStart)
178208
return;
179209

180210
var emulatorStateBeforeStart = EmulatorState;
211+
181212
// Only create a new instance of SystemRunner if we previously has not started (so resume after pause works).
182213
if (EmulatorState == EmulatorState.Uninitialized)
183-
_systemRunner = await _systemList.BuildSystemRunner(_selectedSystemName, _selectedSystemConfigurationVariant);
214+
_systemRunner = await _systemList.BuildSystemRunner(systemToBeStarted);
184215

185216
InitInstrumentation(_systemRunner!.System);
186217

@@ -225,10 +256,7 @@ public void Stop()
225256
_systemRunner = default!;
226257

227258
EmulatorState = EmulatorState.Uninitialized;
228-
229-
// TODO: Why is this necessary if cache is invalidated when config is updated?
230-
// Make sure the cached System instance is removed, so it's created again next time (starting fresh).
231-
_systemList.InvalidateSystemCache(SelectedSystemName, _selectedSystemConfigurationVariant);
259+
_selectedSystemTemporary = null; // Clear the temporary system, as it is no longer valid.
232260

233261
OnAfterStop();
234262

@@ -311,30 +339,38 @@ public virtual void OnAfterDrawFrame(bool emulatorRendered) { }
311339

312340
public async Task<bool> IsSystemConfigValid()
313341
{
314-
return await _systemList.IsValidConfig(_selectedSystemName, _selectedSystemConfigurationVariant);
342+
return await _systemList.IsValidConfig(_selectedSystemName);
315343
}
316344
public async Task<(bool, List<string> validationErrors)> IsValidConfigWithDetails()
317345
{
318-
return await _systemList.IsValidConfigWithDetails(_selectedSystemName, _selectedSystemConfigurationVariant);
346+
return await _systemList.IsValidConfigWithDetails(_selectedSystemName);
319347
}
320348

321349
public async Task<bool> IsAudioSupported()
322350
{
323-
return await _systemList.IsAudioSupported(_selectedSystemName, _selectedSystemConfigurationVariant);
351+
return await _systemList.IsAudioSupported(_selectedSystemName);
324352
}
325353

326354
public async Task<bool> IsAudioEnabled()
327355
{
328-
return await _systemList.IsAudioEnabled(_selectedSystemName, _selectedSystemConfigurationVariant);
356+
return await _systemList.IsAudioEnabled(_selectedSystemName);
329357
}
330358
public async Task SetAudioEnabled(bool enabled)
331359
{
332-
await _systemList.SetAudioEnabled(_selectedSystemName, enabled: enabled, _selectedSystemConfigurationVariant);
360+
await _systemList.SetAudioEnabled(_selectedSystemName, enabled: enabled);
333361
}
334362

335363
public async Task<ISystem> GetSelectedSystem()
336364
{
337-
return await _systemList.GetSystem(_selectedSystemName, _selectedSystemConfigurationVariant);
365+
if (EmulatorState == EmulatorState.Uninitialized)
366+
{
367+
// If we havent started started the selected system yet, return a temporary instance of the system (set in SelectSystem method).
368+
if (_selectedSystemTemporary == null)
369+
throw new DotNet6502Exception("Internal state error.");
370+
return _selectedSystemTemporary;
371+
}
372+
// The emulator is running, return the current system runner's system.
373+
return _systemRunner!.System;
338374
}
339375

340376
public void UpdateHostSystemConfig(IHostSystemConfig newConfig)

‎src/libraries/Highbyte.DotNet6502.Systems/ISystemConfigurer.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ namespace Highbyte.DotNet6502.Systems;
33
public interface ISystemConfigurer<TRenderContext, TInputHandlerContext, TAudioHandlerContext>
44
{
55
public string SystemName { get; }
6-
public Task<List<string>> GetConfigurationVariants(IHostSystemConfig hostSystemConfig);
6+
public Task<List<string>> GetConfigurationVariants(ISystemConfig systemConfig);
7+
public Task<ISystem> BuildSystem(string configurationVariant, ISystemConfig systemConfig);
78
public Task<IHostSystemConfig> GetNewHostSystemConfig();
89
public Task PersistHostSystemConfig(IHostSystemConfig hostSystemConfig);
9-
public Task<ISystem> BuildSystem(string configurationVariant, IHostSystemConfig hostSystemConfig);
1010
public Task<SystemRunner> BuildSystemRunner(
1111
ISystem system,
1212
IHostSystemConfig hostSystemConfig,

0 commit comments

Comments
 (0)