I'm using my first ever MySQL Store Procedure for clever cleverness in caching the modeling query!
I realized that checking for the latest contribution timestamp is a pretty reliable way of deciding when modeling data was last updated at all. If that timestamp hasn't changed, we can reuse the results!
I figured that, because query roundtrips are a bottleneck in this environment, I didn't want to make that query separately. So, I built a MySQL procedure to do the check on the database side!
I think I got all up in my head about direct queries for this one, because of a previous implementation I had in mind, and I forgot that I could just query species and color from the cache by reference without breaking out of the API provided to the cache function!
I also learned in here that I _can_ look up things from the root by doing `readField("allSpecies", {__ref: "ROOT_QUERY"})`, which I struggled to figure out my previous time. I couldn't figure out how to read an uncached field with arguments (I couldn't quite figure out how to build a proper FieldNode, and passing the string form seemed to provide `null` to the `species` cache field reader), but it's probably doable!
(the permissioning happens on the backend in the prev change! but yeah we send the auth token in the headers, so the backend knows who you are and whether to show you private data)
(also it is just owned items not in any list!)
Essentially I want the center-y visual balance around the image, and the name and _one_ line of tags. If there are extra tags, I want that to go down on its own, rather than bringing down the image+buttons to center against them.
The single-line-of-tags case is the most common, and I think it makes things feel more consistent for all the items to stick to that basic layout, while trying to keep that layout feeling center-y
I was getting annoyed by how, when you're using search, trying on an item will remove conflicting stuff, and then if you decide you don't like what you tried the old stuff _doesn't come back_
As of this change, it does! When you start a new search, we save the outfit state, and then whenever you change the items we ask "hey can these old ones safely be re-worn again?" and re-wear them if so.
On the search panel, not all items have the remove button, and it's confusing to have the other buttons be in inconsistent positions!
Move the Remove button to the left side of the list, so that everything else is positioned the same regardless
Okay, we handle the new pages correctly! Still some weird bugs when you send requests near each other? Probably wise to migrate to Apollo's new way of doing this
This is just an implementation thing, but I realized we can just insert the Zone data into the initial Apollo cache, instead of doing weird field definitions
I _do_ still want the @client tags in the queries though, to tell them not to make server requests at all
Been bothered by this for a while!
My hope is that this isn't a notable marginal performance hit—we were already walking the table and doing string ops anyway, I can't imagine adding to that is actually that much of a marginal lift, when the main bottleneck was probably reads. And the perf should be identical for simple single-word queries anyway. But we'll see how it feels!
This was a subtle little thing for a while! If you switch species/color, such that an item doesn't fit the pet anymore, we used to just hide it. Now, we show it in a list, so that you can understand what went wrong, and have the option to remove it.
Previously, if you switched species/color such that one of your items was no longer compatible, we _would_ still apply its zone restrictions to the visible layer set.
In this change, we fix that server-side, since I think it makes the most sense for an empty appearance to be truly empty!
This was bothering me, I'm surprised and pleased by how easy it seems it was to fix? :)
The strategy is just, look for groups that are provably redundant, and filter them out.
I hope it's correct! It's definitely cozier. Kyrii Mage items are good tests, they have a lot of interesting zones!
I figured that we'd want simpler UI in the ItemsPanel when possible… but now that we've got it pretty simple and comfy, I think the consistency is better
These are nice! :)
The `hideSimpleZones` option I'm not sure about yet, but I figure that:
1. For a new user just doing simple outfits, I feel like the double data on the items page just looks silly, so I want to streamline for that
2. But I _do_ want to let the user think about zone complexity when things _are_ multi-zone.
I did also consider just hiding the zone badge for the header you're under, but I figured the consistency of having the item and its badges look the same in all the places in the list was more important.
This is in preparation for hiding bio zone restrictions but showing item zone restrictions!
I also refactor the build-cached-data script substantially, to run GraphQL against the server instead of a custom query.
okay so the PetAppearance restrictions are stored on the asset, because that's how they're defined on Neopets.com too
but I think that's a confusing API, so here I define `PetAppearance.restrictedZones`, which just maps over the layers and aggregates the zones server-side, same as we would have done on the client
I think that's much easier to understand than having layer contain a field, but having to know that item restrictions _don't_ work that way, you know?
Huh, maybe this is a Firefox bug or something? but the container wasn't applying partial opacity correctly to its children, it was only doing 0 or 1, I think maybe because the children weren't static? I refactor here to make the DOM structure a bit more natural, and fade ins work again 🤷♀️
I'm not sure when this regressed, but changing the outfit was clearing out the whole preview and showing an empty loading state, instead of the intended behavior of showing a loading spinner over the old preview. This affected both the home page and the wardrobe page.
Yeah, so, huh. Fixed! I hope I didn't goof anything else with these effect trigger changes though 😅
The reason I'm doing this now is not just that it's annoying, but as a pair with the color change fix from just now, I want those color changes feeling buttery smooth!
Previously, when changing a pet's color, we would refresh the items panel and send a new network request for the item appearances, even though they're all the same. This is because item appearance data is queried by species/color, for ease of specification.
But! Item appearances are //cached// by body ID. So, if this is a standard color, it's not hard to look in the cache for the standard color's body ID!
Now, most color changes are faster and don't flicker the item panel anymore. We do still refresh the panel and send the requests for color changes that _do_ matter though, like standard <-> mutant!
ahh, in a recent change I made glitched states valid for canonical poses, but didn't make the corresponding change here! This meant that I think the PosePicker showed them, but other ways of getting to them didn't work, including the Candy Acara (who is 100% marked glitched) was no longer pickable at all
Oops, opening the dropdown would auto-focus the left arrow button, because it's now the first focusable element! Make explicit that we want the dropdown instead.
I noticed in prod that the Vercel edge cache can show old data in the Support tool right after you edit it and reload the page, which is super confusing!
In this change, we stop caching the endpoint we use for Support tools, so that the Support tools always feel real-time and trustworthy. (The standard pose picker might still be cached, so it could be a bit confusing for that to be out of sync, but at least you can toggle into Support mode and see that your changes happened _there_, so you don't panic that they're _gone_.)
Previously, I was filtering out glitched appearances from the canonical ones.
But now, I'm thinking it's better to serve glitched ones than no data for a pose at all.
I'm inspired by the case of the Candy Acara, which has _only_ glitched appearances in our db, and I'd like to mark them for reference—but then the site would treat it as no data at all.
Easier to move between appearances quickly! I'm adding this in anticipation of a use case of rapidly labeling Unknown appearances—I want it to be easy to label one and go to the next!
For most users, I want to hide the pose picker if there's not actually anything to pick from.
But I want Support users to be able to open it and use Support mode, to inspect and label Unknown poses!
In this change, I add new UI to show a question mark pose picker button, and a note explaining the empty picker; but only show them for Support users.
I also made a new `useSupport` hook, to replace `useSupportSecret`, now that I have a use case for "is support user?" that doesn't require the secret. Will migrate the rest!
Previously, toggling between the two PosePicker modes could dramatically disrupt the layout, because Popover didn't know to re-measure itself.
In this change, we add a hacky workaround to simulate a window resize event in moments when we know the content is changing. (The realistic ideal solution would still have these manual triggers, but would use an official API in Chakra to notify the Popper instance directly.)
Still just read-only stuff, but now you can look at all the different poses we have for a species/color!
Soon I'll make the pose/glitched stuff editable :3
Some sizable refactors here to add the ability to specify appearance ID as well as pose… most of the app still doesn't use it, it's mostly just lil extra logic to make it win if it's available!
(The rationale for making it an override, rather than always tracking appearance ID, is that it gets really inconvenient in practice to //wait// on looking up the appearance ID in order to start loading various queries. Species/color/pose is a more intuitive key, and works better and faster when the canonical appearance is what you want!)
similar to the layer zoning tools I just rolled out!
not thrilled about the outfit state hacks here bc of how we cache restrict on the appearance rather than the item, but oh well! this escape hatch is pretty easy and solid, and it's a cleanup for another day
Also did a code split here, now that this file is getting larger, to only load this for support users. I don't actually care about restricting console stuff to support users (I'd honestly rather not), but saving the bytes is worth it I think, since support mode is pretty easy to enter when we need to
This is in support of a caching issue in a hack tool coming next! Without this, the change to ItemAppearance restricted zones would make other ItemAppearance fields go missing (bc our hack tool didn't also specify them), so the query would re-execute over the network to find the missing fields we overwrote with nothingness—which would undo the local hack change.
A dev tool to preview what an item would look like in a different zone!
To use, open the browser console, and type `DTIHackLayerZone(layerId, zoneIdOrName)`, but replace `layerId` with the layer's DTI ID, and `zoneIdOrName` with either the zone ID or its name in quotes.
Previously, we were using a custom-y `id` field to help Apollo cross-reference `petAppearance` queries with the results from bulk `petAppearances` queries. Now, instead, we deprecate `petStateId`, and start using `id` to have the same stable value!
This is in anticipation of pet appearance support tools: a stable ID will make it easier to edit them, esp changing their pose (which would otherwise have changed the ID!)
Previously, we would load all `petAppearances` in `PosePicker`, and use cache keys to instantly find it again as a single `petAppearance` in `OutfitPreview` after switching poses.
In this change, we instead have `PosePicker` explicitly load all 6 poses as separate `petAppearance` queries. This simplifies cache sharing between the two components' queries: Apollo can do it automatically, because they were queried the same way in the first place.
I'm doing this in preparation for changing the `id` field of `PetAppearance`, to become `petStateId`. This will help me build pet appearance support tools, by giving the appearances stable identifiers that won't be affected by editing which pose an appearance is!
Huh, some 8-bit species are broken and use the standard body ID!
This was causing our body name query to prioritize 8-bit for standard assets, as the alphabetically-first compatible color; but 8-bit isn't marked standard, so the function kept it labeled 8-bit.
This should fix it and show "Standard Draik" when deleting an asset off the standard draik body!
In practice I saw that this doesn't actually tell you what you _really_ want to know about where the change happened! You want to know it was broken on the Acara or w/e.
Dice reported this, thank you!
My mistake here was that `loadImage` _does_ reject when the image fails to load… but it ends up throwing `undefined`, since I forgot to pass the error along from `onerror` to `reject`!
So we would cancel stuff, but then store `undefined` as our error in state, which our component interprets as no-error.
I tested this by using Firefox DevTools request blocking!
On the homepage, I want to keep the ability to enter invalid species/color pairs, so that you can say "Alien, Aisha" instead of having to pick the Aisha first.
But on the wardrobe page, we were rejecting invalid state changes anyway, so I decided to remove invalid color options from the list. And I added an ability to still switch to any species, and potentially resetting to a basic color automatically to match.
Oops, our loading state logic was eating the error case! I'm not sure exactly where the gap was happening, but I've rewritten the states to be a bit more foolproof, since that first condition was confusing I think.
Ahaha I fucked up a bit! I was indexing into the array of cached zones, instead of looking up by ID. This meant that all zone names were wrong, and some search results weren't loading bc there was no zone data!
I made a fix here, and also added some fallback values, so that if there's an issue in the future we can at least fall back more gracefully than the infinite-spinner case we had here.
In this change, we cache the zones table as part of the JS build process. This keeps the database as our source of truth, while aggressively caching the data at deploy time.
See the new README for some rationale!
I tested this by pulling up dev Honeycomb, and observing that we no longer run db queries to `zones` in the new traces for the wardrobe page. (It's a good thing we did it this way, because I noticed some code in the server that was still loading the zone anyway, and fixed it here!)
This reverts commit 0f7ab9d10e.
The Production Vercel deploys don't seem to like how I did this build trick, even though the Preview deploys seem fine with it 🤔 Reverting for now, sent a message to Vercel support.
Here's just some simple caching: we try to load the asset manifest from the db with the rest of the asset. If it's not present, we load it via HTTP, and write it to the database.
I might try to do a bulk write of manifests at some point, too.
This is because I noticed that one of the main bottlenecks in most of the endpoints now (and definitely the highest-variance) was loading from images.neopets.com.
Another approach I considered was HTTP/2 to load the manifests, because it kinda looks like the server is refusing to open all these sockets at once and effectively does the requests in waves? But images.neopets.com doesn't support HTTP/2 right now anyway, so oh well! (And that would have probably cut us down to ~250ms of HTTP time still, instead of ~600–700. Also, why is network out of Vercel so slow? :p)
I noticed that, while looking up zone data from the db is near instant when you're on the same box, it's like 300ms here!
In this change, we start downloading zone data into the build process. That way, we can have a very fast and practically-up-to-date cache (I'm not sure I've changed it in many years), while being confident that it's in sync with the database source of truth (for things like join queries).
Oops, of course, we weren't actually taking proper advantage of the dataloader here! The queries got over-complicated, but more importantly, subsequent requests to the same loader would re-submit the query!
I noticed it in the SearchPanel operation, in this Honeycomb trace:
https://ui.honeycomb.io/openneo/datasets/dress-to-impress--2020-/trace/aMuhsTjQFZY
We got bit by the "can't run anything after the response finishes" thing
so I'm just forcing the response to wait for Honeycomb submit to finish
I hope this isn't like, just awful for perf lol. but puts to honeycomb seem fast?
An even better resolution mode for "Dyeworks Pink: Peaceful Tree Garland", which doesn't leave any big chunks or holes, and instead takes all the leaves and gives them chunky outlines 😅
The "Dyeworks Pink: Peaceful Tree Garland" was a tricky case, with animated falling leaves…
we decided that having transparency in the main pet area, and some incorrect transparent holes in the trees, was a better conflict resolution for this one
Probably would be good to manually upload a Totally Good version, but like, this flag is probably good to have
Hey wow this was not so hard, just set some global styles, removed some hardcoded colors, and walked through the remaining hardcoded colors to pick a dark mode variant :) neat!!
oops another thing I missed in the refactor! I wasn't providing the selected pose to the hook, so it wasn't styling any of the poses as selected. Now it does again!
Oops, I removed some fields from the pose object, so the aria-label for the options were coming out as "undefined and undefined"!
Now they come out correct!
I tried to get the alignment juuust right in Chrome and Firefox, but Dice's computer was still including a 1px sliver of border.
In this change, I'm adding 1px of extra padding between the border and the Flash area. I figure that, if the screenshots are incorrectly aligned by 1px, that's no big deal and I'm okay accepting that slightly-incorrect upload rather than having folks not be able to use the tool!
I skipped this in the past runs because I had a hard time getting consistency from the results… but they seem to be behaving now?
It really seemed like there were some races on certain query orders… maybe there still is, but my more-reliable connection today is making them resolve in a more consistent order?
Anyway if I see goofs again, I'll consider adding a snapshot matcher that isn't picky about query order 🤔
This happened in an earlier change too, I forget where, but there's some compiler bug that causes comments placed in this kind of spot to yield this error in production, but not in dev:
https://reactjs.org/docs/error-decoder.html/?invariant=152&args
Oh well, moved the comment!
On mobile, it was pretty annoying that you had to show the controls by tapping the preview area to simulate a hover—because it could also click the underlying elements, and do bad stuff!
Here, I add a click capture to prevent the clicks from going down if the controls aren't visible. And I add a toggle, so that you can dismiss the controls, like how YouTube feels :)
Huh, weird how we seem to need preventDefault for buttons, but stopPropagation for links? idk, in any case, the Remove button was doing that thing where it clicks Remove but also bubbles up to the container and clicks it too :p
This was a surprisingly big win! Item is heavier than it looks, because it has like 6 Chakra components, which aren't expensive but aren't _cheap_ in a re-rendered list that needs to be fast, you know?
And it's even more important on search, where there's a lot of items on the page. (we should virtualize it too but that's a thing for another day)
I noticed that item wear/unwear is slow on mobile, because we re-render the whole app tree, and my laptop handles that super fine, but my few-years-old fun takes ~300ms, which is very noticeable.
There's some hacks we could do to get faster feedback, but first I'm diving into the render tree to find the unnecessary renders and stop 'em! That should help build perf across the board, rather than in just one spot, and hopefully be less of a weird sore spot :)
The button section was capable of flex-shrinking, and having those wrap pretty much always looks bad imo. Here, we get more precise about the areas and their flex rules, including only letting the name area grow/shrink. (If the screen is too small for the name to wrap further, the panel area container gains a horizontal scrollbar, which feels like a really good compromise imo)
I think it's more helpful to auto-switch: if you're using the species/color picker for pet compatibility, that's probably what you want! And it's all buffered behind the Save Changes button anyway, so nbd to play a bit fast and loose!
oof, got "too many connections" from mysql, this is probably gonna be a scaling issue in time… for now, stop requesting a pool of 5, even on dev lolol, and just go with a single connection per instance
One fix was that `position: absolute` stuff was appearing over the drawer, including the item search clear button, oof!
Another fix was for a weird bug(?) in Apollo Client, see comments
Note that there's a bug when switching back to the null case… when I look in the Apollo dev tools, it's definitely getting set in the cache correctly at the right time… but the query isn't updating for some reason? I'm hoping it's an Apollo bug that will fix itself someday with an upgrade!
Optimistic UI seems to just be like, not working… I'm seeing some Google results suggesting maybe just get to v3, which is a bit of upgrade work but might be worth it
Oh oops, Chakra UI Next deprecates usePortal for the popover, so it wasn't escaping properly! Add a Portal component to let it escape the top area again!
I guess if you return a reference to an object that doesn't exist, it registers as null; and you need to provide the `true` here to declare that it _is_ real and should be treated as an _insufficiently_ defined object?
Using the newly extracted OutfitPreview! I'm really happy with how this turned out :3
It also makes the pageload after clicking Start super smooth, no spinner! Thanks Apollo cache!!
Kinda surprised our cache redirect isn't good enough for this, but I guess Apollo can't differentiate between a confident empty list vs an uncertain empty list!
It looks like, in prod, our lambda fn here got optimized away, and I'm guessing it's because it starts with a comment now? I moved the comment down a line, fingers crossed!
