From 89bf529a5e3c7282eeb4d36c3abde606c4d5ec86 Mon Sep 17 00:00:00 2001
From: Sam <samuelkeiffer@gmail.com>
Date: Wed, 2 Mar 2022 22:44:31 -0500
Subject: [PATCH] Addressed review comments

---
 CHANGELOG.md                           |  2 +-
 assets/voxygen/shaders/trail-frag.glsl |  1 +
 voxygen/src/render/renderer/drawer.rs  |  9 ++++++---
 voxygen/src/scene/mod.rs               |  2 +-
 voxygen/src/scene/trail.rs             | 11 +----------
 voxygen/src/session/mod.rs             | 16 +++++++---------
 6 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 399f011f0b..792dd049d7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - Waypoints saved between sessions and shared with group members.
 - New rocks
-- Weapont rails
+- Weapon trails
 
 ### Changed
 
diff --git a/assets/voxygen/shaders/trail-frag.glsl b/assets/voxygen/shaders/trail-frag.glsl
index eef9fbe9f9..23d6585f49 100644
--- a/assets/voxygen/shaders/trail-frag.glsl
+++ b/assets/voxygen/shaders/trail-frag.glsl
@@ -37,6 +37,7 @@ void main() {
     float light_variable = 0.075;
 
     // Make less faint at day (relative to night) by adding light to alpha. Probably hacky but looks fine.
+    // TODO: Trails should also eventually account for shadows, nearby lights, attenuation of sunlight in water, and block based lighting. Note: many of these will require alternative methods that don't require a normal.
     trail_alpha += get_sun_brightness() * light_variable;
 
     tgt_color = vec4(trail_color, trail_alpha);
diff --git a/voxygen/src/render/renderer/drawer.rs b/voxygen/src/render/renderer/drawer.rs
index 5db119816a..1ce91101ca 100644
--- a/voxygen/src/render/renderer/drawer.rs
+++ b/voxygen/src/render/renderer/drawer.rs
@@ -214,6 +214,7 @@ impl<'frame> Drawer<'frame> {
 
     /// Returns None if the clouds pipeline is not available
     pub fn second_pass(&mut self) -> Option<SecondPassDrawer> {
+        let pipelines = &self.borrow.pipelines.all()?;
         let encoder = self.encoder.as_mut().unwrap();
         let device = self.borrow.device;
         let mut render_pass =
@@ -239,8 +240,8 @@ impl<'frame> Drawer<'frame> {
         Some(SecondPassDrawer {
             render_pass,
             borrow: &self.borrow,
-            clouds_pipeline: &self.borrow.pipelines.all()?.clouds,
-            trail_pipeline: &self.borrow.pipelines.all()?.trail,
+            clouds_pipeline: &pipelines.clouds,
+            trail_pipeline: &pipelines.trail,
         })
     }
 
@@ -945,12 +946,14 @@ impl<'pass> SecondPassDrawer<'pass> {
     }
 
     pub fn draw_trails(&mut self) -> Option<TrailDrawer<'_, 'pass>> {
+        let shadow = &self.borrow.shadow?;
+
         let mut render_pass = self.render_pass.scope("trails", self.borrow.device);
 
         render_pass.set_pipeline(&self.trail_pipeline.pipeline);
         set_quad_index_buffer::<trail::Vertex>(&mut render_pass, self.borrow);
 
-        render_pass.set_bind_group(1, &self.borrow.shadow?.bind.bind_group, &[]);
+        render_pass.set_bind_group(1, &shadow.bind.bind_group, &[]);
 
         Some(TrailDrawer { render_pass })
     }
diff --git a/voxygen/src/scene/mod.rs b/voxygen/src/scene/mod.rs
index 26939a206b..36b81468ae 100644
--- a/voxygen/src/scene/mod.rs
+++ b/voxygen/src/scene/mod.rs
@@ -309,7 +309,7 @@ impl Scene {
             select_pos: None,
             light_data: Vec::new(),
             particle_mgr: ParticleMgr::new(renderer),
-            trail_mgr: TrailMgr::new(renderer),
+            trail_mgr: TrailMgr::default(),
             figure_mgr: FigureMgr::new(renderer),
             sfx_mgr: SfxMgr::default(),
             music_mgr: MusicMgr::default(),
diff --git a/voxygen/src/scene/trail.rs b/voxygen/src/scene/trail.rs
index e5ad87c217..67b5f3c74b 100644
--- a/voxygen/src/scene/trail.rs
+++ b/voxygen/src/scene/trail.rs
@@ -12,6 +12,7 @@ struct MeshKey {
     is_main_weapon: bool,
 }
 
+#[derive(Default)]
 pub struct TrailMgr {
     /// Meshes for each entity, usize is the last offset tick it was updated
     entity_meshes: HashMap<MeshKey, (Mesh<TrailVertex>, usize)>,
@@ -33,16 +34,6 @@ const TRAIL_DYNAMIC_MODEL_SIZE: usize = 15;
 const TRAIL_SHRINKAGE: f32 = 0.8;
 
 impl TrailMgr {
-    pub fn new(_renderer: &mut Renderer) -> Self {
-        Self {
-            entity_meshes: HashMap::new(),
-            pos_cache: HashMap::new(),
-            offset: 0,
-            dynamic_model: None,
-            model_len: 0,
-        }
-    }
-
     pub fn maintain(&mut self, renderer: &mut Renderer, scene_data: &SceneData) {
         span!(_guard, "maintain", "TrailMgr::maintain");
 
diff --git a/voxygen/src/session/mod.rs b/voxygen/src/session/mod.rs
index a00a76e438..495aa31209 100644
--- a/voxygen/src/session/mod.rs
+++ b/voxygen/src/session/mod.rs
@@ -1551,15 +1551,13 @@ impl PlayState for SessionState {
         };
 
         // Render world
-        {
-            self.scene.render(
-                drawer,
-                client.state(),
-                client.entity(),
-                client.get_tick(),
-                &scene_data,
-            );
-        }
+        self.scene.render(
+            drawer,
+            client.state(),
+            client.entity(),
+            client.get_tick(),
+            &scene_data,
+        );
 
         if let Some(mut second_pass) = drawer.second_pass() {
             // Clouds