Right, when there are zero layers, we shouldn't say we're loading!
This is a consequence of the HACK below. If we didn't short-circuit the effect when length == 0, then we would go through and successfully load 0 layers.
Movies often have a lot of assets, which are more likely to be cache misses, and take script time to render! So the time until the user sees something is often huge.
Here, we start loading our PNG image at the same time. This is a filesize loading increase, but even in slow connections, it's generally worth it as a _sharp_ improvement in time until you get to see something!
One noteworthy UI weakness here is that we don't show _any_ loading indicator while the image is visible and the movie is still loading. This makes sense from a practical standpoint, but could be a problem when a movie takes a particularly long amount of time. I also want to be cognizant of whether the blink-of-content ever gets annoying! (We could make it fade out 🤔)
In my last change, I didn't try to change the APIs too much, and kept the concept of `crossOrigin` running through `getBestImageUrlForLayer`.
Now, I've moved the `safeImageUrl` call _outside_ `getBestImageUrlForLayer`, by putting it at the call site: We now call `safeImageUrl` from `loadImage` (which needs to know the `crossOrigin` flag anyway!), and at the `img` tag call site.
This simplifies all of the call sites a lot, I think!
I've noticed that our Fastly proxy adds a surprising amount of latency on cache misses (500-1000ms). And, while our overall hit ratio of 80% is pretty good, most misses happen at inopportune times, like loading items from search.
But now that the Neopets CDN supports HTTPS, we can safely switch back to theirs for *most* image loads. (Some features, like downloads and movies, still require CORS headers, which our proxy is still reponsible for adding.)
This forgoes some minor performance wins (like the Download button now requires separate network requests), and some potential filesize reduction opportunities (like Fastly's auto-gzip which we're today using for SVGs, and eventually using their Image Optimizer for assets), to decrease latency. We could still potentially do something more powerful for low-power connections someday… but for now, with the cache miss latency being *so* heavy, this seems like the clear win for almost certainly *all* users today.
I have a hunch that people aren't finding the Download button! I'm not 100% sure what to do about that, but to start, I want right-clicking the image to give you a hint about it 😅
Woo, it's the big UI experiment! Let's see how it plays for folks 😅
Scrolling through a big lists page right now, I think this is a _huge_ improvement, I can get a sense of the lists and what's in them fast, and see matches fast, and dive quickly and with no extra load time when I want more. I'm pleased tbh!
Pulled MarkdownAndSafeHTML into a shared component, and use it on the single list page now too!
I also simplified some of the logic for the item list, because I figure we'll have to give the trade matching stuff its own pass, y'know?
I'm not sure real item data "should" ever do this? Our "Written Word Shower" had a blank description, but I think that was an error on our end.
Anyway, it's clearer than showing infinite loading, so!
Oops, okay, I guess I didn't test the new preview centering stuff with 1200x1200 images, like the Usul's Damask Markings.
Now, I apply a max size to the whole-ass container, and make the parent responsible for centering it.
So I finally started looking into the race condition that makes item previews sometimes fail to load, and as expected, it was that we were trying to load the movie before CreateJS had necessarily loaded. Usually the timing worked out, esp after a reload, but not under certain circumstances!
Anyway, I've been wanting for a while to just bundle them instead. That'll help us more eagerly load them when we need them, and not depend on external CDNs, and remove a bunch of loading state!
So yeah, I had to learn how the `easeljs` and `tweenjs` NPM packages did their bundling, and how to use `imports-loader` to let them just register straight onto `window`! But we got there and it's pretty nice tbh!
Woof, the "Swirl of Power Effect" item tanks my CPU waaay too much
(I bet it's those 7000x7000 PNGs lolol 😬)
Anyway, before thinking about optimizing specific issues, I'm just adding this emergency switch: if we detect FPS < 2 on any layer, we just pause the whole outfit, until the user decides to unpause.
Oh hey, turns out I was missing a step in movie clip stuff! Images aren't just for sprite sheets, but also sometimes they just want the raw images!
Here, we register them, uwu
Items like "Swirl of Power Effect" and probably others work correctly now!
That said, Swirl of Power takes WAY too much CPU lmao, I want to maybe add some kind of automatic kill switch lol
There are a couple spots where we parse SWF URLs to get the ID out! Most visibly, our Support tools were crashing on it. And internally, manifest loading wasn't working. (I'm not sure if this got caught or if it caused crashes in user space? I didn't see them when wearing a failing item)
Anyway, fixed now!
This is because it's the terminology I'm using elsewhere ("Items" and "Closet" are too overloaded in the UI), and because I want to start putting specific lists at like `/user/:userId/lists/:listId`!
I also create a redirect from the old URL, and also from the DTI Classic variant of the URL
Ah right, React state batching doesn't always work how I expect it to. The separate state caused the hook to return and cache `{loading: false, error: null, data: null}`, and then on a _later_ tick the data value showed up, but only _after_ the response was already cached!
This broken a bunch of species/color picker stuff, now it's fixed!
Okay, so getting the initial render down time for these faces is annoying, though I might come back to it…
But actually, the _worst_ part isn't the _initial_ render, which just kinda gets processed as part of the page navigation, right?
The _worst_ part is that we render it slowly _twice_: once on page load, as we send the `useAllValidPetPoses` fetch request; and then again when the fetch request ~instantly comes back from the network cache.
The fact that this requires a double-render, instead of just rendering with the cached valids data in the first place (like how our GraphQL client does), causes a second and highly-visible render of a slow-to-render UI!
So, here we update `useAllValidPetPoses` to cache its response in JS memory, similar in principle to how Apollo Client does. That way, we can return the valids instantly on the first render, if you already loaded them from the homepage or the wardrobe page or another item page!
This is a pretty easy change, that makes re-renders faster when something about the item preview state changes!
That said, the initial render is still pretty slow, too, and that's the one that's bothering me more lol
Ah oops, because I forgot to set `hiResMode` here in the image preloader, we would preload the PNG, and _then_ load the SVG separately.
This doubled the effective image loading time in Hi-Res Mode!
Now, the image preloader respects hi-res mode, and will preload the SVG in the SVG case, and the PNG in the PNG case.
If you're not in hi-res mode, then you don't care about broken SVGs, because you wouldn't have seen them anyway!
We also update the message to reference Hi-Res Mode.
We're just having too many glitchy SVGs for my taste, esp since TNT seems to just be using PNGs for now?
This change defaults us to using PNGs for users by default, with the option to use SVGs as a new "hi-res mode" setting.
This is our first ever setting, wow!
I'm also envisioning that like, if we get Fastly Image Optimizer set up, this could be a way to tune the quality of the incoming images.
We could also consider a setting to turn off animations altogether—like, just download the PNG instead of the movie, whereas right now we download the movie on the assumption that you might play it at any time.
The way we were checking for UC compatibility issues, was also triggering while the appearance was still loading, so items didn't have any appearance layers yet!
Now, we check for loading before testing for that glitch.
Oh right, adding user data to this query makes it uncacheable!
Split the query into the main public data, which will cache; and the user data, which will load in later.
I refactor the hide-badge thing into 3 "trade matching modes". Then, the logic for whether to hide specific badges moves into the component, and we use that same flag to decide whether to show the big word "match"!
Boom, cute owns/wants badges on "Latest items", and the item search page, and trade matches!
I'm gonna add some additional flair to the trade match case, too!
This has been bugging me for a while lol, the background was leaking out of the corners!
I had applied the styles to the `InputGroup` because I didn't realize how Chakra implements this… I had assumed that the left/right elements wouldn't also get the background.
But it turns out, `InputGroup` uses `position: absolute` stuff, and uses padding to create visual space in the `Input` below them! So, this works perfect!
Copied styles from the similar layout in the bulk converter tool! The status will flush to the right of the field header on desktop, and move below the input on mobile.
Mm right, when we first render the output, `imageUrl` is `undefined`, so the output textbox renders with `value={undefined}`, which is an "uncontrolled" component that the DOM is free to change.
In practice, this isn't an issue because the textbox has `isReadOnly`, so the user can't _actually_ change it. But it's still a good idea for consistency and clarity to use an empty string instead of `undefined`, and it removes warning spam from my console!
Oops, previously the MajorErrorMessage was willing to shrink the width of the cute Grundo Programmer icon, to allow error messages with long words to avoid word breaks.
Here, we switch `1fr` for `minmax(0, 1fr)`, which allows the text zone to get smaller. (`1fr` is short for `minmax(auto, 1fr)`, which isn't capable of shrinking smaller than the natural value.)
Now, the error text is more willing to shrink by word-wrapping, than the image is by shrinking the image. Success!
Oops, my cute API idea for `speciesPickerProps` breaks `React.memo`, of course!
We could fix this by having the caller memoize the `speciesPickerProps` object, but that's too unusual and error-prone. We could also fix this by writing a custom function for `React.memo` to determine whether props match, but that seems like overkill when there's only one actual prop we're using here in practice.
So yeah, I've updated `SpeciesColorPicker` to just accept `speciesTestId` and `colorTestId` props instead!
Note that actually this component _is_ still re-rendering too often, because of a Chakra bug I just discovered and reported! So this change won't immediately improve performance, but it should stop re-rendering too often once we _also_ upgrade Chakra after this bug is fixed. https://github.com/chakra-ui/chakra-ui/issues/4080
This folder will include code shared by both the client-side app and the server!
The server isn't using it yet, but it will in a new API endpoint soon! I'm doing this in a separate commit to avoid lumping all the import-change noise into that commit.
I'm doing this in preparation for an API endpoint to build outfit images by ID. It'll need the same logic to decide which layers are visible, and the same GQL fragments to load the relevant data!
Oh right, labeling PB items as NP is confusing! Here, we add a "PB" case to the lil badge on the corner of the item thumbnail, in item search page & homepage Newest Items.
Oops, we did an in-place sort on the search variables we passed to Apollo! This meant that Apollo's first read of the variables wouldn't match later reads, so it would always decide the variables had changed, causing an infinite re-render loop.
Remember to copy existing arrays before sorting! 😅
Incidentally, this only happened for Markings, by coincidence: it's the only (I think) searchable zone label with multiple zone IDs, that don't sort alphabetically the same as they sort numerically. This `.sort()` sorts them alphabetically, whereas they come in numerical order in `allZones`, because that's the order the GQL server returns them in `build-cached-data.js`.
There have been usability problems with this search filter UI, and I think they mostly come down to people accidentally selecting filters when they don't mean to—sometimes pressing Enter to indicate that they're done typing, but accidentally selecting something.
Here, we remove that behavior, and additionally add a new behavior to clear the suggestions on pressing Enter.
Boom, pulled some stuff out into util! Now we also have error boundaries in both panels of the wardrobe page.
You can test this one by visiting `/outfits/new?send-test-error-for-sentry`, just like on the home page! Util component for fake errors owo
Previously, we would tear down to a blank white page. Now, for errors within most page content, we show a cute error message with a grundo programmer!
To test, visit `/?send-test-error-for-sentry`, which will trigger an intentional render error on the home page.
Note that this does _not_ cover pages that don't use PageLayout, namely the wardrobe page! I'll want to add other boundaries there…
Crashes on clearing the search box. I really thought I fixed this when I refactored `forceReset` to be a function, but I guess I missed it when I went back-and-forth deciding whether to actually do the refactor!
Before this, if you made a change while the outfit was auto-saving, it would reset your changes back and forth in an infinite loop, oops!
This was because the response from the save would reset the outfit state to match, but the _debounced_ outfit state would still show the user's changes, so we'd trigger another save. And then the same thing would happen in reverse, and back and forth again!
Oops, it was possible after saving an outfit to get into a state where we would show the `<OutfitThumbnailIfCached />` behind the outfit even after it was saved, and then removing items would look weird until auto-saving caught up.
We had used the `backdrop` property because we wanted smoother partial load-ins, but for now I'm just fixing this by switching it to `placeholder`, which already has the right loading-only behavior.
This was also the only call site for `backdrop`, so I've removed it!
Oops, the sequence here was:
1) Save a new outfit
2) The debounced outfit state still contains id=null, which doesn't match the saved outfit, which triggers an auto-save
3) And now again, the debounced outfit state contains the _previous_ saved outfit ID, but the saved outfit has a _new_ ID, so we save the _previous_ outfit again
and back and forth forever.
Right, ok, simple change: if the saved outfit ID changes, reset the debounced state immediately, so it can't even be out of sync in the first place! (I also considered checking it in the condition, but I didn't really understand what the timing properties of being out of sync due to debouncing would be, and it seemed to not represent the reality I want.)
Hope it actually work-works lol
Did some refactors in useOutfitState to support the new reset action we do after auto-saving, in case the server tweaked things like the name.
Note that we implemented the actual horn behavior described in the message, simply by marking the yellow horn appearance glitched for Fem, but not for Masc! Also, we don't have a yellow-horn Sick Masc model, so it's blue too.
It wouldn't open, because I'd set `isLazy` on the popover, so opening the modal would close and UNMOUNT the popover, which unmounted the modal!
Now, we use the new `lazyBehavior` prop to keep it mounted _after_ the first time it opens. This is why I needed to upgrade Chakra!
Oops, I made a recent change to automatically add `appearanceId` to the outfit state when you open the Support pose picker, to avoid navigation issues.
But I didn't realize this happened _silently_ when you open the page as a Support user, because the Popover preloads!
Now, the Popover doesn't preload its content. This is probably better for normal users too, the PosePicker UI is a bit heavier with 6 previews than I really want!
Oops, our "items to reconsider" feature was preventing unwearing/removing items you're already wearing!
This feature helps you try stuff in Search, without disrupting your outfit. e.g. if you try on a new Background, then change your mind and unwear it, then we reapply whatever old Background you had on the outfit before.
But this made it impossible to remove your _current_ background from the search page if you went back and searched for it again, because we would remove it and then reconsider and reapply it 😅
Now we, um, stop that!
Huh, dunno when I regressed this! Or maybe I never did it for search results, just the main items page? But we're needlessly re-rendering the entire search results list when you wear/unwear something, because `onRemove` always changes, and that breaks the `React.useMemo` on `Item`.
Now, we cache the `onRemove` callback with `React.useCallback`, so perf is much happier!
Oops, we extracted Support fields out from the default `appearanceLayerFragment`!
This was causing the page to silently fail to show any changes, because `layer.remoteId` was evaluating to `undefined` rather than one of the ID numbers in the range.
Here, I've added both `remoteId` explictly because we use it directly, and also the support fields because that's what the layer support UI needs!
Oops, making changes in PosePickerSupport would sometimes trigger a re-fetch in PosePicker.
Specifically, PosePicker needs some fields that PosePickerSupport doesn't, so changing the canonical poses causes PosePicker to ask for stuff again—which will probably serve a SWR'd cached version that doesn't reflect the Support changes!
Here, we update the PosePickerSupport query to prefetch all the fields the PosePicker _would_ want for any of these poses. That way, if we swap in a new one as the canonical appearance for a pose, there's no refetch needed, and therefore no risk of hitting a stale cache.
We move to an actual GQL query, instead of approximating with /api/validPetPoses.
Notable changes are omitting glitched states from UNKNOWN, so we don't prompt Support users to fill in missing states with bad states; and omitting glitched states from standard, so that we _do_ prompt Support users to check UNKNOWN states for new _non-glitched_ versions we can start to use.
Now, when viewing a saved outfit that you own, you'll see a "Saved" indicator if it matches the version on the server, or a temporary UI of "Not saved" and a tooltip if not.
Auto-save coming next!
Previously, the PNG link for a pet layer would show the 150x150 version. This was both an inconvenient size, but also not reflective of how the layer actually behaved, because we only use Neopets's official PNG for the 600x600 version!
Ah, oops, the `id` field from `useOutfitState` went missing and I didn't notice, so `useOutfitSaving` didn't correctly detect that this was an existing outfit!
This made saves on existing outfits create new copies, which isn't a bad behavior exactly, but I don't want to go there; saving a copy is just gonna pollute people's outfit lists rn, worse than no option imo.
Just a basic e2e starting point! Simple logic, with simple gates to prevent saving outfits we're not ready for. Safe to ship, despite being very incomplete!
The layer preloader already takes advantage of, and primes, the HTTP cache.
But we still do duplicate work, on every OutfitPreview render, to re-execute movie clip libraries, and create a movie clip to test for animations. The former is nontrivial cost, and the latter is often large cost. This can make even basic outfit changes slow, when there's no change to the movie clip layers and the player is paused!
Here, we add an LRU cache for movie clip libraries, and for the question of "is it animated?". This should speed up a number of places where we would reload the movie (including between toggling the item), and various changes that were triggering full movie clip rebuilds unnecessarily.
We _aren't_ solving here for the fact that toggling an animated item requires rebuilding the movie clip, which could conceivably be cached—but with some state management trickiness, because ideally it should be a separate clip for each context where it's being shown. Imo not yet worth the effort! (esp because I think users understand that toggling an animated item can be slow, whereas this was affecting _other_ actions way too much)
Here, we consolidate useOutfitState to get its base `outfitState` from a few different places: parsing the URL, and processing the saved outfit data, return an object of the same shape as the state stored in the reducer.
This enables us to just pick one of the three, instead of our kinda awkward individual-field fallbacks.
This will also help us with some upcoming work to make Back/Forward navigation work better.
Now we show a message for pet layers with the DISPLAYS_INCORRECTLY_BUT_CAUSE_UNKNOWN glitch applied! Previously, there would just be no message, because we only had UI logic for when it was applied to an _item_ layer.
Some of the "MiniMME11-S1: Approaching Eventide Skirt" visuals are pretty clearly glitched on TNT's end, like the Jubjub, which just has a single flat version of the dress floating in the corner of the screen.
This is a message to make that case even clearer!
I'm applying this to the "MiniMME11-S1: Approaching Eventide Skirt" on the Acara, which seems to load all 1000 images from the manifest, but then show no animation and no errors. Not sure what's up, and not inclined to deep-debug until we have a check on whether it works on-site!
I wanted the ability to clear out closet list text for Support users, and figured I should just build the UI for end users too, and grant Support users the same access!
This helps with items like "Living in Watermelon Foreground and Background", which has a species-specific foreground and bodyId=0 background.
With this flag set on the background, it won't appear for pets that don't _also_ have something else that fits. In this case, it hides it from Standard Vandas, and all non-standard colors.
There's some hacky limitations here: the item page still highlights the Vanda, even though clicking gives nothing; and the zone info for it is messy too, with the Background claiming to fit all species, and the LFI claiming to fit 54 specific species. But those don't seem important enough to code for!
I like making these more concise and consistent across the dropdown and the cards, I'm still not 100% sold on the design but it seems ok for now!
At first I had it in the dropdown as "Background (3)", but realized that conflicts with the usual pattern of like, saying how many items match a certain filter…
Using this to get an at-a-glance check on how Neopets IDs are typically assigned for body-specific items… looks like it's increasing, and alphabetical by species? (not by species ID?)
I think this will be generally useful to minimize switching around for common operations, but also I'm thinking of building a bulk assign tool for things with broken body IDs, and this will be the place for it to live, I think
Oops, the HTML5Badge was using the presence of an `svgUrl` to decide if the item is converted!
Here, we add an extra condition that if the OFFICIAL_SVG_IS_INCORRECT glitch is applied, then *that* indicates HTML5 conversion, too.
This was one more bit that needed fixing for "Flying in an Airplane": it wasn't just the official SVG that was incorrect, but also the official SWF. So our converted PNG was also incorrect!
Here, we now try to use the official Neopets PNG when the manifest provides it, instead of our own.
Inspired by the "Flying in an Airplane" bug (item 82287), where the official SVG (and I think SWF) were visually glitched and included both zones in the image, but the official PNG was correct.
This flag lets us use the PNG, like the official player does—but only for this item, while still keeping SVGs for everyone else!
I've decided that covering up the species faces with other species info is too weird! It feels like it's removing some ability to cross-reference.
A cool UI affordance would be to have this and the faces interact with each other, like you can hover to highlight the relevant species faces, or even vice-versa, to show the relevant zones for this species. But that's probably way overkill for this relatively niche feature.
Some looked really bad in the new design, like Jewelled Staff, which was breaking between the words "2 species", making a real bad tooltip target too.
Now, there's no line breaks allowed inside a list item at all! We force it to break between items, instead. (Could have also maybe implemented this with flex wrapping? This seemed like a straighter path, but…)
