Simplify the lore counting path and share the expression reader

Sunrise/src/client/content/items/packages/package_node_build.cpp

@@ -5,6 +5,7 @@
 
 #include "../../../../core/logging/log.h"
 #include "../../../../state/build_data/runtime.h"
+#include "../../../../middleware/content/packages/tables/unlock_expression.h"
 #include "internal.h"
 
 namespace sunrise::client::content::items::packages {
@@ -22,88 +23,7 @@ void report(const char* stage, unsigned long long detail) noexcept {
     }
 }
 
-/** Reads a value slot out of one node expression, or reports that it names none. */
-[[nodiscard]] bool expression_value_slot(std::span<const std::byte> table,
-                                         std::size_t rowAt,
-                                         std::size_t field,
-                                         std::int16_t& slot) noexcept {
-    std::int64_t count = 0;
-    std::int64_t relative = 0;
-    std::memcpy(&count, table.data() + rowAt + field, sizeof count);
-    std::memcpy(&relative, table.data() + rowAt + field + 8, sizeof relative);
-    if (count < 1 || count > tables::kNodeExpressionCapacity) {
-        return false;
-    }
-    const std::size_t pointerAt = rowAt + field + 8;
-    const std::int64_t target = static_cast<std::int64_t>(pointerAt) + relative
-                                + static_cast<std::int64_t>(tables::kHeaderSkip);
-    if (target < 0
-        || static_cast<std::size_t>(target)
-                   + static_cast<std::size_t>(count) * tables::kUnlockInstructionStride
-               > table.size()) {
-        return false;
-    }
-    const auto base = static_cast<std::size_t>(target);
-    for (std::int64_t index = 0; index < count; ++index) {
-        std::uint32_t opcode = 0;
-        std::uint32_t operand = 0;
-        const std::size_t at =
-            base + static_cast<std::size_t>(index) * tables::kUnlockInstructionStride;
-        std::memcpy(&opcode, table.data() + at, sizeof opcode);
-        std::memcpy(&operand, table.data() + at + 4, sizeof operand);
-        if (opcode > tables::kUnlockOpcodeCeiling) {
-            return false;
-        }
-        // Opcode ten reads a value, which is the slot a node's progress bar shows.
-        if (opcode == tables::kUnlockReadValueOpcode
-            && operand <= static_cast<std::uint32_t>(INT16_MAX)) {
-            slot = static_cast<std::int16_t>(operand);
-            return true;
-        }
-    }
-    return false;
-}
 
-/** Reads the flag slot one expression tests, or reports that it tests none. */
-[[nodiscard]] bool expression_flag_slot(std::span<const std::byte> table,
-                                        std::size_t rowAt,
-                                        std::size_t field,
-                                        std::int16_t& slot) noexcept {
-    std::int64_t count = 0;
-    std::int64_t relative = 0;
-    std::memcpy(&count, table.data() + rowAt + field, sizeof count);
-    std::memcpy(&relative, table.data() + rowAt + field + 8, sizeof relative);
-    if (count < 1 || count > tables::kNodeExpressionCapacity) {
-        return false;
-    }
-    const std::size_t pointerAt = rowAt + field + 8;
-    const std::int64_t target = static_cast<std::int64_t>(pointerAt) + relative
-                                + static_cast<std::int64_t>(tables::kHeaderSkip);
-    if (target < 0
-        || static_cast<std::size_t>(target)
-                   + static_cast<std::size_t>(count) * tables::kUnlockInstructionStride
-               > table.size()) {
-        return false;
-    }
-    const auto base = static_cast<std::size_t>(target);
-    for (std::int64_t index = 0; index < count; ++index) {
-        std::uint32_t opcode = 0;
-        std::uint32_t operand = 0;
-        const std::size_t at =
-            base + static_cast<std::size_t>(index) * tables::kUnlockInstructionStride;
-        std::memcpy(&opcode, table.data() + at, sizeof opcode);
-        std::memcpy(&operand, table.data() + at + 4, sizeof operand);
-        if (opcode > tables::kUnlockOpcodeCeiling) {
-            return false;
-        }
-        if (opcode == tables::kUnlockReadFlagOpcode
-            && operand <= static_cast<std::uint32_t>(INT16_MAX)) {
-            slot = static_cast<std::int16_t>(operand);
-            return true;
-        }
-    }
-    return false;
-}
 
 } // namespace
 
@@ -148,41 +68,6 @@ bool build_nodes(const reader::Source& source,
         }
     }
 
-    // TEMPORARY PROBE: one lore gate resolves to no account row. Root slot 111 holds several flag
-    // mapping tables and only the account one has been read, so the gate is likely in another scope
-    // rather than absent. Enumerate every descriptor that yields a table and say where 14907 lands.
-    for (std::size_t descriptor = 0; descriptor <= 72; descriptor += 8) {
-        tables::Array probeRows{};
-        if (!tables::find_array_at(std::span<const std::byte>{blob}, descriptor, probeRows)
-            || probeRows.count == 0
-            || probeRows.dataOffset
-                       + static_cast<std::size_t>(probeRows.count) * tables::kUnlockMapRowStride
-                   > blob.size()) {
-            continue;
-        }
-        int found = -1;
-        for (std::uint64_t row = 0; row < probeRows.count; ++row) {
-            std::int16_t slot = 0;
-            std::memcpy(&slot,
-                        blob.data() + probeRows.dataOffset
-                            + static_cast<std::size_t>(row) * tables::kUnlockMapRowStride
-                            + tables::kUnlockMapDestinationSlotOffset,
-                        sizeof slot);
-            if (slot == static_cast<std::int16_t>(14907)) {
-                found = static_cast<int>(row);
-                break;
-            }
-        }
-        std::array<char, 200> line{};
-        const int told = std::snprintf(line.data(), line.size(),
-                                       "ev=fmapscope descriptor=%zu rows=%llu slot14907_index=%d",
-                                       descriptor,
-                                       static_cast<unsigned long long>(probeRows.count), found);
-        if (told > 0) {
-            core::log::write(core::log::Channel::client, core::log::Level::info,
-                             {line.data(), static_cast<std::size_t>(told)});
-        }
-    }
 
     tables::Array characterFlagMapRows{};
     std::unordered_map<std::int16_t, std::uint16_t> characterFlagIndexBySlot{};
@@ -232,40 +117,6 @@ bool build_nodes(const reader::Source& source,
         indexBySlot.emplace(slot, static_cast<std::uint16_t>(row));
     }
 
-    // TEMPORARY PROBE: one lore book's gate reads a value slot that the account map does not
-    // carry, so it is scoped elsewhere. Say which table holds it.
-    for (std::size_t descriptor = 0; descriptor <= 72; descriptor += 8) {
-        tables::Array probeRows{};
-        if (!tables::find_array_at(std::span<const std::byte>{blob}, descriptor, probeRows)
-            || probeRows.count == 0
-            || probeRows.dataOffset
-                       + static_cast<std::size_t>(probeRows.count) * tables::kUnlockMapRowStride
-                   > blob.size()) {
-            continue;
-        }
-        int found = -1;
-        for (std::uint64_t row = 0; row < probeRows.count; ++row) {
-            std::int16_t slot = 0;
-            std::memcpy(&slot,
-                        blob.data() + probeRows.dataOffset
-                            + static_cast<std::size_t>(row) * tables::kUnlockMapRowStride
-                            + tables::kUnlockMapDestinationSlotOffset,
-                        sizeof slot);
-            if (slot == static_cast<std::int16_t>(8275)) {
-                found = static_cast<int>(row);
-                break;
-            }
-        }
-        std::array<char, 200> line{};
-        const int told = std::snprintf(line.data(), line.size(),
-                                       "ev=vmapscope descriptor=%zu rows=%llu slot8275_index=%d",
-                                       descriptor,
-                                       static_cast<unsigned long long>(probeRows.count), found);
-        if (told > 0) {
-            core::log::write(core::log::Channel::client, core::log::Level::info,
-                             {line.data(), static_cast<std::size_t>(told)});
-        }
-    }
 
     tables::Array characterValueMapRows{};
     std::unordered_map<std::int16_t, std::uint16_t> characterValueIndexBySlot{};
@@ -315,8 +166,8 @@ bool build_nodes(const reader::Source& source,
         // The expression sits at one of two fields, and only one of them holds it on any node.
         std::int16_t slot = 0;
         const bool named =
-            expression_value_slot(table, at, tables::kNodeExpressionFieldPrimary, slot)
-            || expression_value_slot(table, at, tables::kNodeExpressionFieldAlternate, slot);
+            tables::expression_value_slot(table, at, tables::kNodeExpressionFieldPrimary, slot)
+            || tables::expression_value_slot(table, at, tables::kNodeExpressionFieldAlternate, slot);
         if (named) {
             const auto found = indexBySlot.find(slot);
             if (found != indexBySlot.end()) {
@@ -340,8 +191,8 @@ bool build_nodes(const reader::Source& source,
         // played: with no title shown there is nothing inside to claim, and nothing to claim leaves
         // the gate shut. Resolve that flag so the gate can be satisfied.
         std::int16_t gateSlot = 0;
-        if (expression_flag_slot(table, at, tables::kNodeExpressionFieldPrimary, gateSlot)
-            || expression_flag_slot(table, at, tables::kNodeExpressionFieldAlternate, gateSlot)) {
+        if (tables::expression_flag_slot(table, at, tables::kNodeExpressionFieldPrimary, gateSlot)
+            || tables::expression_flag_slot(table, at, tables::kNodeExpressionFieldAlternate, gateSlot)) {
             const auto gate = flagIndexBySlot.find(gateSlot);
             if (gate != flagIndexBySlot.end()) {
                 definition.visibilityFlagIndex = gate->second;
@@ -352,100 +203,7 @@ bool build_nodes(const reader::Source& source,
             }
         }
 
-        // TEMPORARY PROBE: sweep the whole row, every four byte offset, not a hand written list
-        // of fields. The previous sweep covered sixteen to one hundred and twenty eight in steps of
-        // eight and so never looked at the first two fields or anything past the child array, which
-        // is most of the reason a gate could still be sitting somewhere unread.
-        if (row == 819 || row == 838) {
-            for (std::size_t field = 0; field + 16 <= tables::kNodeRowStride; field += 4) {
-                std::int64_t count = 0;
-                std::int64_t relative = 0;
-                std::memcpy(&count, table.data() + at + field, sizeof count);
-                std::memcpy(&relative, table.data() + at + field + 8, sizeof relative);
-                if (count < 1 || count > 4096 || relative == 0) {
-                    continue;
-                }
-                const std::size_t pointerAt = at + field + 8;
-                const std::int64_t target = static_cast<std::int64_t>(pointerAt) + relative
-                                            + static_cast<std::int64_t>(tables::kHeaderSkip);
-                if (target < 0
-                    || static_cast<std::size_t>(target)
-                               + static_cast<std::size_t>(count) * tables::kUnlockInstructionStride
-                           > table.size()) {
-                    continue;
-                }
-
-                // Print the stream itself. A field carrying a plausible count and a target inside
-                // the table is worth reading whatever its offset, and the opcodes say at once
-                // whether it is an expression or an array being misread as one.
-                const auto base = static_cast<std::size_t>(target);
-                const std::int64_t shown = count < 8 ? count : 8;
-                for (std::int64_t step = 0; step < shown; ++step) {
-                    std::uint32_t opcode = 0;
-                    std::uint32_t operand = 0;
-                    const std::size_t stepAt =
-                        base + static_cast<std::size_t>(step) * tables::kUnlockInstructionStride;
-                    std::memcpy(&opcode, table.data() + stepAt, sizeof opcode);
-                    std::memcpy(&operand, table.data() + stepAt + 4, sizeof operand);
-                    std::array<char, 200> line{};
-                    const int told = std::snprintf(
-                        line.data(), line.size(),
-                        "ev=fullsweep node=%llu field=%zu count=%lld step=%lld opcode=%u operand=%u",
-                        static_cast<unsigned long long>(row), field,
-                        static_cast<long long>(count), static_cast<long long>(step), opcode,
-                        operand);
-                    if (told > 0) {
-                        core::log::write(core::log::Channel::client, core::log::Level::info,
-                                         {line.data(), static_cast<std::size_t>(told)});
-                    }
-                }
-            }
-        }
 
-        // TEMPORARY PROBE: three lore books resolve no gate at all, yet the client redacts them,
-        // so something gates them that is neither a flag read nor a value read in the two fields
-        // checked. Sweep every aligned field of those rows and dump whatever parses as an
-        // expression, opcode and operand, rather than guessing which field or opcode it uses.
-        if (row == 819 || row == 837 || row == 838) {
-            for (std::size_t field = 0; field + 16 <= tables::kNodeRowStride; field += 4) {
-                std::int64_t instructions = 0;
-                std::int64_t relative = 0;
-                std::memcpy(&instructions, table.data() + at + field, sizeof instructions);
-                std::memcpy(&relative, table.data() + at + field + 8, sizeof relative);
-                if (instructions < 1 || instructions > tables::kNodeExpressionCapacity) {
-                    continue;
-                }
-                const std::size_t pointerAt = at + field + 8;
-                const std::int64_t target = static_cast<std::int64_t>(pointerAt) + relative
-                                            + static_cast<std::int64_t>(tables::kHeaderSkip);
-                if (target < 0
-                    || static_cast<std::size_t>(target)
-                               + static_cast<std::size_t>(instructions)
-                                     * tables::kUnlockInstructionStride
-                           > table.size()) {
-                    continue;
-                }
-                const auto base = static_cast<std::size_t>(target);
-                for (std::int64_t step = 0; step < instructions; ++step) {
-                    std::uint32_t opcode = 0;
-                    std::uint32_t operand = 0;
-                    const std::size_t stepAt =
-                        base + static_cast<std::size_t>(step) * tables::kUnlockInstructionStride;
-                    std::memcpy(&opcode, table.data() + stepAt, sizeof opcode);
-                    std::memcpy(&operand, table.data() + stepAt + 4, sizeof operand);
-                    std::array<char, 180> line{};
-                    const int told = std::snprintf(
-                        line.data(), line.size(),
-                        "ev=noroute node=%llu field=%zu step=%lld opcode=%u operand=%u",
-                        static_cast<unsigned long long>(row), field,
-                        static_cast<long long>(step), opcode, operand);
-                    if (told > 0) {
-                        core::log::write(core::log::Channel::client, core::log::Level::info,
-                                         {line.data(), static_cast<std::size_t>(told)});
-                    }
-                }
-            }
-        }
 
         // Records the node owns, four bytes each as a row and a gate.
         std::int64_t childCount = 0;

Sunrise/src/client/content/items/packages/package_record_build.cpp

@@ -5,6 +5,7 @@
 #include "../../../../core/logging/log.h"
 
 #include "../../../../state/build_data/runtime.h"
+#include "../../../../middleware/content/packages/tables/unlock_expression.h"
 #include "internal.h"
 
 namespace sunrise::client::content::items::packages {
@@ -27,46 +28,6 @@ void report(const char* stage, unsigned long long detail) noexcept {
     return slot > 0;
 }
 
-/** Reads a value slot out of one expression field of one row, or reports that it names none. */
-[[nodiscard]] bool expression_value_slot(std::span<const std::byte> table,
-                                         std::size_t rowAt,
-                                         std::size_t field,
-                                         std::int16_t& slot) noexcept {
-    std::int64_t count = 0;
-    std::int64_t relative = 0;
-    std::memcpy(&count, table.data() + rowAt + field, sizeof count);
-    std::memcpy(&relative, table.data() + rowAt + field + 8, sizeof relative);
-    if (count < 1 || count > tables::kNodeExpressionCapacity) {
-        return false;
-    }
-    const std::size_t pointerAt = rowAt + field + 8;
-    const std::int64_t target = static_cast<std::int64_t>(pointerAt) + relative
-                                + static_cast<std::int64_t>(tables::kHeaderSkip);
-    if (target < 0
-        || static_cast<std::size_t>(target)
-                   + static_cast<std::size_t>(count) * tables::kUnlockInstructionStride
-               > table.size()) {
-        return false;
-    }
-    const auto base = static_cast<std::size_t>(target);
-    for (std::int64_t index = 0; index < count; ++index) {
-        std::uint32_t opcode = 0;
-        std::uint32_t operand = 0;
-        const std::size_t at =
-            base + static_cast<std::size_t>(index) * tables::kUnlockInstructionStride;
-        std::memcpy(&opcode, table.data() + at, sizeof opcode);
-        std::memcpy(&operand, table.data() + at + 4, sizeof operand);
-        if (opcode > tables::kUnlockOpcodeCeiling) {
-            return false;
-        }
-        if (opcode == tables::kUnlockReadValueOpcode
-            && operand <= static_cast<std::uint32_t>(INT16_MAX)) {
-            slot = static_cast<std::int16_t>(operand);
-            return true;
-        }
-    }
-    return false;
-}
 
 } // namespace
 
@@ -188,7 +149,7 @@ bool build_records(const reader::Source& source,
         definition.scoreValue = score <= 0xFFFFU ? static_cast<std::uint16_t>(score) : 0U;
         std::int16_t categorySlot = 0;
         if (!valueIndexBySlot.empty()
-            && expression_value_slot(std::span<const std::byte>{blob},
+            && tables::expression_value_slot(std::span<const std::byte>{blob},
                                      at,
                                      tables::kRecordCategoryExpressionField,
                                      categorySlot)

Sunrise/src/middleware/content/packages/tables/unlock_expression.h

@@ -0,0 +1,87 @@
+#pragma once
+
+#include <cstddef>
+#include <cstdint>
+#include <cstring>
+#include <span>
+
+#include "definition_index_table.h"
+
+namespace sunrise::middleware::content::packages::tables {
+
+/**
+ * Reading unlock expressions out of definition rows.
+ *
+ * A row field holds a count and, eight bytes on, a self-relative offset to a run of instructions.
+ * Each instruction is an opcode then an operand. Which field a row uses varies, and so does what it
+ * carries: the same field holds a value read on one row and a flag test on another, so both readers
+ * are tried against the same fields rather than each field being treated as fixed-purpose.
+ */
+
+/**
+ * Reads the first operand of a given opcode out of one expression field.
+ * @param table Blob the row sits in.
+ * @param rowAt Byte offset of the row.
+ * @param field Byte offset of the expression field within the row.
+ * @param opcode Opcode to look for: kUnlockReadValueOpcode or kUnlockReadFlagOpcode.
+ * @param slot Receives the operand when one is found.
+ * @return True when the field parses as an expression and names that opcode.
+ */
+[[nodiscard]] inline bool expression_operand(std::span<const std::byte> table,
+                                             std::size_t rowAt,
+                                             std::size_t field,
+                                             std::uint32_t opcode,
+                                             std::int16_t& slot) noexcept {
+    std::int64_t count = 0;
+    std::int64_t relative = 0;
+    if (rowAt + field + 16 > table.size()) {
+        return false;
+    }
+    std::memcpy(&count, table.data() + rowAt + field, sizeof count);
+    std::memcpy(&relative, table.data() + rowAt + field + 8, sizeof relative);
+    if (count < 1 || count > kNodeExpressionCapacity) {
+        return false;
+    }
+    const std::size_t pointerAt = rowAt + field + 8;
+    const std::int64_t target = static_cast<std::int64_t>(pointerAt) + relative
+                                + static_cast<std::int64_t>(kHeaderSkip);
+    if (target < 0
+        || static_cast<std::size_t>(target) + static_cast<std::size_t>(count) * kUnlockInstructionStride
+               > table.size()) {
+        return false;
+    }
+    const auto base = static_cast<std::size_t>(target);
+    for (std::int64_t index = 0; index < count; ++index) {
+        std::uint32_t instruction = 0;
+        std::uint32_t operand = 0;
+        const std::size_t at = base + static_cast<std::size_t>(index) * kUnlockInstructionStride;
+        std::memcpy(&instruction, table.data() + at, sizeof instruction);
+        std::memcpy(&operand, table.data() + at + 4, sizeof operand);
+        if (instruction > kUnlockOpcodeCeiling) {
+            return false;
+        }
+        if (instruction == opcode && operand <= static_cast<std::uint32_t>(INT16_MAX)) {
+            slot = static_cast<std::int16_t>(operand);
+            return true;
+        }
+    }
+    return false;
+}
+
+/** Reads the value slot one expression field names, or reports that it names none. */
+[[nodiscard]] inline bool expression_value_slot(std::span<const std::byte> table,
+                                                std::size_t rowAt,
+                                                std::size_t field,
+                                                std::int16_t& slot) noexcept {
+    return expression_operand(table, rowAt, field, kUnlockReadValueOpcode, slot);
+}
+
+/** Reads the flag slot one expression field tests, or reports that it tests none. */
+[[nodiscard]] inline bool expression_flag_slot(std::span<const std::byte> table,
+                                               std::size_t rowAt,
+                                               std::size_t field,
+                                               std::int16_t& slot) noexcept {
+    return expression_operand(table, rowAt, field, kUnlockReadFlagOpcode, slot);
+}
+
+} // namespace sunrise::middleware::content::packages::tables

Sunrise/src/state/build_data/nodes/definition.h

@@ -28,6 +28,11 @@ inline constexpr std::uint16_t kUnavailableValueIndex = 0xFFFFU;
 inline constexpr std::uint16_t kLoreNodeFirst = 815U;
 inline constexpr std::uint16_t kLoreNodeLast = 854U;
 
+/** @return True when this node is a lore book category, the only kind this build counts. */
+[[nodiscard]] constexpr bool lore_category(std::uint16_t definitionIndex) noexcept {
+    return definitionIndex >= kLoreNodeFirst && definitionIndex <= kLoreNodeLast;
+}
+
 /** A node whose gate names no addressable flag carries this instead of an index. */
 inline constexpr std::uint16_t kUnavailableFlagIndex = 0xFFFFU;
 

Sunrise/src/state/build_data/nodes/node_catalog.cpp

@@ -69,6 +69,14 @@ std::size_t count() noexcept {
     return g_definitions.count();
 }
 
+/** Calls back for every node, under the shared lock. */
+void for_each(void* context, void (*visit)(void*, const Definition&) noexcept) noexcept {
+    const Lock::Shared guard(g_lock);
+    for (const Definition& node : g_definitions.rows()) {
+        visit(context, node);
+    }
+}
+
 /** Sets the visibility gate of every lore book category. */
 std::size_t apply_visibility(std::span<std::uint8_t> accountFlags) noexcept {
     const Lock::Shared guard(g_lock);
@@ -100,15 +108,6 @@ std::size_t apply_character_visibility(std::span<std::byte> characterFlags) noex
             static_cast<std::byte>(unlocks::kFlagSet);
         ++set;
     }
-    // TEMPORARY: say whether the character scoped gates are actually being written. This is the one
-    // fix in the batch never confirmed to land, and a silent no-op looks identical to a wrong index.
-    std::array<char, 160> line{};
-    const int written = std::snprintf(line.data(), line.size(),
-                                      "ev=charvis set=%zu bank=%zu", set, characterFlags.size());
-    if (written > 0) {
-        core::log::write(core::log::Channel::state, core::log::Level::info,
-                         {line.data(), static_cast<std::size_t>(written)});
-    }
     return set;
 }
 

Sunrise/src/state/build_data/nodes/node_catalog.h

@@ -50,6 +50,16 @@ void for_each_driving(void* context,
  */
 std::size_t apply_visibility(std::span<std::uint8_t> accountFlags) noexcept;
 
+/**
+ * Calls back for every node, under the shared lock.
+ *
+ * Copying the table out costs a hundred and fifty kilobytes a call, and both callers wanted only a
+ * few fields of a few rows. The callback must not take the catalog lock again.
+ * @param context Passed through untouched.
+ * @param visit Called once per node in native order.
+ */
+void for_each(void* context, void (*visit)(void*, const Definition&) noexcept) noexcept;
+
 /**
  * Sets the character scoped visibility gates of the lore book categories.
  * @param characterFlags Character bank already filled from the authored policy.

Sunrise/src/state/record_claims/record_claims.cpp

@@ -261,6 +261,45 @@ struct NodeProgress {
 
 } // namespace
 
+/**
+ * @return How many of a category's children are claimed. The caller owns the claim lock.
+ *
+ * Both passes count the same thing against different banks, so they share the count rather than
+ * each carrying a copy of the loop.
+ */
+[[nodiscard]] std::int32_t claimed_children(const build_data::nodes::Definition& node) noexcept {
+    std::int32_t claimed = 0;
+    for (std::size_t child = 0; child < node.childCount; ++child) {
+        build_data::records::Definition record{};
+        if (build_data::find_record_definition(node.children[child], record)
+            && record.completionFlagIndex != build_data::records::kUnavailableFlagIndex
+            && claimed_locked(record.completionFlagIndex)) {
+            ++claimed;
+        }
+    }
+    return claimed;
+}
+
+/**
+ * @return One when a category's parent record is itself claimed, otherwise zero.
+ *
+ * The parent is the child naming the category's own value slot. It sits in the category's count and
+ * not in its own bar's, so subtracting it is what separates the two totals. The caller owns the
+ * claim lock.
+ */
+[[nodiscard]] std::int32_t claimed_parent(const build_data::nodes::Definition& node) noexcept {
+    for (std::size_t child = 0; child < node.childCount; ++child) {
+        build_data::records::Definition record{};
+        if (build_data::find_record_definition(node.children[child], record)
+            && record.completionFlagIndex != build_data::records::kUnavailableFlagIndex
+            && record.categoryValueIndex == node.valueIndex
+            && claimed_locked(record.completionFlagIndex)) {
+            return 1;
+        }
+    }
+    return 0;
+}
+
 /** Writes each node's claimed-child count into the value slot its bar reads. */
 std::size_t apply_node_progress(std::span<std::int32_t> objectiveValues) noexcept {
     // The account image is the latest point anything asks for the node table, and on a warm start it
@@ -274,20 +313,15 @@ std::size_t apply_node_progress(std::span<std::int32_t> objectiveValues) noexcep
     // Every category's own index, so the walk can tell a free slot above a category from the next
     // category along. Without it, writing the slot above drives whichever book owns that slot.
     std::vector<char> categoryFlags(objectiveValues.size(), 0);
-    {
-        namespace nodes = build_data::nodes;
-        std::vector<nodes::Definition> all(nodes::kDefinitionCapacity);
-        std::size_t total = 0;
-        if (nodes::snapshot(std::span<nodes::Definition>{all}, total)) {
-            for (std::size_t row = 0; row < total; ++row) {
-                const std::uint16_t index = all[row].valueIndex;
-                if (index != nodes::kUnavailableValueIndex
-                    && static_cast<std::size_t>(index) < categoryFlags.size()) {
-                    categoryFlags[index] = 1;
-                }
+    build_data::nodes::for_each(
+        &categoryFlags, [](void* context, const build_data::nodes::Definition& node) noexcept {
+            auto* flags = static_cast<std::vector<char>*>(context);
+            const std::uint16_t index = node.valueIndex;
+            if (index != build_data::nodes::kUnavailableValueIndex
+                && static_cast<std::size_t>(index) < flags->size()) {
+                (*flags)[index] = 1;
             }
-        }
-    }
+        });
 
     NodeProgress progress{objectiveValues, 0, std::span<const char>{categoryFlags}};
     // The claim lock is taken first and the node lock inside the walk. Nothing takes them the other
@@ -296,35 +330,18 @@ std::size_t apply_node_progress(std::span<std::int32_t> objectiveValues) noexcep
     build_data::nodes::for_each_driving(
         &progress, [](void* context, const build_data::nodes::Definition& node) noexcept {
             auto* state = static_cast<NodeProgress*>(context);
-            // Only the lore books are counted here. Every other category that drives a bar keeps
-            // whatever the authored policy gave it: node 896 carries an authored -1 at its value
-            // index, and writing a count over it replaced a deliberate sentinel with a zero. This
-            // build has no business flattening authored values for categories it does not manage.
-            if (node.definitionIndex < build_data::nodes::kLoreNodeFirst
-                || node.definitionIndex > build_data::nodes::kLoreNodeLast) {
+            // Only the lore books are counted. Every other category that drives a bar keeps what
+            // the authored policy gave it: node 896 carries an authored -1 at its value index, and
+            // writing a count over it replaced a deliberate sentinel with a zero.
+            if (!build_data::nodes::lore_category(node.definitionIndex)) {
                 return;
             }
-            // Two counts, because a category and its parent record keep separate bars. The
-            // category counts every child it owns, the parent record among them; the parent's own
-            // bar counts only the chapters, which is why its denominator is one lower. The parent
-            // is the child that names the category's own value slot.
-            std::int32_t claimed = 0;
-            std::int32_t claimedChapters = 0;
-            for (std::size_t child = 0; child < node.childCount; ++child) {
-                build_data::records::Definition record{};
-                if (!build_data::find_record_definition(node.children[child], record)
-                    || record.completionFlagIndex
-                           == build_data::records::kUnavailableFlagIndex) {
-                    continue;
-                }
-                if (!claimed_locked(record.completionFlagIndex)) {
-                    continue;
-                }
-                ++claimed;
-                if (record.categoryValueIndex != node.valueIndex) {
-                    ++claimedChapters;
-                }
-            }
+            // Two counts, because a category and its parent record keep separate bars. The category
+            // counts every child it owns, the parent among them; the parent's own bar counts only
+            // the chapters, which is why its denominator is one lower. The parent is the child
+            // naming the category's own value slot.
+            const std::int32_t claimed = claimed_children(node);
+            const std::int32_t claimedChapters = claimed - claimed_parent(node);
             // The parent's bar sits one slot above its category on the books where that slot is
             // free, confirmed by marker on two of them. Categories are allocated contiguously
             // though, so for many books the slot above is the next book's category, and writing it
@@ -338,21 +355,6 @@ std::size_t apply_node_progress(std::span<std::int32_t> objectiveValues) noexcep
             if (static_cast<std::size_t>(node.valueIndex) < state->values.size()) {
                 state->values[node.valueIndex] = claimed;
                 ++state->written;
-                // TEMPORARY: name every node driven, so a bar that does not move can be told from
-                // a node that was never counted.
-                std::array<char, 160> line{};
-                const int written = std::snprintf(
-                    line.data(), line.size(),
-                    "ev=nodeprog node=%u value_index=%u parent_index=%u children=%u claimed=%d "
-                    "chapters=%d",
-                    static_cast<unsigned>(node.definitionIndex),
-                    static_cast<unsigned>(node.valueIndex),
-                    static_cast<unsigned>(node.parentValueIndex),
-                    static_cast<unsigned>(node.childCount), claimed, claimedChapters);
-                if (written > 0) {
-                    core::log::write(core::log::Channel::state, core::log::Level::info,
-                                     {line.data(), static_cast<std::size_t>(written)});
-                }
             }
         });
     return progress.written;
@@ -360,36 +362,21 @@ std::size_t apply_node_progress(std::span<std::int32_t> objectiveValues) noexcep
 
 /** Writes each category's claimed-child count into the character value slot its bar reads. */
 std::size_t apply_character_node_progress(std::span<std::int32_t> characterValues) noexcept {
-    namespace nodes = build_data::nodes;
-    std::vector<nodes::Definition> rows(nodes::kDefinitionCapacity);
-    std::size_t count = 0;
-    if (!nodes::snapshot(std::span<nodes::Definition>{rows}, count)) {
-        return 0;
-    }
-
+    NodeProgress progress{characterValues, 0, {}};
+    // Same lock order as the account pass: claims first, catalog inside the walk.
     const std::lock_guard<std::mutex> guard(g_lock);
-    std::size_t written = 0;
-    for (std::size_t row = 0; row < count; ++row) {
-        const nodes::Definition& node = rows[row];
-        if (node.definitionIndex < nodes::kLoreNodeFirst
-            || node.definitionIndex > nodes::kLoreNodeLast
-            || node.characterValueIndex == nodes::kUnavailableValueIndex
-            || static_cast<std::size_t>(node.characterValueIndex) >= characterValues.size()) {
-            continue;
-        }
-        std::int32_t claimed = 0;
-        for (std::size_t child = 0; child < node.childCount; ++child) {
-            build_data::records::Definition record{};
-            if (build_data::find_record_definition(node.children[child], record)
-                && record.completionFlagIndex != build_data::records::kUnavailableFlagIndex
-                && claimed_locked(record.completionFlagIndex)) {
-                ++claimed;
+    build_data::nodes::for_each(
+        &progress, [](void* context, const build_data::nodes::Definition& node) noexcept {
+            auto* state = static_cast<NodeProgress*>(context);
+            if (!build_data::nodes::lore_category(node.definitionIndex)
+                || node.characterValueIndex == build_data::nodes::kUnavailableValueIndex
+                || static_cast<std::size_t>(node.characterValueIndex) >= state->values.size()) {
+                return;
             }
-        }
-        characterValues[node.characterValueIndex] = claimed;
-        ++written;
-    }
-    return written;
+            state->values[node.characterValueIndex] = claimed_children(node);
+            ++state->written;
+        });
+    return progress.written;
 }
 
 /** @return Total score of every held claim. */