Address route code review requests

CHANGELOG.md

@@ -53,7 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
 - Shadow quality option (replaces shadow checkbox)
 - Added Arrow Hotkeys to navigate selected object heirarchies @Froggy618157725
   in [#36](https://github.com/cohaereo/alkahest/pull/36)
-- Added hotkeys to add to current route. + adds at the end of the route, or after current node, - adds before the selected node
+- Added hotkeys to add to current route. <kbd>+</kbd> adds at the end of the route, or after current node, <kbd>-</kbd> adds before the selected node
 - Content confidentiality dialog on first startup
 - Log unimplemented TFX bytecode ops in the TFX debugger
 ### Changed

crates/alkahest-renderer/src/ecs/mod.rs

@@ -10,6 +10,7 @@ pub mod hierarchy;
 pub mod map;
 pub mod render;
 pub mod resources;
+pub mod route;
 pub mod tags;
 pub mod transform;
 pub mod utility;

crates/alkahest-renderer/src/ecs/utility.rs

@@ -1,32 +1,28 @@
-use std::{f32::consts::PI, fmt::Write};
+use std::f32::consts::PI;
 
-use anyhow::Context;
 use bevy_ecs::{
     bundle::Bundle,
     entity::Entity,
     prelude::Component,
-    system::{Commands, In, Query, Res, ResMut},
+    system::{In, Query, Res, ResMut},
 };
 use destiny_pkg::TagHash;
 use ecolor::Rgba;
 use glam::Vec3;
 
 use super::{
-    common::{Global, Icon, Label, Mutable, RenderCommonBundle},
-    hierarchy::Parent,
-    tags::{EntityTag, NodeFilter, Tags},
-    transform::TransformFlags,
-    visibility::{Visibility, VisibilityHelper},
-    MapInfo, SceneInfo,
+    common::{Icon, Label, Mutable, RenderCommonBundle},
+    route::{Route, RouteNode},
+    tags::{NodeFilter, Tags},
+    visibility::VisibilityHelper,
+    MapInfo,
 };
 use crate::{
     ecs::{
         hierarchy::Children, resources::SelectedEntity, transform::Transform,
-        visibility::ViewVisibility, Scene,
-    },
-    icons::{
-        ICON_MAP_MARKER, ICON_MAP_MARKER_PATH, ICON_RULER_SQUARE, ICON_SIGN_POLE, ICON_SPHERE,
+        visibility::ViewVisibility,
     },
+    icons::{ICON_RULER_SQUARE, ICON_SIGN_POLE, ICON_SPHERE},
     renderer::{LabelAlign, Renderer, RendererShared},
     util::{
         color::{Color, ColorExt, Hsv},
@@ -152,210 +148,6 @@ impl Utility for Beacon {
     }
 }
 
-pub struct RouteNodeHolder {
-    pub pos: Vec3,
-    pub map_hash: Option<TagHash>,
-    pub is_teleport: bool,
-    pub label: Option<String>,
-}
-
-impl Default for RouteNodeHolder {
-    fn default() -> Self {
-        Self {
-            pos: Vec3::ZERO,
-            map_hash: None,
-            is_teleport: false,
-            label: None,
-        }
-    }
-}
-pub struct RouteHolder {
-    pub path: Vec<RouteNodeHolder>,
-    pub color: Color,
-    pub rainbow: bool,
-    pub speed_multiplier: f32,
-    pub scale: f32,
-    pub marker_interval: f32,
-    pub show_all: bool,
-    pub activity_hash: Option<TagHash>,
-}
-
-impl Default for RouteHolder {
-    fn default() -> Self {
-        Self {
-            path: vec![],
-            color: Color::WHITE,
-            rainbow: false,
-            speed_multiplier: 1.0,
-            scale: 1.0,
-            marker_interval: 0.0,
-            show_all: false,
-            activity_hash: None,
-        }
-    }
-}
-
-#[derive(Component, Default)]
-pub struct RouteNode {
-    pub map_hash: Option<TagHash>,
-    pub is_teleport: bool,
-}
-
-#[derive(Bundle)]
-pub struct RouteNodeBundle {
-    pub parent: Parent,
-    pub transform: Transform,
-    pub node: RouteNode,
-    pub global: Global,
-    pub util_common: UtilityCommonBundle,
-}
-
-impl RouteNodeBundle {
-    pub fn new(parent: Entity, node: RouteNodeHolder) -> Self {
-        Self {
-            parent: Parent(parent),
-            transform: Transform {
-                translation: node.pos,
-                flags: TransformFlags::IGNORE_ROTATION | TransformFlags::IGNORE_SCALE,
-                ..Default::default()
-            },
-            node: RouteNode {
-                map_hash: node.map_hash,
-                is_teleport: node.is_teleport,
-            },
-            global: Global,
-            util_common: UtilityCommonBundle {
-                label: if let Some(label) = node.label {
-                    RouteNode::label(&label)
-                } else {
-                    RouteNode::default_label()
-                },
-                icon: RouteNode::icon(),
-                filter: NodeFilter::Utility,
-                tags: Tags::from_iter([EntityTag::Utility, EntityTag::Global]),
-                mutable: Mutable,
-                render_common: RenderCommonBundle::default(),
-            },
-        }
-    }
-}
-
-#[derive(Component)]
-pub struct Route {
-    pub color: Color,
-    pub rainbow: bool,
-    pub speed_multiplier: f32,
-    pub scale: f32,
-    pub marker_interval: f32,
-    pub show_all: bool,
-    pub activity_hash: Option<TagHash>,
-}
-
-impl Default for Route {
-    fn default() -> Self {
-        Self {
-            color: Color::WHITE,
-            rainbow: false,
-            speed_multiplier: 1.0,
-            scale: 1.0,
-            marker_interval: 0.0,
-            show_all: false,
-            activity_hash: None,
-        }
-    }
-}
-
-impl Route {
-    pub fn get_command(&self, scene: &Scene, entity: Entity) -> anyhow::Result<String> {
-        let mut command = String::from("route");
-        if let Some(hash) = self.activity_hash.as_ref() {
-            write!(&mut command, " hash {}", hash.0)?;
-        }
-        if let Some(children) = scene.entity(entity).get::<Children>() {
-            for child_ent in &children.0 {
-                let pos = scene
-                    .entity(*child_ent)
-                    .get::<Transform>()
-                    .context("Missing Transform")?;
-                let node = scene
-                    .entity(*child_ent)
-                    .get::<RouteNode>()
-                    .context("Missing Route Node")?;
-                let label = scene
-                    .entity(*child_ent)
-                    .get::<Label>()
-                    .context("Missing Label")?;
-
-                write!(
-                    &mut command,
-                    " node {} {} {}{}{}{}",
-                    pos.translation[0],
-                    pos.translation[1],
-                    pos.translation[2],
-                    if node.is_teleport { " tp" } else { "" },
-                    node.map_hash
-                        .map_or(String::new(), |h| { format!(" hash {}", h.0) }),
-                    if !label.default {
-                        format!(
-                            " label {}",
-                            label.label.replace('\\', r"\\").replace(' ', r"\s")
-                        )
-                    } else {
-                        String::new()
-                    }
-                )?;
-            }
-        }
-        Ok(command)
-    }
-
-    pub fn fixup_visiblity(&self, scene: &Scene, cmd: &mut Commands, entity: Entity) {
-        let mut prev_visible = false;
-        if let Some(children) = scene.get::<Children>(entity) {
-            for (i, child_ent) in children.0.iter().enumerate() {
-                let ent = children.0.get(i + 1);
-                let next_node = ent.and_then(|e| scene.entity(*e).get::<RouteNode>());
-                if let Some(node) = scene.get::<RouteNode>(*child_ent) {
-                    let current_visible = node.map_hash == scene.get_map_hash();
-                    let next_visible =
-                        next_node.map_or(false, |n| n.map_hash == scene.get_map_hash());
-                    let e = scene.entity(*child_ent);
-                    if self.show_all || prev_visible || current_visible || next_visible {
-                        cmd.entity(e.id()).insert(Visibility::Visible);
-                    } else {
-                        cmd.entity(e.id()).insert(Visibility::Hidden);
-                    }
-                    prev_visible = current_visible;
-                }
-            }
-        }
-    }
-}
-
-impl Utility for Route {
-    fn icon() -> Icon {
-        Icon::Unicode(ICON_MAP_MARKER_PATH)
-    }
-
-    fn default_label() -> Label {
-        Label::new_default("Route")
-    }
-}
-
-impl Utility for RouteNode {
-    fn icon() -> Icon {
-        Icon::Unicode(ICON_MAP_MARKER)
-    }
-
-    fn label(str: &str) -> Label {
-        Label::from(str).with_offset(0.0, 0.0, 0.12)
-    }
-
-    fn default_label() -> Label {
-        Label::new_default("").with_offset(0.0, 0.0, 0.12)
-    }
-}
-
 #[allow(clippy::too_many_arguments)]
 pub fn draw_utilities_system(
     In(renderer): In<RendererShared>,

crates/alkahest/src/gui/console.rs

@@ -19,9 +19,9 @@ use alkahest_renderer::{
             dynamic_geometry::DynamicModelComponent, light::ShadowMapRenderer,
             static_geometry::StaticModelSingle,
         },
+        route::{RouteData, RouteNodeData},
         tags::{EntityTag, Tags},
         transform::{OriginalTransform, Transform},
-        utility::{RouteHolder, RouteNodeHolder},
         visibility::Visibility,
     },
     icons::ICON_CUBE,
@@ -617,7 +617,7 @@ fn execute_command(command: &str, args: &[&str], resources: &AppResources) {
             maps.set_maps(resources, &[]);
         }
         "route" => {
-            let mut route = RouteHolder::default();
+            let mut route = RouteData::default();
             let mut i: usize = 0;
             if args[i].to_lowercase().as_str() == "hash" {
                 i += 1;
@@ -641,7 +641,7 @@ fn execute_command(command: &str, args: &[&str], resources: &AppResources) {
                 }
             }
             while i < args.len() {
-                let mut node = RouteNodeHolder::default();
+                let mut node = RouteNodeData::default();
                 match args[i].to_lowercase().as_str() {
                     "node" => 'node: {
                         i += 1;

crates/alkahest/src/gui/inspector/mod.rs

@@ -16,9 +16,10 @@ use alkahest_renderer::{
             light::LightRenderer,
         },
         resources::SelectedEntity,
+        route::{Route, RouteNode},
         tags::{insert_tag, remove_tag, EntityTag, Tags},
         transform::{OriginalTransform, Transform, TransformFlags},
-        utility::{Beacon, Route, RouteNode, Ruler, Sphere},
+        utility::{Beacon, Ruler, Sphere},
         visibility::{Visibility, VisibilityHelper},
         Scene,
     },

crates/alkahest/src/gui/inspector/util.rs

@@ -3,10 +3,9 @@ use alkahest_renderer::{
     ecs::{
         hierarchy::{Children, Parent},
         resources::SelectedEntity,
+        route::{Route, RouteNode, RouteNodeBundle, RouteNodeData},
         transform::Transform,
-        utility::{
-            Beacon, Route, RouteNode, RouteNodeBundle, RouteNodeHolder, Ruler, Sphere, Utility,
-        },
+        utility::{Beacon, Ruler, Sphere, Utility},
         Scene, SceneInfo,
     },
     icons::{
@@ -391,7 +390,7 @@ impl ComponentPanel for Route {
                 let node = cmd
                     .spawn(RouteNodeBundle::new(
                         e.id(),
-                        RouteNodeHolder {
+                        RouteNodeData {
                             pos: camera.position(),
                             map_hash: scene.get_map_hash(),
                             ..Default::default()
@@ -487,7 +486,7 @@ impl ComponentPanel for RouteNode {
                     let node = cmd
                         .spawn(RouteNodeBundle::new(
                             parent.0,
-                            RouteNodeHolder {
+                            RouteNodeData {
                                 pos: camera.position(),
                                 map_hash: scene.get_map_hash(),
                                 ..Default::default()
@@ -517,7 +516,7 @@ impl ComponentPanel for RouteNode {
                     let node = cmd
                         .spawn(RouteNodeBundle::new(
                             parent.0,
-                            RouteNodeHolder {
+                            RouteNodeData {
                                 pos: camera.position(),
                                 map_hash: scene.get_map_hash(),
                                 ..Default::default()

crates/alkahest/src/gui/menu/utility.rs

@@ -4,9 +4,10 @@ use alkahest_renderer::{
         common::{Global, Icon, Label, Mutable, RenderCommonBundle},
         hierarchy::Children,
         resources::SelectedEntity,
+        route::{Route, RouteNodeBundle, RouteNodeData},
         tags::{EntityTag, NodeFilter, Tags},
         transform::{Transform, TransformFlags},
-        utility::{Beacon, Route, RouteNodeBundle, RouteNodeHolder, Ruler, Sphere, Utility},
+        utility::{Beacon, Ruler, Sphere, Utility},
         SceneInfo,
     },
     icons::{ICON_MAP_MARKER_PATH, ICON_POKEBALL, ICON_RULER_SQUARE, ICON_SIGN_POLE, ICON_SPHERE},
@@ -157,7 +158,7 @@ impl MenuBar {
                     .scene
                     .spawn(RouteNodeBundle::new(
                         route_id,
-                        RouteNodeHolder {
+                        RouteNodeData {
                             pos: camera.position(),
                             map_hash: map.scene.get_map_hash(),
                             ..Default::default()

crates/alkahest/src/maplist.rs

@@ -8,7 +8,7 @@ use alkahest_renderer::{
             static_geometry::update_static_instances_system,
         },
         resources::SelectedEntity,
-        utility::Route,
+        route::Route,
         visibility::propagate_entity_visibility_system,
         Scene, SceneInfo,
     },