diff --git a/src/GridOS/Core/Configuration/MainConfig.cs b/src/GridOS/Core/Configuration/BaseConfig.cs similarity index 97% rename from src/GridOS/Core/Configuration/MainConfig.cs rename to src/GridOS/Core/Configuration/BaseConfig.cs index f7e802c..ba195d1 100644 --- a/src/GridOS/Core/Configuration/MainConfig.cs +++ b/src/GridOS/Core/Configuration/BaseConfig.cs @@ -4,7 +4,7 @@ namespace IngameScript { // TODO: Finalize setting change notification implementation. Decide what way to go with configuration; separate static from per display instance. Try to move defaults to some plain DTO. - class MainConfig : IMenuPresentationConfig, IBreadcrumbConfig, IDisplayConfig + class BaseConfig : IMenuPresentationConfig, IBreadcrumbConfig, IDisplayConfig { public string PathSeparator { get; set; } = "›"; diff --git a/src/GridOS/Core/DisplaySystem/DisplayControls/Breadcrumb.cs b/src/GridOS/Core/DisplaySystem/DisplayControls/Breadcrumb.cs index 4738f65..3eb8269 100644 --- a/src/GridOS/Core/DisplaySystem/DisplayControls/Breadcrumb.cs +++ b/src/GridOS/Core/DisplaySystem/DisplayControls/Breadcrumb.cs @@ -26,6 +26,9 @@ public Breadcrumb(IBreadcrumbConfig config) WidthUnit = SizeUnit.Percent; } + public override void Dispose() + {} + public override StringBuilder GetContent(ContentGenerationHelper _, bool FlushCache = false) { if (FlushCache) diff --git a/src/GridOS/Core/DisplaySystem/DisplayControls/Control.cs b/src/GridOS/Core/DisplaySystem/DisplayControls/Control.cs index 4ab25ad..adca708 100644 --- a/src/GridOS/Core/DisplaySystem/DisplayControls/Control.cs +++ b/src/GridOS/Core/DisplaySystem/DisplayControls/Control.cs @@ -34,7 +34,7 @@ abstract class Control : IControl public SizeUnit BorderUnit { get; set; } public Thickness Border { get; set; } - public Color BorderColor { get; set; } = Color.Transparent; + public Color? BorderColor { get; set; } public string FontName { get; set; } = null; public float FontSize { get; set; } = 1f; @@ -51,5 +51,7 @@ protected virtual void OnRedrawRequired() { RedrawRequired?.Invoke(this); } + + public abstract void Dispose(); } } diff --git a/src/GridOS/Core/DisplaySystem/DisplayControls/DisplayHeader.cs b/src/GridOS/Core/DisplaySystem/DisplayControls/DisplayHeader.cs index 81e4c55..97d8301 100644 --- a/src/GridOS/Core/DisplaySystem/DisplayControls/DisplayHeader.cs +++ b/src/GridOS/Core/DisplaySystem/DisplayControls/DisplayHeader.cs @@ -7,12 +7,9 @@ class DisplayHeader : Control { protected readonly StringBuilder _buffer = new StringBuilder(); protected readonly ProgressIndicator2 _spinner = new ProgressIndicator2(); - protected readonly IDiagnosticService _diagnostics; - public DisplayHeader(IDiagnosticService diagnostics) + public DisplayHeader() { - _diagnostics = diagnostics; - FontSize = 0.6f; WidthUnit = SizeUnit.Percent; Width = 100; @@ -22,6 +19,9 @@ public DisplayHeader(IDiagnosticService diagnostics) BackgroundColor = new Color(255, 255, 255, 90); } + public override void Dispose() + {} + public override StringBuilder GetContent(ContentGenerationHelper _, bool FlushCache = false) { _buffer.Clear(); diff --git a/src/GridOS/Core/DisplaySystem/DisplayControls/IControl.cs b/src/GridOS/Core/DisplaySystem/DisplayControls/IControl.cs index 1444ec7..ada97be 100644 --- a/src/GridOS/Core/DisplaySystem/DisplayControls/IControl.cs +++ b/src/GridOS/Core/DisplaySystem/DisplayControls/IControl.cs @@ -3,7 +3,7 @@ namespace IngameScript { - interface IControl + interface IControl : IDisposable { bool Visible { get; set; } @@ -29,7 +29,7 @@ interface IControl Thickness Border { get; set; } SizeUnit BorderUnit { get; set; } - Color BorderColor { get; set; } + Color? BorderColor { get; set; } string FontName { get; set; } float FontSize { get; set; } diff --git a/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/DisplayController.cs b/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/DisplayController.cs index 2c02e1d..02ccc91 100644 --- a/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/DisplayController.cs +++ b/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/DisplayController.cs @@ -13,7 +13,6 @@ class DisplayController : IDisposable, IMenuInstanceServices public IMenuPresentationConfig MenuConfig { get; } private readonly ICommandDispatcher _commandDispatcher; - private readonly IDiagnosticService _diagnostics; private readonly IView _view; private readonly IGlobalEvents _globalEvents; private readonly Breadcrumb _breadcrumb; @@ -23,39 +22,54 @@ class DisplayController : IDisposable, IMenuInstanceServices private readonly CommandItem _downCommand; private readonly CommandItem _selectCommand; - public DisplayController(string name, ICommandDispatcher commandDispatcher, MainConfig config, IDiagnosticService diagnostics, IView view, IMenuGroup menuRoot, IGlobalEvents globalEvents) + public DisplayController(string name, ICommandDispatcher commandDispatcher, BaseConfig config, IMenuGroup menuRoot, IGlobalEvents globalEvents, IMyTextSurface surface) { Name = name; MenuConfig = config; DisplayConfig = config; - _view = view; - _diagnostics = diagnostics; - _commandDispatcher = commandDispatcher; _globalEvents = globalEvents; + _commandDispatcher = commandDispatcher; - _globalEvents.ExecutionWillFinish += DisplayTick; - - _upCommand = new CommandItem($"{Name}Up", OnMoveUp); - _downCommand = new CommandItem($"{Name}Down", OnMoveDown); - _selectCommand = new CommandItem($"{Name}Select", OnSelect); - - _menu = new Menu( - new MenuModel(menuRoot, this), - config); - _breadcrumb = new Breadcrumb(config); - _menu.NavigationPathChanged += _breadcrumb.OnPathChanged; - - _view - .AddControl(new DisplayHeader(_diagnostics)) - .AddControl(_breadcrumb) - .AddControl(_menu); - - _commandDispatcher - .AddCommand_OverwriteExisting(_upCommand) - .AddCommand_OverwriteExisting(_downCommand) - .AddCommand_OverwriteExisting(_selectCommand); - - _menu.PushUpdate(); + try + { + _view = new DisplayView( + surface, + config, + new TextSurfaceWordWrapper() + ); + + _globalEvents.ExecutionWillFinish += DisplayTick; + + _upCommand = new CommandItem($"{Name}Up", OnMoveUp); + _downCommand = new CommandItem($"{Name}Down", OnMoveDown); + _selectCommand = new CommandItem($"{Name}Select", OnSelect); + + _menu = new Menu( + new MenuModel(menuRoot, this), + config); + _breadcrumb = new Breadcrumb(config); + _menu.NavigationPathChanged += _breadcrumb.OnPathChanged; + + _view + .AddControl(new DisplayHeader()) + .AddControl(_breadcrumb) + .AddControl(_menu); + + _commandDispatcher + .AddCommand_OverwriteExisting(_upCommand) + .AddCommand_OverwriteExisting(_downCommand) + .AddCommand_OverwriteExisting(_selectCommand); + + _menu.PushUpdate(); + } + catch (Exception) + { + _view?.Dispose(); + _view = null; + _menu?.Dispose(); + _menu = null; + throw; + } } public void SetFontType(string fontName) @@ -73,9 +87,11 @@ public void SetBackgroundColor(Color color) public void Dispose() { _globalEvents.ExecutionWillFinish -= DisplayTick; + _menu.NavigationPathChanged -= _breadcrumb.OnPathChanged; _commandDispatcher.RemoveCommand(_upCommand); _commandDispatcher.RemoveCommand(_downCommand); _commandDispatcher.RemoveCommand(_selectCommand); + _view.Dispose(); } private void OnMoveUp(CommandItem _, string __) diff --git a/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/DisplayView.cs b/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/DisplayView.cs index 09a48db..a122949 100644 --- a/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/DisplayView.cs +++ b/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/DisplayView.cs @@ -10,18 +10,17 @@ namespace IngameScript /// class DisplayView : IView { - protected IMyTextSurface _surface; - protected bool _contentDirty = true; protected RectangleF _viewport; protected readonly List>> _content = new List>>(); protected readonly IWordWrapperController _wordWrapper; - protected readonly MainConfig _config; + protected readonly IMyTextSurface _surface; + protected readonly BaseConfig _config; private readonly StringBuilder _buffer = new StringBuilder(); - public DisplayView(IMyTextSurface surface, MainConfig config, IWordWrapperController wordWrapper) + public DisplayView(IMyTextSurface surface, BaseConfig config, IWordWrapperController wordWrapper) { _surface = surface; _config = config; @@ -31,6 +30,12 @@ public DisplayView(IMyTextSurface surface, MainConfig config, IWordWrapperContro AdaptToSurface(); } + public void Dispose() + { + ClearControls(); + DecommissionSurface(); + } + public DisplayView AddControl(IControl control) { if (_content.Any(x => x.Key == control)) @@ -47,14 +52,18 @@ public void RemoveControl(IControl control) if (controlIndex > -1) { control.RedrawRequired -= OnRedrawRequired; + control.Dispose(); _content.RemoveAt(controlIndex); } } public void ClearControls() { - foreach (var kvp in _content) - kvp.Key.RedrawRequired -= OnRedrawRequired; + foreach (var control in _content.Select(x => x.Key)) + { + control.RedrawRequired -= OnRedrawRequired; + control.Dispose(); + } _content.Clear(); } @@ -98,7 +107,7 @@ public void Redraw(bool flush = false) if (controlEntry.Key.Visible) { // TODO: Add dirty flag to controls, and if not dirty, use cached. - verticalWritingPosition = GenerateSpritesForControl(controlEntry.Key, controlEntry.Value, verticalWritingPosition); + verticalWritingPosition = DrawControl(controlEntry.Key, controlEntry.Value, verticalWritingPosition); frame.AddRange(controlEntry.Value); } } @@ -111,7 +120,7 @@ public void Redraw(bool flush = false) /// Generates sprites for the specified control in the specified list. /// /// Returns the resulting vertical writing position after the control is rendered. - private float GenerateSpritesForControl(IControl control, List targetList, float verticalWritingPosition) + private float DrawControl(IControl control, List targetList, float verticalWritingPosition) { targetList.Clear(); @@ -248,6 +257,12 @@ private void SetupSurface() _surface.PreserveAspectRatio = true; } + private void DecommissionSurface() + { + _surface.ContentType = ContentType.TEXT_AND_IMAGE; + _surface.WriteText(string.Empty); + } + private void OnRedrawRequired(IControl control) { _contentDirty = true; diff --git a/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/IView.cs b/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/IView.cs index a9462d4..5be07ca 100644 --- a/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/IView.cs +++ b/src/GridOS/Core/DisplaySystem/DisplayInfrastructure/IView.cs @@ -1,6 +1,8 @@ -namespace IngameScript +using System; + +namespace IngameScript { - interface IView + interface IView : IDisposable { DisplayView AddControl(IControl control); void RemoveControl(IControl control); diff --git a/src/GridOS/Core/DisplaySystem/DisplayOrchestrator.cs b/src/GridOS/Core/DisplaySystem/DisplayOrchestrator.cs index 2268002..45a5511 100644 --- a/src/GridOS/Core/DisplaySystem/DisplayOrchestrator.cs +++ b/src/GridOS/Core/DisplaySystem/DisplayOrchestrator.cs @@ -9,69 +9,62 @@ namespace IngameScript /// class DisplayOrchestrator { - // TODO: Now that the menu system is dynamic... need to think about whether we need to dispose stuff when an item is removed. - // What to do if non-empty item is removed? Remove all children, or put them somewhere else? How to react if we're displaying items below a group node that is removed? + private const string _controllerNameTemplate = "Lcd"; - // Root group displayed by default, parent of all other items/groups + // All menu content. private readonly IMenuGroup _menuRoot = new MenuGroup("Main"); - private readonly List _registeredTextSurfaces = new List(); - private readonly List _controllers = new List(); - private const string _controllerNameTemplate = "Lcd"; - private int _controllerCounter = 0; - + // All displays used by the system, along with the corresponding controller. + private readonly Dictionary _registeredSurfaces = new Dictionary(); + private readonly ICommandDispatcher _commandDispatcher; - private readonly IDiagnosticService _diagnostics; private readonly IGlobalEvents _globalEvents; + private readonly ILogger _logger; + private int _controllerCounter = 0; - public DisplayOrchestrator(ICommandDispatcher commandDispatcher, IDiagnosticService diagnostics, IGlobalEvents globalEvents) + public DisplayOrchestrator(ICommandDispatcher commandDispatcher, IGlobalEvents globalEvents, ILogger logger) { _commandDispatcher = commandDispatcher; - _diagnostics = diagnostics; _globalEvents = globalEvents; + _logger = logger; } public void RegisterTextSurface(IMyTextSurface textSurface) { - if (_registeredTextSurfaces.Contains(textSurface)) + if (_registeredSurfaces.ContainsKey(textSurface)) return; + DisplayController controller = null; + try { - var config = new MainConfig(); + var instanceConfig = new BaseConfig(); - _controllers.Add( - new DisplayController( + controller = new DisplayController( NextControllerName(), _commandDispatcher, - config, - _diagnostics, - new DisplayView( - textSurface, - config, - new TextSurfaceWordWrapper()), + instanceConfig, _menuRoot, - _globalEvents) - ); + _globalEvents, + textSurface); + + _registeredSurfaces.Add(textSurface, controller); } catch (Exception e) { - throw new Exception(e.Message + "\n" + e.StackTrace); + _logger.Log(LogLevel.Error, $"Display Orchestrator failed to add LCD '{textSurface.DisplayName}' to the system.\r\n\r\nMessage: {e.Message}\r\n\r\nStack trace: {e.StackTrace}"); + controller?.Dispose(); } - - _registeredTextSurfaces.Add(textSurface); } public void UnregisterTextSurface(IMyTextSurface textSurface) { - if (!_registeredTextSurfaces.Contains(textSurface)) - return; - - // TODO: Create proper removal infrastructure. CHECK REFERENCING to see if disposal is needed. - int sharedIndex = _registeredTextSurfaces.IndexOf(textSurface); - _controllers.Remove(_controllers[sharedIndex]); - - _registeredTextSurfaces.Remove(textSurface); + DisplayController controller; + if (_registeredSurfaces.TryGetValue(textSurface, out controller)) + { + controller.Dispose(); + _registeredSurfaces.Remove(textSurface); + } } public void RegisterMenuItem(IMenuItem item) @@ -83,10 +76,5 @@ private string NextControllerName() { return $"{_controllerNameTemplate}{++_controllerCounter}"; } - - public void ClearAll() - { - _registeredTextSurfaces.ForEach(x => x.WriteText("")); - } } } \ No newline at end of file diff --git a/src/GridOS/Core/DisplaySystem/MenuInfrastructure/Menu.cs b/src/GridOS/Core/DisplaySystem/MenuInfrastructure/Menu.cs index 34422f9..bc6b41d 100644 --- a/src/GridOS/Core/DisplaySystem/MenuInfrastructure/Menu.cs +++ b/src/GridOS/Core/DisplaySystem/MenuInfrastructure/Menu.cs @@ -5,13 +5,13 @@ namespace IngameScript { - /// s + /// /// Draws a scrollable menu, and exposes menu navigation commands. Very deliberately not layered into traditional full content creation + offsetting, for performance reasons. /// This implementation efficiently fills only the menu lines displayed in the viewport, skipping the processing of all other out-of-viewport elements. /// This class has multiple responsibilities, arguably violates SRP, but this degree of feature density allows for an optimized solution to a specific problem. /// Also, I included plenty of comments to aid maintenability, since the code here is less self-explanatory than normally. /// - class Menu : Control, IDisposable + class Menu : Control { public event Action> NavigationPathChanged; @@ -64,7 +64,7 @@ public Menu(IMenuModel model, IMenuPresentationConfig config, IWordWrapper wordW _lineGenerator = new MenuLineGenerator(config); } - public void Dispose() + public override void Dispose() { _model.CurrentViewChanged -= OnListChanged; _model.MenuItemChanged -= OnItemChanged; diff --git a/src/GridOS/Core/GridOS.cs b/src/GridOS/Core/GridOS.cs index a5fd642..11e6e70 100644 --- a/src/GridOS/Core/GridOS.cs +++ b/src/GridOS/Core/GridOS.cs @@ -31,7 +31,7 @@ public GridOS(MyGridProgram p) Action _updateFrequencySetter = (x) => _p.Runtime.UpdateFrequency = x; _commandDispatcher = new CommandDispatcher(_diagnostics); - _displayOrchestrator = new DisplayOrchestrator(_commandDispatcher, _diagnostics, _eventDispatcher); + _displayOrchestrator = new DisplayOrchestrator(_commandDispatcher, _eventDispatcher, _diagnostics); _updateDispatcher = new FastUpdateDispatcher((ILogger)_diagnostics, _updateFrequencyGetter, _updateFrequencySetter); // TODO: Remove 'help' menu. The only reason it's still here is that it's ideal for testing word wrapping and scrolling. @@ -133,7 +133,7 @@ public void RegisterTextSurface(IMyTextSurface textSurface) private void TryExecuteCustomData(string customData) { - if (customData == string.Empty) + if (string.IsNullOrWhiteSpace(customData)) return; foreach (var line in customData.Split(new[] { Environment.NewLine, "\r", "\n" }, StringSplitOptions.RemoveEmptyEntries)) @@ -157,9 +157,9 @@ private void CommandHandler_AddLcd(CommandItem sender, string param) surfaceIndex = Math.Max(0, int.Parse(param.Substring(delimiter + 1, param.Length - (delimiter + 1))) - 1); - } - param = param.Substring(0, delimiter); + param = param.Substring(0, delimiter); + } } } diff --git a/src/GridOS/GridOS.projitems b/src/GridOS/GridOS.projitems index e15711f..d978f62 100644 --- a/src/GridOS/GridOS.projitems +++ b/src/GridOS/GridOS.projitems @@ -32,7 +32,7 @@ - + diff --git a/tests/GridOS.UnitTests/Core/DisplaySystem/MenuInfrastructure/MenuTests.cs b/tests/GridOS.UnitTests/Core/DisplaySystem/MenuInfrastructure/MenuTests.cs index 1ef0b96..8a5c2f3 100644 --- a/tests/GridOS.UnitTests/Core/DisplaySystem/MenuInfrastructure/MenuTests.cs +++ b/tests/GridOS.UnitTests/Core/DisplaySystem/MenuInfrastructure/MenuTests.cs @@ -17,14 +17,14 @@ class MenuTests ContentGenerationHelper contentHelper; readonly Mock mockModel = new Mock(); readonly Mock mockWordWrapper = new Mock(); - MainConfig config; + BaseConfig config; readonly MenuItem firstMenuItem = new MenuItem("Item1"); readonly MenuItem seventhMenuItem = new MenuItem("item7"); [SetUp] public void SetUp() { - config = new MainConfig() { SelectionMarker = menuSelectionMarker, PaddingLeft = 0 }; + config = new BaseConfig() { SelectionMarker = menuSelectionMarker, PaddingLeft = 0 }; mockModel.Setup(x => x.CurrentView) .Returns(new List() {