From 6ec6bbec57689420b1a205f289c2d9ff1f34b4aa Mon Sep 17 00:00:00 2001
From: Matchu <matchu1993@gmail.com>
Date: Tue, 6 Oct 2020 06:37:51 -0700
Subject: [PATCH] first-time modeling UX improvements

These changes are most relevant for playing around in the dev server, modeing against an empty database. But they'll also help in real-world modeling scenarios! e.g. modeling a new species/color combo is now a bit nicer, we don't show a blank entry in the color picker
---
 src/app/components/SpeciesColorPicker.js |  7 ++++++-
 src/server/types/AppearanceLayer.js      | 12 +++++++++---
 src/server/types/PetAppearance.js        | 11 +++++++++++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/app/components/SpeciesColorPicker.js b/src/app/components/SpeciesColorPicker.js
index e5e517a..26c490e 100644
--- a/src/app/components/SpeciesColorPicker.js
+++ b/src/app/components/SpeciesColorPicker.js
@@ -171,10 +171,15 @@ function SpeciesColorPicker({
   // think this matches users' mental hierarchy of species -> color: showing
   // supported colors for a species makes sense, but the other way around feels
   // confusing and restrictive.)
+  //
+  // Also, if a color is provided that wouldn't normally be visible, we still
+  // show it. This can happen when someone models a new species/color combo for
+  // the first time - the boxes will still be red as if it were invalid, but
+  // this still smooths out the experience a lot.
   let visibleColors = allColors;
   if (stateMustAlwaysBeValid && valids && speciesId) {
     visibleColors = visibleColors.filter(
-      (c) => getValidPoses(valids, speciesId, c.id).size > 0
+      (c) => getValidPoses(valids, speciesId, c.id).size > 0 || c.id === colorId
     );
   }
 
diff --git a/src/server/types/AppearanceLayer.js b/src/server/types/AppearanceLayer.js
index 386f84e..afa6780 100644
--- a/src/server/types/AppearanceLayer.js
+++ b/src/server/types/AppearanceLayer.js
@@ -94,7 +94,12 @@ const resolvers = {
     imageUrl: async ({ id }, { size }, { swfAssetLoader }) => {
       const layer = await swfAssetLoader.load(id);
 
-      if (!layer.hasImage) {
+      // If there's no image, return null. (In the development db, which isn't
+      // aware which assets we have images for on the DTI CDN, assume we _do_
+      // have the image - it's usually true, and better for testing.)
+      const hasImage =
+        layer.hasImage || process.env["DB_ENV"] === "development";
+      if (!hasImage) {
         return null;
       }
 
@@ -234,12 +239,13 @@ async function loadAndCacheAssetManifest(db, layer) {
   //
   // TODO: Someday the manifests will all exist, right? So we'll want to
   //       reload all the missing ones at that time.
-  manifest = manifest || "";
+  const manifestJson = manifest ? JSON.stringify(manifest) : "";
+
   const [
     result,
   ] = await db.execute(
     `UPDATE swf_assets SET manifest = ? WHERE id = ? LIMIT 1;`,
-    [manifest, layer.id]
+    [manifestJson, layer.id]
   );
   if (result.affectedRows !== 1) {
     throw new Error(
diff --git a/src/server/types/PetAppearance.js b/src/server/types/PetAppearance.js
index b804b05..9f2fa9f 100644
--- a/src/server/types/PetAppearance.js
+++ b/src/server/types/PetAppearance.js
@@ -106,6 +106,17 @@ const resolvers = {
         speciesId: id,
         colorId: "8", // Blue
       });
+
+      // In production, this should ~never happen, because all species have a
+      // Blue version, or at least if a new one is added it will be modeled
+      // quickly! But in development, before modeling happens, it's possible
+      // for this to be empty, so we return a fake body ID. (This seems better
+      // than making it nullable, which adds downstream complexity for a
+      // particularly edge-y case that generally isn't worth considering.)
+      if (!petType) {
+        return `<ERROR-BLUE-PET-NOT-MODELED-FOR-SPECIES-${id}>`;
+      }
+
       return petType.bodyId;
     },
   },