Set up a view model for the game review and highlight current node

otg/core/src/lib.rs

@@ -32,3 +32,6 @@ mod types;
 pub use types::{
     BoardError, Color, Config, ConfigOption, DepthTree, LibraryPath, Player, Rank, Size,
 };
+
+mod view_models;
+pub use view_models::GameReviewViewModel;

otg/core/src/types.rs

@@ -1,6 +1,7 @@
 use crate::goban::{Coordinate, Goban};
 use config::define_config;
 use config_derive::ConfigOption;
+use nary_tree::NodeRef;
 use serde::{Deserialize, Serialize};
 use sgf::GameTree;
 use std::{
@@ -252,6 +253,12 @@ impl Deref for DepthTree {
     }
 }
 
+impl Default for DepthTree {
+    fn default() -> Self {
+        Self(nary_tree::Tree::new())
+    }
+}
+
 #[derive(Debug)]
 pub struct SizeNode {
     /// Use this to map back to the node in the original game tree. This way we know how to
@@ -273,32 +280,9 @@ impl SizeNode {
 }
 
 impl DepthTree {
-    // My previous work to convert from a node tree to this tree-with-width dependend on the node tree
-    // being a recursive data structure. Now I need to find a way to convert a slab tree to this width
-    // tree.
-    //
-    // It all feels like a lot of custom weirdness. I shouldn't need a bunch of custom data structures,
-    // so I want to eliminate the "Tree" above and keep using the slab tree. I think I should be able
-    // to build these Node objects without needing a custom data structure.
-    fn new() -> Self {
-        Self(nary_tree::Tree::new())
-        /*
-        Tree {
-            nodes: vec![Node {
-                id: 0,
-                node: root,
-                parent: None,
-                depth: 0,
-                width: RefCell::new(None),
-                children: vec![],
-            }],
-        }
-        */
-    }
-
     /*
     pub fn node(&self, idx: usize) -> &T {
-        &self.nodes[idx].node
+        &self.nodes[idx].content
     }
 
     // Add a node to the parent specified by parent_idx. Return the new index. This cannot be used
@@ -309,7 +293,7 @@ impl DepthTree {
 
         self.nodes.push(Node {
             id: next_idx,
-            node,
+            content: node,
             parent: Some(parent_idx),
             depth: parent.depth + 1,
             width: RefCell::new(None),
@@ -487,7 +471,7 @@ impl<'a> From<&'a GameTree> for DepthTree {
 
                 Self(tree)
             }
-            None => Self::new(),
+            None => Self::default(),
         }
     }
 }
@@ -529,7 +513,7 @@ pub struct BFSIter<'a, T> {
 }
 
 impl<'a, T> Iterator for BFSIter<'a, T> {
-    type Item = &'a T;
+    type Item = NodeRef<'a, T>;
 
     fn next(&mut self) -> Option<Self::Item> {
         let retval = self.queue.pop_front();
@@ -538,7 +522,7 @@ impl<'a, T> Iterator for BFSIter<'a, T> {
                 .children()
                 .for_each(|noderef| self.queue.push_back(noderef));
         }
-        retval.map(|retval| retval.data())
+        retval
     }
 }
 
@@ -732,66 +716,4 @@ mod test {
         assert_eq!(tree.position(test_tree.node_g), (1, 4));
         */
     }
-
-    #[ignore]
-    #[test]
-    fn breadth_first_iter() {
-        /*
-            let mut node_a = MoveNode::new(sgf::Color::Black, Move::Move("dp".to_owned()));
-            let mut node_b = MoveNode::new(sgf::Color::Black, Move::Move("dp".to_owned()));
-            let mut node_c = MoveNode::new(sgf::Color::Black, Move::Move("dp".to_owned()));
-            let node_d = MoveNode::new(sgf::Color::Black, Move::Move("dp".to_owned()));
-            let node_e = MoveNode::new(sgf::Color::Black, Move::Move("dp".to_owned()));
-            let node_f = MoveNode::new(sgf::Color::Black, Move::Move("dp".to_owned()));
-            let node_g = MoveNode::new(sgf::Color::Black, Move::Move("dp".to_owned()));
-            let mut node_h = MoveNode::new(sgf::Color::Black, Move::Move("dp".to_owned()));
-            let node_i = MoveNode::new(sgf::Color::Black, Move::Move("dp".to_owned()));
-
-            let game = GameRecord::new(
-                GameType::Go,
-                Size {
-                    width: 19,
-                    height: 19,
-                },
-                Player {
-                    name: Some("Black".to_owned()),
-                    rank: None,
-                    team: None,
-                },
-                Player {
-                    name: Some("White".to_owned()),
-                    rank: None,
-                    team: None,
-                },
-            );
-
-            node_c.children.push(GameNode::MoveNode(node_d.clone()));
-            node_c.children.push(GameNode::MoveNode(node_e.clone()));
-            node_c.children.push(GameNode::MoveNode(node_f.clone()));
-
-            node_b.children.push(GameNode::MoveNode(node_c.clone()));
-
-            node_h.children.push(GameNode::MoveNode(node_i.clone()));
-
-            node_a.children.push(GameNode::MoveNode(node_b.clone()));
-            node_a.children.push(GameNode::MoveNode(node_g.clone()));
-            node_a.children.push(GameNode::MoveNode(node_h.clone()));
-
-            let game_tree = GameNode::MoveNode(node_a.clone());
-
-            let tree = Tree::from(&game_tree);
-
-            let mut iter = tree.bfs_iter();
-
-            assert_matches!(iter.next(), Some(Node { node: uuid, .. }) => assert_eq!(*uuid, node_a.id));
-            assert_matches!(iter.next(), Some(Node { node: uuid, .. }) => assert_eq!(*uuid, node_b.id));
-            assert_matches!(iter.next(), Some(Node { node: uuid, .. }) => assert_eq!(*uuid, node_g.id));
-            assert_matches!(iter.next(), Some(Node { node: uuid, .. }) => assert_eq!(*uuid, node_h.id));
-            assert_matches!(iter.next(), Some(Node { node: uuid, .. }) => assert_eq!(*uuid, node_c.id));
-            assert_matches!(iter.next(), Some(Node { node: uuid, .. }) => assert_eq!(*uuid, node_i.id));
-            assert_matches!(iter.next(), Some(Node { node: uuid, .. }) => assert_eq!(*uuid, node_d.id));
-            assert_matches!(iter.next(), Some(Node { node: uuid, .. }) => assert_eq!(*uuid, node_e.id));
-            assert_matches!(iter.next(), Some(Node { node: uuid, .. }) => assert_eq!(*uuid, node_f.id));
-        */
-    }
 }

otg/core/src/view_models/game_review.rs

@@ -0,0 +1,105 @@
+/*
+Copyright 2024, Savanni D'Gerinel <savanni@luminescent-dreams.com>
+
+This file is part of On the Grid.
+
+On the Grid is free software: you can redistribute it and/or modify it under the terms of
+the GNU General Public License as published by the Free Software Foundation, either version 3 of
+the License, or (at your option) any later version.
+
+On the Grid is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License along with On the Grid. If not, see <https://www.gnu.org/licenses/>.
+*/
+
+// Currenty my game review is able to show the current game and its tree. Now, I want to start
+// tracking where in the tree that I am. This should be a combination of the abstract Tree and the
+// gameTree. Chances are, if I just keep track of where I am in the abstract tree, I can find the
+// relevant node in the game tree and then reproduce the line to get to that node.
+//
+// Moving through the game review tree shouldn't require a full invocatian. This object, and most
+// other view models, should be exported to the UI.
+
+use crate::{types::SizeNode, DepthTree, Goban};
+use nary_tree::{NodeId, NodeRef};
+use sgf::{GameRecord, Player};
+use std::sync::{Arc, RwLock};
+
+struct GameReviewViewModelPrivate {
+    current_position: Option<NodeId>,
+    game: GameRecord,
+    review_tree: DepthTree,
+}
+
+#[derive(Clone)]
+pub struct GameReviewViewModel {
+    imp: Arc<RwLock<GameReviewViewModelPrivate>>,
+}
+
+impl GameReviewViewModel {
+    pub fn new(game: GameRecord) -> Self {
+        let (review_tree, current_position) = if !game.trees.is_empty() {
+            let review_tree = DepthTree::from(&game.trees[0]);
+            let current_position = game.mainline().unwrap().last().map(|nr| nr.node_id());
+            (review_tree, current_position)
+        } else {
+            (DepthTree::default(), None)
+        };
+
+        Self {
+            imp: Arc::new(RwLock::new(GameReviewViewModelPrivate {
+                current_position,
+                game,
+                review_tree,
+            })),
+        }
+    }
+
+    pub fn black_player(&self) -> Player {
+        self.imp.read().unwrap().game.black_player.clone()
+    }
+
+    pub fn white_player(&self) -> Player {
+        self.imp.read().unwrap().game.white_player.clone()
+    }
+
+    pub fn game_view(&self) -> Goban {
+        let imp = self.imp.read().unwrap();
+        let mainline = imp.game.mainline();
+        match mainline {
+            Some(mainline) => Goban::default()
+                .apply_moves(mainline.map(|nr| nr.data()))
+                .unwrap(),
+            None => Goban::default(),
+        }
+    }
+
+    pub fn map_tree<F>(&self, f: F)
+    where
+        F: Fn(NodeRef<'_, SizeNode>, Option<NodeId>),
+    {
+        let imp = self.imp.read().unwrap();
+
+        for node in imp.review_tree.bfs_iter() {
+            f(node, imp.current_position);
+        }
+    }
+
+    pub fn move_forward(&self) {
+        unimplemented!()
+    }
+
+    pub fn move_backward(&self) {
+        unimplemented!()
+    }
+
+    pub fn next_variant(&self) {
+        unimplemented!()
+    }
+
+    pub fn previous_variant(&self) {
+        unimplemented!()
+    }
+}

otg/core/src/view_models/mod.rs

@@ -0,0 +1,20 @@
+/*
+Copyright 2024, Savanni D'Gerinel <savanni@luminescent-dreams.com>
+
+This file is part of On the Grid.
+
+On the Grid is free software: you can redistribute it and/or modify it under the terms of
+the GNU General Public License as published by the Free Software Foundation, either version 3 of
+the License, or (at your option) any later version.
+
+On the Grid is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License along with On the Grid. If not, see <https://www.gnu.org/licenses/>.
+*/
+
+
+mod game_review;
+pub use game_review::GameReviewViewModel;
+

otg/gtk/src/app_window.rs

@@ -19,10 +19,10 @@ use adw::prelude::*;
 
 use otg_core::{
     settings::{SettingsRequest, SettingsResponse},
-    CoreRequest, CoreResponse,
+    CoreRequest, CoreResponse, GameReviewViewModel,
 };
 use sgf::GameRecord;
-use std::{rc::Rc, sync::{Arc, RwLock}};
+use std::sync::{Arc, RwLock};
 
 use crate::views::{GameReview, HomeView, SettingsView};
 
@@ -91,13 +91,16 @@ impl AppWindow {
 
     pub fn open_game_review(&self, game_record: GameRecord) {
         let header = adw::HeaderBar::new();
-        let game_review = GameReview::new(self.core.clone(), game_record, self.resources.clone());
+        let game_review = GameReview::new(
+            GameReviewViewModel::new(game_record),
+            self.resources.clone(),
+        );
 
         let layout = gtk::Box::builder()
             .orientation(gtk::Orientation::Vertical)
             .build();
         layout.append(&header);
-        layout.append(&game_review);
+        layout.append(&game_review.widget());
 
         let page = adw::NavigationPage::builder()
             .can_pop(true)

otg/gtk/src/components/goban.rs

@@ -37,16 +37,10 @@ You should have received a copy of the GNU General Public License along with On
 
 use crate::{perftrace, Resource, ResourceManager};
 
-use gio::resources_lookup_data;
 use glib::Object;
-use gtk::{
+use gtk::{gdk_pixbuf::Pixbuf, prelude::*, subclass::prelude::*};
-    gdk_pixbuf::{Colorspace, InterpType, Pixbuf},
-    prelude::*,
-    subclass::prelude::*,
-};
-use image::{io::Reader as ImageReader, ImageError};
 use otg_core::{Color, Coordinate};
-use std::{cell::RefCell, io::Cursor, rc::Rc};
+use std::{cell::RefCell, rc::Rc};
 
 const WIDTH: i32 = 800;
 const HEIGHT: i32 = 800;
@@ -257,11 +251,11 @@ impl Pen {
         let (x_loc, y_loc) = self.stone_location(row, col);
         match color {
             Color::White => match self.white_stone {
-                Some(ref white_stone) => ctx.set_source_pixbuf(&white_stone, x_loc, y_loc),
+                Some(ref white_stone) => ctx.set_source_pixbuf(white_stone, x_loc, y_loc),
                 None => ctx.set_source_rgb(0.9, 0.9, 0.9),
             },
             Color::Black => match self.black_stone {
-                Some(ref black_stone) => ctx.set_source_pixbuf(&black_stone, x_loc, y_loc),
+                Some(ref black_stone) => ctx.set_source_pixbuf(black_stone, x_loc, y_loc),
                 None => ctx.set_source_rgb(0.0, 0.0, 0.0),
             },
         }
@@ -309,34 +303,3 @@ impl Pen {
         )
     }
 }
-
-fn load_pixbuf(
-    path: &str,
-    transparency: bool,
-    width: i32,
-    height: i32,
-) -> Result<Option<Pixbuf>, ImageError> {
-    let image_bytes = resources_lookup_data(path, gio::ResourceLookupFlags::NONE).unwrap();
-
-    let image = ImageReader::new(Cursor::new(image_bytes))
-        .with_guessed_format()
-        .unwrap()
-        .decode();
-    image.map(|image| {
-        let stride = if transparency {
-            image.to_rgba8().sample_layout().height_stride
-        } else {
-            image.to_rgb8().sample_layout().height_stride
-        };
-        Pixbuf::from_bytes(
-            &glib::Bytes::from(image.as_bytes()),
-            Colorspace::Rgb,
-            transparency,
-            8,
-            image.width() as i32,
-            image.height() as i32,
-            stride as i32,
-        )
-        .scale_simple(width, height, InterpType::Nearest)
-    })
-}

otg/gtk/src/components/review_tree.rs

@@ -15,50 +15,29 @@ You should have received a copy of the GNU General Public License along with On
 */
 
 use cairo::Context;
-use glib::Object;
+use gtk::prelude::*;
-use gtk::{prelude::*, subclass::prelude::*};
+use otg_core::GameReviewViewModel;
-use otg_core::DepthTree;
-use sgf::GameRecord;
-use std::{cell::RefCell, rc::Rc};
-use uuid::Uuid;
 
 const WIDTH: i32 = 200;
 const HEIGHT: i32 = 800;
 
-#[derive(Default)]
+#[derive(Clone)]
-pub struct ReviewTreePrivate {
+pub struct ReviewTree {
-    record: Rc<RefCell<Option<GameRecord>>>,
+    widget: gtk::DrawingArea,
-    tree: Rc<RefCell<Option<DepthTree>>>,
-}
 
-#[glib::object_subclass]
+    view: GameReviewViewModel,
-impl ObjectSubclass for ReviewTreePrivate {
-    const NAME: &'static str = "ReviewTree";
-    type Type = ReviewTree;
-    type ParentType = gtk::DrawingArea;
-}
-
-impl ObjectImpl for ReviewTreePrivate {}
-impl WidgetImpl for ReviewTreePrivate {}
-impl DrawingAreaImpl for ReviewTreePrivate {}
-
-glib::wrapper! {
-    pub struct ReviewTree(ObjectSubclass<ReviewTreePrivate>) @extends gtk::Widget, gtk::DrawingArea;
 }
 
 impl ReviewTree {
-    pub fn new(record: GameRecord) -> Self {
+    pub fn new(view: GameReviewViewModel) -> ReviewTree {
-        let s: Self = Object::new();
+        let widget = gtk::DrawingArea::new();
 
-        // TODO: there can be more than one tree, especially in instructional files. Either unify
+        widget.set_width_request(WIDTH);
-        // them into a single tree in the GameTree, or draw all of them here.
+        widget.set_height_request(HEIGHT);
-        *s.imp().tree.borrow_mut() = Some(DepthTree::from(&record.trees[0]));
-        *s.imp().record.borrow_mut() = Some(record);
 
-        s.set_width_request(WIDTH);
+        let s = Self { widget, view };
-        s.set_height_request(HEIGHT);
 
-        s.set_draw_func({
+        s.widget.set_draw_func({
             let s = s.clone();
             move |_, ctx, width, height| {
                 s.redraw(ctx, width, height);
@@ -68,27 +47,26 @@ impl ReviewTree {
         s
     }
 
-    pub fn redraw(&self, ctx: &Context, _width: i32, _height: i32) {
+    fn redraw(&self, ctx: &Context, _width: i32, _height: i32) {
-        let tree: &Option<DepthTree> = &self.imp().tree.borrow();
+        self.view.map_tree(move |node, current| {
-        match tree {
+            ctx.set_source_rgb(0.7, 0.7, 0.7);
-            Some(ref tree) => {
+            let (row, column) = node.data().position();
-                for node in tree.bfs_iter() {
+            let y = (row as f64) * 20. + 10.;
-                    // draw a circle given the coordinates of the nodes
+            let x = (column as f64) * 20. + 10.;
-                    // I don't know the indent. How do I keep track of that? Do I track the position of
+            ctx.arc(x, y, 5., 0., 2. * std::f64::consts::PI);
-                    // the parent? do I need to just make it more intrinsically a part of the position
+
-                    // code?
+            if current == Some(node.node_id()) {
-                    ctx.set_source_rgb(0.7, 0.7, 0.7);
+                ctx.set_line_width(3.);
-                    let (row, column) = node.position();
+            } else {
-                    let y = (row as f64) * 20. + 10.;
+                ctx.set_line_width(1.);
-                    let x = (column as f64) * 20. + 10.;
-                    ctx.arc(x, y, 5., 0., 2. * std::f64::consts::PI);
-                    let _ = ctx.stroke();
-                }
             }
-            None => {
+
-                // if there is no tree present, then there's nothing to draw!
+            let _ = ctx.stroke();
-            }
+        });
-        }
+    }
+
+    pub fn widget(&self) -> gtk::Widget {
+        self.widget.clone().upcast::<gtk::Widget>()
     }
 }
 
@@ -125,12 +103,6 @@ impl ReviewTree {
 // A bottom-up traversal:
 // - Figure out the number of nodes at each depth
 
-/*
-struct Tree {
-    width: Vec<usize>, // the total width of the tree at each depth
-}
-*/
-
 #[cfg(test)]
 mod test {
     use super::*;

otg/gtk/src/lib.rs

@@ -49,8 +49,8 @@ pub struct ResourceManager {
     resources: Rc<RefCell<HashMap<String, Resource>>>,
 }
 
-impl ResourceManager {
+impl Default for ResourceManager {
-    pub fn new() -> Self {
+    fn default() -> Self {
         let mut resources = HashMap::new();
 
         for (path, xres, yres, transparency) in [
@@ -88,7 +88,9 @@ impl ResourceManager {
             resources: Rc::new(RefCell::new(resources)),
         }
     }
+}
 
+impl ResourceManager {
     pub fn resource(&self, path: &str) -> Option<Resource> {
         self.resources.borrow().get(path).cloned()
     }
@@ -123,7 +125,6 @@ impl ResourceManager {
             .scale_simple(width, height, InterpType::Nearest)
         })
     }
-
 }
 
 pub fn perftrace<F, A>(trace_name: &str, f: F) -> A

otg/gtk/src/main.rs

@@ -122,7 +122,7 @@ fn main() {
 
     app.connect_activate({
         move |app| {
-            let resources = ResourceManager::new();
+            let resources = ResourceManager::default();
             let core_api = CoreApi { core: core.clone() };
             let app_window = AppWindow::new(app, core_api, resources);
 

otg/gtk/src/views/game_review.rs

@@ -22,16 +22,15 @@ You should have received a copy of the GNU General Public License along with On
 // I'll get all of the information about the game from the core, and then render everything in the
 // UI. So this will be a heavy lift on the UI side.
 
-use crate::{
+use crate::{components::{Goban, PlayerCard, ReviewTree}, ResourceManager};
-    components::{Goban, PlayerCard, ReviewTree}, CoreApi, ResourceManager
+use gtk::{prelude::*};
-};
+use otg_core::{Color, GameReviewViewModel};
-use glib::Object;
-use gtk::{prelude::*, subclass::prelude::*};
-use otg_core::Color;
-use sgf::GameRecord;
 
+/*
 #[derive(Default)]
-pub struct GameReviewPrivate {}
+pub struct GameReviewPrivate {
+    model: Rc<RefCell<Option<GameReviewViewModel>>>,
+}
 
 #[glib::object_subclass]
 impl ObjectSubclass for GameReviewPrivate {
@@ -52,14 +51,40 @@ impl GameReview {
     pub fn new(_api: CoreApi, record: GameRecord, resources: ResourceManager) -> Self {
         let s: Self = Object::builder().build();
 
+
+        s
+    }
+}
+*/
+
+pub struct GameReview {
+    widget: gtk::Box,
+
+    resources: ResourceManager,
+    view: GameReviewViewModel,
+}
+
+impl GameReview {
+    pub fn new(view: GameReviewViewModel, resources: ResourceManager) -> Self {
+        let widget = gtk::Box::builder().build();
+
+        let s = Self {
+            widget,
+            resources,
+            view,
+        };
+
+        s.render();
+
+        s
+    }
+
+    fn render(&self) {
         // It's actually really bad to be just throwing away errors. Panics make everyone unhappy.
         // This is not a fatal error, so I'll replace this `unwrap` call with something that
         // renders the board and notifies the user of a problem that cannot be resolved.
-        let board_repr = match record.mainline() {
+        let board_repr = self.view.game_view();
-            Some(iter) => otg_core::Goban::default().apply_moves(iter).unwrap(),
+        let board = Goban::new(board_repr, self.resources.clone());
-            None => otg_core::Goban::default(),
-        };
-        let board = Goban::new(board_repr, resources);
 
         /*
         s.attach(&board, 0, 0, 2, 2);
@@ -76,7 +101,7 @@ impl GameReview {
         // The review tree needs to know the record for being able to render all of the nodes. Once
         // keyboard input is being handled, the tree will have to be updated on each keystroke in
         // order to show the user where they are within the game record.
-        let review_tree = ReviewTree::new(record.clone());
+        let review_tree = ReviewTree::new(self.view.clone());
 
         // I think most keyboard focus is going to end up being handled here in GameReview, as
         // keystrokes need to affect both the goban and the review tree simultanesouly. Possibly
@@ -88,14 +113,18 @@ impl GameReview {
             .spacing(4)
             .build();
 
-        player_information_section.append(&PlayerCard::new(Color::Black, &record.black_player));
+        player_information_section
-        player_information_section.append(&PlayerCard::new(Color::White, &record.white_player));
+            .append(&PlayerCard::new(Color::Black, &self.view.black_player()));
+        player_information_section
+            .append(&PlayerCard::new(Color::White, &self.view.white_player()));
 
-        s.append(&board);
+        self.widget.append(&board);
         sidebar.append(&player_information_section);
-        sidebar.append(&review_tree);
+        sidebar.append(&review_tree.widget());
-        s.append(&sidebar);
+        self.widget.append(&sidebar);
+    }
 
-        s
+    pub fn widget(&self) -> gtk::Widget {
+        self.widget.clone().upcast::<gtk::Widget>()
     }
 }

sgf/src/game.rs

@@ -2,8 +2,8 @@ use crate::{
     parser::{self, Annotation, Evaluation, Move, SetupInstr, Size, UnknownProperty},
     Color, Date, GameResult, GameType,
 };
-use serde::{Deserialize, Serialize};
 use nary_tree::{NodeId, NodeMut, NodeRef, Tree};
+use serde::{Deserialize, Serialize};
 use std::{
     collections::{HashMap, HashSet, VecDeque},
     fmt,
@@ -136,7 +136,7 @@ impl GameRecord {
     /// Generate a list of moves which constitute the main line of the game. This is the game as it
     /// was actually played out, and by convention consists of the first node in each list of
     /// children.
-    pub fn mainline(&self) -> Option<impl Iterator<Item = &'_ GameNode>> {
+    pub fn mainline(&self) -> Option<impl Iterator<Item = NodeRef<'_, GameNode>>> {
         if !self.trees.is_empty() {
             Some(MainlineIter {
                 next: self.trees[0].root(),
@@ -405,7 +405,7 @@ pub struct MainlineIter<'a> {
 }
 
 impl<'a> Iterator for MainlineIter<'a> {
-    type Item = &'a GameNode;
+    type Item = NodeRef<'a, GameNode>;
 
     fn next(&mut self) -> Option<Self::Item> {
         if let Some(next) = self.next.take() {
@@ -413,7 +413,7 @@ impl<'a> Iterator for MainlineIter<'a> {
             self.next = next
                 .first_child()
                 .and_then(|child| self.tree.get(child.node_id()));
-            Some(ret.data())
+            Some(ret)
         } else {
             None
         }
@@ -634,6 +634,7 @@ mod test {
         assert_eq!(tree.nodes().len(), 0);
     }
 
+    /*
     #[test]
     fn it_can_add_moves_to_a_game() {
         let mut game = GameRecord::new(
@@ -660,6 +661,7 @@ mod test {
         assert_eq!(nodes[1].id(), second_move.id);
         */
     }
+*/
 
     #[ignore]
     #[test]
@@ -820,6 +822,7 @@ mod path_test {
                 let moves = game
                     .mainline()
                     .expect("there should be a mainline in this file")
+                    .map(|nr| nr.data())
                     .collect::<Vec<&GameNode>>();
                 assert_matches!(moves[0], GameNode::MoveNode(node) => {
                     assert_eq!(node.color, Color::Black);
@@ -845,6 +848,7 @@ mod path_test {
             let moves = game
                 .mainline()
                 .expect("there should be a mainline in this file")
+                .map(|nr| nr.data())
                 .collect::<Vec<&GameNode>>();
             assert_matches!(moves[1], GameNode::MoveNode(node) => {
                 assert_eq!(node.color, Color::White);