Move hashing to SCBlockAccessor, fix #3455

Fixes #3455 by adding bool for insular sea, and adjusting overall progress values as listed.
Closes #3456 (supersedes)

Updates SCBlockAccessor to eliminate bounds check (integer overflow, which isn't possible with our array size), adds some overloads, and xmldoc.

Co-Authored-By: Jonathan Herbert <3344332+foohyfooh@users.noreply.github.com>

PKHeX.Core/Saves/Access/SCBlockAccessor.cs

@@ -1,87 +1,92 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 
-namespace PKHeX.Core
+namespace PKHeX.Core;
+
+/// <summary>
+/// <see cref="SwishCrypto"/> block accessor, where blocks are ordered by ascending <see cref="SCBlock.Key"/>.
+/// </summary>
+public abstract class SCBlockAccessor : ISaveBlockAccessor<SCBlock>
 {
+    public abstract IReadOnlyList<SCBlock> BlockInfo { get; }
+
+    /// <inheritdoc cref="SCBlock.GetValue"/>
+    public object GetBlockValue(uint key) => GetBlock(key).GetValue();
+
+    /// <inheritdoc cref="SCBlock.SetValue"/>
+    public void SetBlockValue(uint key, object value) => GetBlock(key).SetValue(value);
+
+    /// <summary> Checks if there is any <see cref="SCBlock"/> with the requested <see cref="key"/>. </summary>
+    public bool HasBlock(uint key) => FindIndex(BlockInfo, key) != -1;
+
+    /// <summary> Returns the <see cref="SCBlock"/> reference with the corresponding <see cref="key"/>. </summary>
+    public SCBlock GetBlock(uint key) => Find(BlockInfo, key);
+
+    #region Ease of Use Overloads
+    /// <inheritdoc cref="GetBlock(string)"/>
+    /// <param name="name">Block name (un-hashed)</param>
+    public SCBlock GetBlock(string name) => GetBlock(Hash(name.AsSpan()));
+
+    /// <inheritdoc cref="GetBlock(string)"/>
+    public SCBlock GetBlock(ReadOnlySpan<char> name) => GetBlock(Hash(name));
+    private static uint Hash(ReadOnlySpan<char> name) => (uint)FnvHash.HashFnv1a_64(name);
+
+    /// <inheritdoc cref="GetBlock(string)"/>
+    public SCBlock GetBlock(ReadOnlySpan<byte> name) => GetBlock(Hash(name));
+    private static uint Hash(ReadOnlySpan<byte> name) => (uint)FnvHash.HashFnv1a_64(name);
+    #endregion
+
     /// <summary>
-    /// <see cref="SwishCrypto"/> block accessor, where blocks are ordered by ascending <see cref="SCBlock.Key"/>.
+    /// Tries to grab the actual block, and returns a new dummy if the block does not exist.
     /// </summary>
-    public abstract class SCBlockAccessor : ISaveBlockAccessor<SCBlock>
+    /// <param name="key">Block Key</param>
+    /// <returns>Block if exists, dummy if not. Dummy key will not match requested key.</returns>
+    public SCBlock GetBlockSafe(uint key) => FindOrDefault(BlockInfo, key);
+
+    private static SCBlock Find(IReadOnlyList<SCBlock> array, uint key)
     {
-        public abstract IReadOnlyList<SCBlock> BlockInfo { get; }
+        var index = FindIndex(array, key);
+        if (index != -1)
+            return array[index];
+        throw new KeyNotFoundException(nameof(key));
+    }
 
-        public object GetBlockValue(uint key) => GetBlock(key).GetValue();
-        public void SetBlockValue(uint key, object value) => GetBlock(key).SetValue(value);
-        public bool HasBlock(uint key) => FindIndex(BlockInfo, key) != -1;
+    private static SCBlock FindOrDefault(IReadOnlyList<SCBlock> array, uint key)
+    {
+        var index = FindIndex(array, key);
+        if (index != -1)
+            return array[index];
+        return new SCBlock(0, SCTypeCode.None);
+    }
 
-        // Rather than storing a dictionary of keys, we can abuse the fact that the SCBlock[] is stored in order of ascending block key.
-        // Binary Search doesn't require extra memory like a Dictionary would; also, we only need to find a few blocks.
-        public SCBlock GetBlock(uint key) => BinarySearch(BlockInfo, key);
-
-        /// <summary>
-        /// Tries to grab the actual block, and returns a new dummy if the block does not exist.
-        /// </summary>
-        /// <param name="key">Block Key</param>
-        /// <returns>Block if exists, dummy if not. Dummy key will not match requested key.</returns>
-        public SCBlock GetBlockSafe(uint key) => BinarySearchSafe(BlockInfo, key);
-
-        private static SCBlock BinarySearch(IReadOnlyList<SCBlock> arr, uint key)
+    /// <summary>
+    /// Finds a specified <see cref="key"/> within the <see cref="array"/>.
+    /// </summary>
+    /// <remarks>
+    /// Rather than storing a dictionary of keys, we can abuse the fact that the <see cref="BlockInfo"/> is stored in order of ascending block key.
+    /// <br></br>
+    /// Binary Search doesn't require extra memory like a Dictionary would; also, we usually only need to find a few blocks.
+    /// </remarks>
+    /// <param name="array">Index-able collection</param>
+    /// <param name="key"><see cref="SCBlock.Key"/> to find.</param>
+    /// <returns>Returns -1 if no match found.</returns>
+    private static int FindIndex(IReadOnlyList<SCBlock> array, uint key)
+    {
+        int min = 0;
+        int max = array.Count - 1;
+        do
         {
-            int min = 0;
-            int max = arr.Count - 1;
-            do
-            {
-                int mid = (min + max) / 2;
-                var entry = arr[mid];
-                var ek = entry.Key;
-                if (key == ek)
-                    return entry;
+            int mid = min + ((max - min) >> 1);
+            var entry = array[mid];
+            var ek = entry.Key;
+            if (key == ek)
+                return mid;
 
-                if (key < ek)
-                    max = mid - 1;
-                else
-                    min = mid + 1;
-            } while (min <= max);
-            throw new KeyNotFoundException(nameof(key));
-        }
-
-        private static SCBlock BinarySearchSafe(IReadOnlyList<SCBlock> arr, uint key)
-        {
-            int min = 0;
-            int max = arr.Count - 1;
-            do
-            {
-                int mid = (min + max) / 2;
-                var entry = arr[mid];
-                var ek = entry.Key;
-                if (key == ek)
-                    return entry;
-
-                if (key < ek)
-                    max = mid - 1;
-                else
-                    min = mid + 1;
-            } while (min <= max);
-            return new SCBlock(0, SCTypeCode.None);
-        }
-
-        private static int FindIndex(IReadOnlyList<SCBlock> arr, uint key)
-        {
-            int min = 0;
-            int max = arr.Count - 1;
-            do
-            {
-                int mid = (min + max) / 2;
-                var entry = arr[mid];
-                var ek = entry.Key;
-                if (key == ek)
-                    return mid;
-
-                if (key < ek)
-                    max = mid - 1;
-                else
-                    min = mid + 1;
-            } while (min <= max);
-            return -1;
-        }
+            if (key < ek)
+                max = mid - 1;
+            else
+                min = mid + 1;
+        } while (min <= max);
+        return -1;
     }
 }

PKHeX.Core/Saves/Encryption/SwishCrypto/FnvHash.cs

@@ -1,22 +1,23 @@
-using System.Collections.Generic;
+using System;
 
 namespace PKHeX.Core;
 
 /// <summary>
 /// Fowler–Noll–Vo non-cryptographic hash
 /// </summary>
+/// <remarks>https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function</remarks>
 public static class FnvHash
 {
     private const ulong kFnvPrime_64 = 0x00000100000001b3;
     private const ulong kOffsetBasis_64 = 0xCBF29CE484222645;
 
     /// <summary>
-    /// Gets the hash code of the input sequence via the default Fnv1 method.
+    /// Gets the hash code of the input sequence via the alternative Fnv1 method.
     /// </summary>
     /// <param name="input">Input sequence</param>
     /// <param name="hash">Initial hash value</param>
     /// <returns>Computed hash code</returns>
-    public static ulong HashFnv1a_64(IEnumerable<char> input, ulong hash = kOffsetBasis_64)
+    public static ulong HashFnv1a_64(ReadOnlySpan<char> input, ulong hash = kOffsetBasis_64)
     {
         foreach (var c in input)
         {
@@ -32,7 +33,7 @@ public static ulong HashFnv1a_64(IEnumerable<char> input, ulong hash = kOffsetBa
     /// <param name="input">Input sequence</param>
     /// <param name="hash">Initial hash value</param>
     /// <returns>Computed hash code</returns>
-    public static ulong HashFnv1a_64(IEnumerable<byte> input, ulong hash = kOffsetBasis_64)
+    public static ulong HashFnv1a_64(ReadOnlySpan<byte> input, ulong hash = kOffsetBasis_64)
     {
         foreach (var c in input)
         {

PKHeX.Core/Saves/SAV8SWSH.cs

@@ -209,6 +209,7 @@ public void UnlockAllDiglett()
             if (SaveRevision == 0)
                 return; // no blocks
 
+            // Zone specific values
             (int Zone, int Max)[] zones =
             {
                 (0201, 16), // Fields of Honor
@@ -229,33 +230,36 @@ public void UnlockAllDiglett()
                 (0231, 9), // Honeycalm Island
             };
             var s = Blocks;
-            static uint Hash(string str) => (uint)FnvHash.HashFnv1a_64(str);
             foreach (var (zone, max) in zones)
             {
                 var baseName = $"z_wr{zone:0000}_F_DHIGUDA";
-                s.GetBlock(Hash(baseName)).ChangeBooleanType(SCTypeCode.Bool2);
+                s.GetBlock(baseName).ChangeBooleanType(SCTypeCode.Bool2);
                 for (int i = 0; i <= max; i++)
                 {
                     var otherName = $"{baseName}_{i}";
-                    s.GetBlock(Hash(otherName)).ChangeBooleanType(SCTypeCode.Bool2);
+                    s.GetBlock(otherName).ChangeBooleanType(SCTypeCode.Bool2);
                 }
 
                 var countName = $"WK_EV_R1_DHIG_WR{zone:0000}";
                 var value = max + 2;
                 if (zone == 0223) // trio
-                    value++;
-                s.GetBlock(Hash(countName)).SetValue((uint)value);
+                    value += 2;
+                s.GetBlock(countName).SetValue((uint)value);
             }
 
-            const string TRIO = "z_wr0223_F_TRIO";
-            s.GetBlock(Hash(TRIO)).ChangeBooleanType(SCTypeCode.Bool2);
+            // Atypical named values
+            const string TRIO1 = "z_wr0223_F_TRIO";
+            const string TRIO2 = "FE_R1_DHIGUDA_TRIO";
+            s.GetBlock(TRIO1).ChangeBooleanType(SCTypeCode.Bool2);
+            s.GetBlock(TRIO2).ChangeBooleanType(SCTypeCode.Bool2);
 
+            // Overall named values
             const string unreported = "WK_EV_R1_DHIGUDA_ADD";
             const string totalCount = "WK_EV_R1_DHIGUDA_COUNT";
             const string progressCt = "WK_EV_R1_DHIGUDA_PROGRESS";
-            s.GetBlock(Hash(unreported)).SetValue((uint)0); // none unreported
-            s.GetBlock(Hash(totalCount)).SetValue((uint)150); // all obtained count
-            s.GetBlock(Hash(progressCt)).SetValue((uint)11); // all obtained progress value
+            s.GetBlock(unreported).SetValue((uint)150); // 150 unreported
+            s.GetBlock(totalCount).SetValue((uint)1); // obtained count
+            s.GetBlock(progressCt).SetValue((uint)1); // none obtained progress value
         }
     }
 }