From 1026503d9241203987dc695a01e4907dcaaffb1f Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Sat, 21 May 2022 04:39:44 +1000 Subject: [PATCH] Refactor firmware config so icons make sense (resolves #3157) --- .../config/FirmwaresConfig.cs | 85 ++++++++----------- .../config/FirmwaresConfigInfo.cs | 16 +--- .../Database/FirmwareOptionStatus.cs | 22 +++-- 3 files changed, 52 insertions(+), 71 deletions(-) diff --git a/src/BizHawk.Client.EmuHawk/config/FirmwaresConfig.cs b/src/BizHawk.Client.EmuHawk/config/FirmwaresConfig.cs index d7e1ea0320..1b01001f38 100644 --- a/src/BizHawk.Client.EmuHawk/config/FirmwaresConfig.cs +++ b/src/BizHawk.Client.EmuHawk/config/FirmwaresConfig.cs @@ -2,7 +2,6 @@ using System.Collections; using System.Collections.Generic; using System.ComponentModel; -using System.Diagnostics; using System.Drawing; using System.IO; using System.Linq; @@ -10,7 +9,6 @@ using System.Windows.Forms; using BizHawk.Common; using BizHawk.Client.Common; -using BizHawk.Client.EmuHawk.Properties; using BizHawk.Common.CollectionExtensions; using BizHawk.Emulation.Common; @@ -28,6 +26,28 @@ namespace BizHawk.Client.EmuHawk { public partial class FirmwaresConfig : Form, IDialogParent { + private const string STATUS_DESC_UNUSED = ""; + + private static readonly IReadOnlyDictionary StatusDescs = new Dictionary + { + [FirmwareOptionStatus.Unset] = STATUS_DESC_UNUSED, + [FirmwareOptionStatus.Bad] = "BAD! Why are you using this file", + [FirmwareOptionStatus.Unknown] = STATUS_DESC_UNUSED, + [FirmwareOptionStatus.Unacceptable] = "NO: This doesn't work on the core", + [FirmwareOptionStatus.Acceptable] = "OK: This works on the core", + [FirmwareOptionStatus.Ideal] = "COOL: Ideal for TASing and anything. There can only be one.", + }; + + internal static readonly IReadOnlyDictionary StatusIcons = new Dictionary + { + [FirmwareOptionStatus.Unset] = Properties.Resources.ExclamationRed, + [FirmwareOptionStatus.Bad] = Properties.Resources.ThumbsDown, // in this main view, bad dumps use this thumbs down (to differentiate from unset); in a record's info view, they use unset's red '!' (to differentiate from unacceptable) + [FirmwareOptionStatus.Unknown] = Properties.Resources.RetroQuestion, + [FirmwareOptionStatus.Unacceptable] = Properties.Resources.ThumbsDown, + [FirmwareOptionStatus.Acceptable] = Properties.Resources.GreenCheck, + [FirmwareOptionStatus.Ideal] = Properties.Resources.Freeze, + }; + private readonly IDictionary _firmwareUserSpecifications; private readonly PathEntryCollection _pathEntries; @@ -71,11 +91,6 @@ namespace BizHawk.Client.EmuHawk public string TargetSystem { get; set; } - private const int IdUnsure = 0; - private const int IdMissing = 1; - private const int IdOk = 2; - private const int IdBad = 3; - private Font _fixedFont, _boldFont, _boldFixedFont; private class ListViewSorter : IComparer @@ -120,17 +135,10 @@ namespace BizHawk.Client.EmuHawk = tbbImport.Image = tbbClose.Image = tbbCloseReload.Image - = tbbOpenFolder.Image = Resources.Placeholder; + = tbbOpenFolder.Image = Properties.Resources.Placeholder; // prep ImageList for ListView - // the order matters, so make sure these match IdUnsure, IdMissing, etc. - imageList1.Images.AddRange(new Image[] - { - Resources.RetroQuestion, - Resources.ExclamationRed, - Resources.GreenCheck, - Resources.ThumbsDown, - }); + foreach (var kvp in StatusIcons.OrderBy(static kvp => kvp.Key)) imageList1.Images.Add(kvp.Value); _listViewSorter = new ListViewSorter(-1); @@ -178,7 +186,7 @@ namespace BizHawk.Client.EmuHawk { Tag = fr, UseItemStyleForSubItems = false, - ImageIndex = IdUnsure, + ImageIndex = (int) FirmwareOptionStatus.Unknown, ToolTipText = null }; lvi.SubItems.Add(sysID); @@ -287,7 +295,7 @@ namespace BizHawk.Client.EmuHawk if (ri == null) { - lvi.ImageIndex = IdMissing; + lvi.ImageIndex = (int) FirmwareOptionStatus.Unset; lvi.ToolTipText = "No file bound for this firmware!"; } else @@ -304,20 +312,17 @@ namespace BizHawk.Client.EmuHawk var hash = ri.KnownFirmwareFile?.Hash; if (hash == null) { - lvi.ImageIndex = IdUnsure; + lvi.ImageIndex = (int) FirmwareOptionStatus.Unknown; lvi.ToolTipText = "You've bound a custom choice here. Hope you know what you're doing."; lvi.SubItems[4].Text = "-custom-"; } - else if (FirmwareDatabase.FirmwareOptions.FirstOrNull(fo => fo.Hash == hash)?.IsAcceptableOrIdeal == false) - { - lvi.ImageIndex = IdBad; - lvi.ToolTipText = "Bad! This file has been bound to a choice which is known to be bad (details in right-click > Info)"; - lvi.SubItems[4].Text = ri.KnownFirmwareFile.Value.Description; - } else { - lvi.ImageIndex = IdOk; - lvi.ToolTipText = "Good! This file has been bound to some kind of a decent choice"; + var fo = FirmwareDatabase.FirmwareOptions.FirstOrNull(fo => fo.Hash == hash); + lvi.ImageIndex = (int) (fo?.Status ?? FirmwareOptionStatus.Unset); // null here means it's an option for a different record, so use the red '!' like unset + lvi.ToolTipText = fo?.IsAcceptableOrIdeal == true + ? "Good! This file has been bound to some kind of a decent choice" + : "Bad! This file has been bound to a choice which is known to be bad (details in right-click > Info)"; lvi.SubItems[4].Text = ri.KnownFirmwareFile.Value.Description; } @@ -336,14 +341,14 @@ namespace BizHawk.Client.EmuHawk // if the user specified a file but its missing, mark it as such if (ri.Missing) { - lvi.ImageIndex = IdMissing; + lvi.ImageIndex = (int) FirmwareOptionStatus.Unset; lvi.ToolTipText = "The file that's specified is missing!"; } // if the user specified a known firmware file but its for some other firmware, it was probably a mistake. mark it as suspicious if (ri.KnownMismatching) { - lvi.ImageIndex = IdUnsure; + lvi.ImageIndex = (int) FirmwareOptionStatus.Unknown; lvi.ToolTipText = "You've manually specified a firmware file, and we're sure it's wrong. Hope you know what you're doing."; } @@ -543,26 +548,8 @@ namespace BizHawk.Client.EmuHawk olvi.SubItems.Add(new ListViewItem.ListViewSubItem()); olvi.SubItems.Add(new ListViewItem.ListViewSubItem()); var ff = FirmwareDatabase.FirmwareFilesByHash[o.Hash]; - if (o.Status == FirmwareOptionStatus.Ideal) - { - olvi.ImageIndex = FirmwaresConfigInfo.idIdeal; - olvi.ToolTipText = FirmwaresConfigInfo.ttIdeal; - } - if (o.Status == FirmwareOptionStatus.Acceptable) - { - olvi.ImageIndex = FirmwaresConfigInfo.idAcceptable; - olvi.ToolTipText = FirmwaresConfigInfo.ttAcceptable; - } - if (o.Status == FirmwareOptionStatus.Unacceptable) - { - olvi.ImageIndex = FirmwaresConfigInfo.idUnacceptable; - olvi.ToolTipText = FirmwaresConfigInfo.ttUnacceptable; - } - if (o.Status == FirmwareOptionStatus.Bad) - { - olvi.ImageIndex = FirmwaresConfigInfo.idBad; - olvi.ToolTipText = FirmwaresConfigInfo.ttBad; - } + olvi.ImageIndex = (int) (o.Status is FirmwareOptionStatus.Bad ? FirmwareOptionStatus.Unset : o.Status); // if bad, use unset's red '!' to differentiate from unacceptable + olvi.ToolTipText = StatusDescs[o.Status]; olvi.SubItems[0].Text = ff.Size.ToString(); olvi.SubItems[0].Font = Font; // why doesn't this work? olvi.SubItems[1].Text = $"sha1:{o.Hash}"; diff --git a/src/BizHawk.Client.EmuHawk/config/FirmwaresConfigInfo.cs b/src/BizHawk.Client.EmuHawk/config/FirmwaresConfigInfo.cs index 948cfea96d..12708df332 100644 --- a/src/BizHawk.Client.EmuHawk/config/FirmwaresConfigInfo.cs +++ b/src/BizHawk.Client.EmuHawk/config/FirmwaresConfigInfo.cs @@ -1,5 +1,5 @@ using System; -using System.Drawing; +using System.Linq; using System.Windows.Forms; // todo - display details on the current resolution status @@ -9,22 +9,12 @@ namespace BizHawk.Client.EmuHawk { public partial class FirmwaresConfigInfo : Form { - public const int idIdeal = 0; - public const int idAcceptable = 1; - public const int idUnacceptable = 2; - public const int idBad = 3; - - public const string ttIdeal = "COOL: Ideal for TASing and anything. There can only be one."; - public const string ttAcceptable = "OK: This works on the core"; - public const string ttUnacceptable = "NO: This doesn't work on the core"; - public const string ttBad = "BAD! Why are you using this file"; - public FirmwaresConfigInfo() { InitializeComponent(); - // prep imagelist for listview with 4 item states for (ideal, acceptable, unacceptable, bad) - imageList1.Images.AddRange(new Image[] { Properties.Resources.GreenCheck, Properties.Resources.Freeze, Properties.Resources.ThumbsDown, Properties.Resources.ExclamationRed }); + // prep imagelist for listview + foreach (var kvp in FirmwaresConfig.StatusIcons.OrderBy(static kvp => kvp.Key)) imageList1.Images.Add(kvp.Value); } private void LvOptions_KeyDown(object sender, KeyEventArgs e) diff --git a/src/BizHawk.Emulation.Common/Database/FirmwareOptionStatus.cs b/src/BizHawk.Emulation.Common/Database/FirmwareOptionStatus.cs index 5df9d1f62d..d54562de3c 100644 --- a/src/BizHawk.Emulation.Common/Database/FirmwareOptionStatus.cs +++ b/src/BizHawk.Emulation.Common/Database/FirmwareOptionStatus.cs @@ -2,16 +2,20 @@ namespace BizHawk.Emulation.Common { public enum FirmwareOptionStatus { - /// Preferred to get checkmarks, and for TASing - Ideal, - - /// Works with our core, but not preferred for TASing - Acceptable, - - /// A good file, but it doesn't work with our core - Unacceptable, + Unset = 0, /// Nonlegitimate files that are notable enough to be worth detecting, even if mainly to categorize as a BAD option - Bad + Bad = 1, + + Unknown = 2, + + /// A good file, but it doesn't work with our core + Unacceptable = 3, + + /// Works with our core, but not preferred for TASing + Acceptable = 4, + + /// Preferred to get checkmarks, and for TASing + Ideal = 5, } }