From 2687aa6339ee00290c36c02b5f52c82c60036db7 Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Thu, 16 Apr 2026 13:32:11 +0000
Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B9=20Refactor=20curriculum=20page=20l?=
 =?UTF-8?q?oading=20logic=20for=20better=20code=20health?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

*   🎯 **What:** Extracted the document page loading logic inside `load_curriculum_pages` into a new helper function `load_pages_for_document`.
*   💡 **Why:** `load_curriculum_pages` was lengthy and slightly unreadable due to inline data manipulations and variable bindings for accessing the global manifest. This new abstraction neatly scopes that complexity into a separate functional unit.
*   ✅ **Verification:** Validated that existing unit tests pass using `cargo test --bin HEMA-training-tool -- --test-threads=1 ui::curriculum_test` and verified compilation with `cargo check`. Code review confirmed it to be entirely safe and correct.
*   ✨ **Result:** A more readable `load_curriculum_pages` system without behavior changes.

Co-authored-by: dynamikdev <717692+dynamikdev@users.noreply.github.com>
---
 src/ui/curriculum.rs | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/src/ui/curriculum.rs b/src/ui/curriculum.rs
index ec664d1..5e8b888 100644
--- a/src/ui/curriculum.rs
+++ b/src/ui/curriculum.rs
@@ -33,6 +33,30 @@ pub fn toggle_curriculum_visibility(
     }
 }
 
+/// Helper function to load asset handles for all pages of a specific document.
+/// It uses the static manifest to determine the correct page count,
+/// ensuring WASM and packaged app compatibility without runtime file system checks.
+fn load_pages_for_document(
+    asset_server: &Res<AssetServer>,
+    grade: &str,
+    document: &str,
+) -> Vec<Handle<Image>> {
+    let mut new_pages = Vec::new();
+    let page_count = crate::constants::CURRICULUM_MANIFEST
+        .get(grade)
+        .and_then(|docs| docs.iter().find(|(doc, _)| doc == &document))
+        .map(|(_, count)| *count)
+        .unwrap_or(1); // Fallback to 1 if not found, as per original logic
+
+    for i in 1..=page_count {
+        let asset_path = format!("Grades Escrime/{}/{}/page-{:02}.jpg", grade, document, i);
+        let handle: Handle<Image> = asset_server.load(asset_path);
+        new_pages.push(handle);
+    }
+
+    new_pages
+}
+
 /// System to dynamically load the asset handles for a selected document's pages.
 pub fn load_curriculum_pages(
     asset_server: Res<AssetServer>,
@@ -53,21 +77,7 @@ pub fn load_curriculum_pages(
         // out of the box in WASM/cross-platform ways easily without plugin extensions, so
         // pre-generating the paths is safer.
         // For our test documents, max page is 13.
-
-        let mut new_pages = Vec::new();
-        // Look up the exact page count from the static manifest to avoid runtime
-        // file system checks, which ensures WASM and packaged app compatibility.
-        let page_count = crate::constants::CURRICULUM_MANIFEST
-            .get(grade.as_str())
-            .and_then(|docs| docs.iter().find(|(doc, _)| doc == document))
-            .map(|(_, count)| *count)
-            .unwrap_or(1); // Fallback to 0 if not found, loading nothing
-
-        for i in 1..=page_count {
-            let asset_path = format!("Grades Escrime/{}/{}/page-{:02}.jpg", grade, document, i);
-            let handle: Handle<Image> = asset_server.load(asset_path);
-            new_pages.push(handle);
-        }
+        let new_pages = load_pages_for_document(&asset_server, grade, document);
 
         // Check if the actual handles changed (e.g. if the document changed but happened to have the same page count)
         if curriculum_state.pages != new_pages {