Oh huh, I guess most of the new items we had when I rewrote this were
Maraquan, and I didn't test enough on standard species-specific items.
Before this change, partially-modeled items for standard pets would
appear as fully modeled, because the presence of the "nonstandard"
color Orange (because of the Orange Chia) meant that the "standard" key
didn't actually have any unique bodies (it was all `["standard", 47]`).
Here, I take my own comments' advice and move away from the standard
label as part of the logic. Instead, we look first for nonstandard
colors with unique bodies, which we'll call out as modelable; and then
check whether there are any basic bodies *not* covered by those special
colors.
That way, compatibility with the Maraquan Acara (a unique body) means
we'll treat Maraquan as a modelable color; and then we'll ignore the
basic bodies, even though it *does* fit the basic Mynci, because there
aren't any compatible basic bodies that aren't *also* Maraquan bodies.
This also means that compatibility with both the Blue Acara and Orange
Acara does *not* preclude a normal item from needing basic pets for
models: because, while Orange is a slightly nonstandard color in the
case of Chia, it doesn't have its own unique body for this item, so we
ignore it when predicting compatibility with basic colors.
This is because labeling poses with the Support tools *should*
invalidate the `PetState.all_supported_poses` cache! But the previous
cache key would only invalidate when a new pet state is *added*, not
when one is *edited*.
This clocks in a bit bigger than what Impress 2020 does in terms of
binary encoding (with gzip it's at 11K instead of 4K), but I'm okay
with that for the simplicity win.
Gonna try to swap this in for where we're still using Impress 2020 for
the species/color picker in the outfit editor!
Before this change, pages that opt in with `use_responsive_design`
would often have the top nav be real cluttered for logged-in users. (I
think I happened to first test this responsive design without being
logged in on my dev box, oops!) Because the home link and `#userbar`
were absolutely positioned on the page, they would frequently overlap.
Here, I stop doing our old tricks to make the top nav load last on the
page. (This was to get "main content" loading faster, which I think is
a. not as relevant today with more commonly faster connections, and b.
was a bit naive to think that it'd be helpful to have to wait a long
time to _navigate_ if a page is unexpectedly large.)
These tricks used to leave some padding at the top of the `#container`,
which these elements would then visually fill via `position: absolute`
once they load.
Next, I update the CSS (for the responsive design pages only) to use
the new `#main-nav` container to lay them out in Flexbox: all in one
row if possible, or wrapped if needed.
Some designs hide stuff like this into a hamburger menu or such when
the screen gets small. I haven't done that here! No specific reason,
I'm just not sold that it's that much better, or worth the trouble.
I tested this on the following combinations:
1. Logged out, homepage
2. Logged in, homepage
3. Logged out, `/items`
4. Logged in, `/items`
5. Logged out, `/items/89487-Dyeworks-Purple-Haunted-Sky-Background`
6. Logged in, `/items/89487-Dyeworks-Purple-Haunted-Sky-Background`
Hope it's solid! 🤞
Before this change, a fully-modeled item (Dyeworks Burgundy: Gown of
the Night) was displaying as still needing the Chia. This was because
looking for "standard" body IDs like this caught up some of the weird
Chia bodies.
I think there's probably something here where we need to like, relabel
certain colors? But honestly, the better version of this logic would
probably be to lean more into the `basic` label in this logic.
But hey, that's a refactor for another time. I gotta go eat!
Noticing a lot of Maraquan items on the homepage today, and they're
doing that thing of expecting standard body types to be relevant too,
because I think we wrote this logic before the Maraquan Mynci ended up
having the same standard Mynci body? (Maybe? Or maybe I'm being
ahistorical and I just wrote this wrong to begin with lol)
In any case, this is more accurate, and I think I'm also maybe
incidentally noticing that it's running faster, at least in my brief
before/after production testing? (There's *more* queries, like 100! But
many of them are *very* fast lookups, coming in at under 1ms—and also a
lot of them are dupes being served by Rails's request-scoped query
cache.)
Huh, I hadn't realized that like, we'd already set up the controller to
always *run* basically all of the modeling logic, and the caching in
the view layer wasn't saving us any queries anymore. Kinda silly!
Remove the caching call, just to simplify the codebase (I like to avoid
caching things that don't specifically need it!).
And hey, love that the modeling code in the controller is now *way*
faster to run! You love to see it!
I have some other changes planned too, but these are some easy ones. I
also turn back on this stuff in development, in hopes that my changes
can make these queries fast enough to not be a big deal anymore!
This is the second part of the previous change `efda6d74`, in which we
switch out the item search query conditions!
This was a two-parter to ease deployment: first deploy the change with
the migration, then *run* the migration (because it's an unusually slow
one), then deploy this change that actually uses it.
Another way to approach this would've been to deploy it all in one
commit, but not set it as `current` until we had run the migration.
That would have been a reasonable approach too!
This is the first part of a change to improve search performance, by
caching occupied zone IDs and supported body IDs onto the Item record
itself, instead of always doing joins with `SwfAsset`.
It's unfortunate, because part of the power of SQL is joins! But doing
joins with big tables, in ways that can't take advantage of indexes in
the same ways as we often want to, is… slow.
It's possible there's something I'm misunderstanding about SQL
optimization, and this _could_ be done with query optimization or
indexes instead of duplicating data like this? This complexity carries
the risk of data getting out of sync in unforeseen ways. But this is
what I know how to do, and it seems to be working, so! Okay!
By default, Rails gives this button the name `commit`, so it appears in
the URL the form sends to. By setting the name to `nil`, Rails doesn't
set a `name` attribute on the HTML element, so it's *not* included.
The lists of pet types and pet states had very similar styles, which I
mostly copy-pasted. Now that I want to use them for Alt Styles too, I'm
refactoring!
Ah whoops, I didn't notice that, when Turbo morphs the
`<measured-container>` into what the server HTML returns, it deletes
the `style` attribute we were using.
In this change, I refactor for `MeasuredContainer` to be the component
rather than `MeasuredContent`, so that it can also be responsible for
listening for changes to its own `style` prop, and remeasuring when
they happen.
We're also careful to avoid infinite loops, by only doing this when the
property is missing! (Otherwise, setting `--natural-width` triggers the
callback again, oops!)
This hasn't worked for a while anyway! Let's remove the bits of code
where we deal with it, and the database field that signals it. (We also
make a corresponding change in Impress 2020, so it doesn't crash trying
to query based on the `prank` column.)
I also ran this snippet to clear out all the Nebula stuff in the db:
```rb
Color.transaction do
nebula = Color.where(prank: true).find_by_name("Nebula")
nebula.pet_types.includes(pet_states: :swf_assets).each do |pet_type|
pet_type.pet_states.each do |pet_state|
pet_state.parent_swf_asset_relationships.each do |psa|
psa.swf_asset.destroy!
psa.destroy!
end
pet_state.destroy!
end
pet_type.destroy!
end
nebula.destroy!
end
```
"Fall Woodland Leaves Filter" is an example, it's part of the two-item
*pack* named "Fall Woodland Minitheus Petpet Foreground". The NC Mall
page for it will include the secondary items in `object_data`, but it's
not part of the storefront itself—and the only thing indicating that is
the `render` list.
Theoretically, we could use this to construct more data about like,
packs and stuff, automatically? But also, I don't want to backfill it
for everything historically, so like. Whatever.
See comment for details! I wonder if other items have been affected by
this in the past. I think probably what happened before was that we
successfully created this item, but failed to create the *translation*,
so when migrating over the Patchwork Staff all its translated fields
were empty? (That's what I found looking in the database today.)
But yeah, thankfully our crash logging at health.openneo.net gave me
the name of a pet someone was trying to model, and so I was able to
find the bug and fix it!
Now that we have such a convenient lil outfit viewer component we built
for the item page preview, it's easy peasy to drop it in here too! And
it's all nice and lightweight, since in this case it's basically just.
image tags, with some supporting enhancements.
Anyway, this page has no actual useful styles of its own yet. Gonna
make it look nice and such!
I'm experimenting with a Rainbow Pool ish UI, mainly as a support tool
for exploring and labeling poses—but one we can probably just show to
real users too!
Right now, I just use pet type images as a placeholder, and I polished
up some of the `pet_type_image` API. But we're probably gonna drop
these for a full outfit viewer, now that I think of it.
This is a transitional gem to help with upgrading from old versions of
Rails: it provides a deprecated feature that Rails removed.
I audited and I *think* we only used it in one place, and that this one
place doesn't even use any of its functionality for styling or
scripting? So, begone!
Oh sweet, I learned about a new CSS feature with good-enough support!
This lets you use CSS transitions for an element as it enters the page,
or becomes visible.
Firefox only has partial support for this feature rn, but its partial
support covers our case, I tested to make sure! (Specifically, it
doesn't handle transitioning from `display: none` yet, which isn't what
we're doing.)
Whew, quite a history here! I didn't _extensively_ audit for these, but
I scanned with pretty good searches and hit major pages and they didn't
crash, so. Good enough for me!
Right, yeah, we've been depending on an external CDN for a long time
for jQuery and the jQuery Template library, and I don't like that kind
of external dependency! Let's put it in with the rest of our libs.
It's only actually used in two JS files, so rather than doing a weird
global `$.ajaxSetup` call, let's just inline it into the small handful
of AJAX calls that actually care.
When I was trying to debug slow view code one time long long ago, I was
like "let's cache any part of the template that's static!"
And like. no that's silly, I don't trust that this speeds anything up,
but it _definitely_ adds complexity. Let's just not.
Our production data now contains basic hashes for all species/color
combinations, and it's easy enough for a dev copy of the site to get
them too by running `rails public_data:pull`. So, I think it's time to
retire this hardcoded set, and get one more file out of our codebase!
The silly motivation is that I wanted to remove `.prettierignore`,
which just exists to omit that one folder from `npm run format`. But it
also seems like this is the standard place to put them—a standard
created long after we first set this up lol
I forget what this was for, I think part of it was for managing item
names in different languages, and the "private" locale thing was
probably for WIP locales? But yeah, not used, delete!
Yeah, I don't remember why So Many Years Ago I felt it was important to
use the Droid fonts; I adapted this choice into the Noto fonts when
modernizing the other day, but, tbh, the default system fonts are
probably just a better fit for like. everything we do, and then *not*
downloading MB of font files.
I also feel like a lot of the contexts where we used serif fonts were
like, frankly incidental, based on where we chose `<p>` for semantic
reasons? I don't think any of them actually are made much better by
serifs, I'm okay with just simplifying and dropping that, instead of
looking for a better serif font stack to replace it.
There's some funny bugs we had here, like "Relic Elephante Jewellery"
and "Royal Girl Skeith Bodice" getting assigned "Ice", and
"Tyrannian Meerca Spear" being "Pea" lmao
I went and checked all the assignments now and they look good to me!
```ruby
Item.is_pb.order(:name).
map { |i| [i.pb_color&.human_name, i.name] }
```
Ahh right, when you indent stuff underneath a tag in HAML, it does the
same indented form in the output HTML, which adds whitespace that
creates a problem for how we're doing this list.
Before this change, the "Engulfed in Flames Effect" item showed below
the preview: `Occupies: Background Item , Lower Foreground Item`, with
an extra space before the comma.
After this change, it now shows
`Occupies: Background Item, Lower Foreground Item`, as intended.
Fun little bug: viewing the "Engulfed in Flames Effect" item was
showing our "502 Bad Gateway" custom error page in the embed. This is
because the Rails app was providing a `Content-Security-Policy` header
value that was longer than nginx is configured by default to allow, so
it was refusing the response, and showing the same 502 error as if the
app hadn't responded at all. (We discovered this by opening
`/var/log/nginx/error.log`, which explained this very clearly, ty~!)
In this change, we no longer list every `images.neopets.com` asset,
instead marking the entire domain as a valid image source for the
SWF asset embed iframe. I don't _love_ this solution, I liked the
property of specifying literally exactly the assets we allow! But I
don't think there's any practical danger here, and it helps a *lot* for
making this more reliable.
(If we could have solved this reliably by increasing nginx's allowed
response header size, I probably would've done that? But I researched a
bit, and ultimately concluded that I don't trust other intermediary
software like firewalls not to have the same issue. Let's not be
pushing the limits of HTTP headers of all things!)
This shouldn't ever be an issue in practice? I just noticed it because
something funny is going on with the `#userbar` element specifically
not using the Delicious font, and so I figured, hey, this simulates a
very real possible scenario, I'd rather use our consistent sans font
in this case!
Huh okay, moving to my other machine, the change to Noto Sans subtly
broke the homepage layout a bit, wrapping the form buttons to the next
line in the three module sections.
Here, I refactor to more modern grid/flexbox sensibilities. Btw, there
was a Flexbox thing that didn't work quite how I expected? I commented
on my confusion, but checked in Chrome and Firefox and it seems to work
in both, so, ok!
Okay cool, so this was an error that was happening *only* when building
assets for production: Sass's CSS minifier isn't familiar with all
modern CSS syntax (I think is the issue?), and so errors on things that
are actually totally okay.
I had previously worked around this in `swf_assets/show.css` with an
equivalent syntax that Sass recognized. But in this latest case with
the new `fonts.css.erb`, it was upset about the `src` list for the
fonts, and I don't know a workaround for that.
So, let's just disable Sass's CSS minification for now. I imagine the
difference isn't huge when CSS compresses just fine with gzip anyway?
(Most of what you can "minify" in CSS is whitespace, and that largely
seems silly to me when gzip is running.)
I was just scrolling our CSS and surprised to find we use Google Fonts
embeds! I don't like depending on external hosts like that.
Google Fonts doesn't offer the Droid fonts for download anymore,
though—looks like the Noto fonts are their spiritual successor. The
Droid Serif and Noto Serif fonts look visually identical to me, but the
Sans ones are a bit different… I kinda like the charm of the Droid Sans
better, but ah well! I'd rather be moving forward with a more modern
font with more reliable glyph support etc for now.
I think this has just been broken for a long time? And I don't think
it's very useful in a world 15 years later, where our problem *used* to
be giant gaps in our library, which isn't really our data problem
anymore.
No more of this loading everything into `application.css`! I'm
arbitrarily starting here because that's where I've been playing
lately, but this is part of a larger effort to move toward a more
straightforward CSS architecture (and away from Sass even?)
Ran into a funny thing on an upcoming change, where a style on the
page-specific stylesheet was getting undone by the *CSS Reset* of all
things in the application stylesheet. Resets come first!
I haven't audited that I didn't break a ton of stuff with this change,
but. I hope not! :)
Closes#3, by adapting the suggested changes! Thank you!!
We both change how we create pet name preview jobs, by catching the `@`
case early; and we better handle symbols in pet names when showing the
thank you message, by parsing the query string more correctly.
Co-Authored-By: Steve C <diceroll123@gmail.com>
This was always modeling correctly, but not showing the message,
because Turbo doesn't handle anchors in redirect URLs the same way the
browser's full page loads do.
I forget why we had this as a `#` URL anyway to begin with. Use `?`
instead!
I haven't been running Prettier consistently on things in this project.
Now, it's quick-runnable, and I've got it on everything!
Also, I just think tabs are the right default for this kind of thing,
and I'm glad to get to switch over to it! (In `package.json`.)
I skipped this for a bit because I couldn't think of a simple way to
adapt this behavior to a web component + vanilla CSS setting, but then
I thought of CSS variables, and sat down and cranked this out!
I noticed the last row of the species faces required a scroll, I forget
when that happened! But I made some tweaks, most notably widened the
container from the normal 800px, so that on bigger screens everything
lays out and aligns nice, without requiring any scrolling of the face
container!
Oh oops, I forgot one of the kinds of restricted zones when refactoring
how we load search data in wardrobe-2020! This made most items with
restricted zones (like Be Gone items) not work correctly when you
search for them to add them to the item—though it *does* work correctly
when you reload the page or change the species, to get to load a
different way.
If something goes wrong, like the site goes down or has an intermittent
error, try a full pageload. That way, we're both retrying, and in a way
that gives the user more control and visibility into what's going on,
and what they can potentially do about it. (e.g. if there's a useful
error message, they will see it!)
I took this ordering from a specific place on Impress 2020, but I think
that was in a context where the pose mattered more? Here though, I'm
realizing that I'd rather show any known-unglitched pose than the happy
masc or whatever we semi-randomly chose.
instead of doing the random choice we do for most colors.
This is especially noticeable in cases where like, I'm looking at the
Elderlyboy Ogrin and like, it has *work* put into the masc eyes, and
them fem eyes are just the standard ones.
When playing with a Rainbow Pool syncing task, I noticed that error
handling wasn't working correctly for requests using `async-http`: if
the block raised an error, the `Sync` block would never return.
My suspicion is that this is because we were never reading or releasing
the request body.
In this change, I upgrade all the relevant gems for good measure, and
switch to using the response object yielded by the _block_, so we can
know it's being resource-managed correctly. Now, failures raise errors
as expected!
(I tested all these relevant service calls, too!)
Huh, this is a bit odd, I think we took this from Impress 2020's
`canonicalPetStateForBodyLoader` SQL query… but actually, it doesn't
really make sense? and `petStatesForPetTypeLoader` has a more sensible
ordering, and is the one the app uses in more ways. Maybe that's a
mistake we made back then, or a bug we fixed only in one place?
Anyway, this fixes why the item previews were like. using a LOT of
glitched pet states and I was like "dang did a lot of them break
recently?"
Nah we were just. not pulling the right ones lol
Oh right, forgot about this lol!
The specific effect on Impress 2020 where the button label expands is,
kinda hard to implement in normal CSS/JS, and so I'm not in the mood
and I'm settling for the `title` attribute lol
Oh right, I need the error indicator to be part of a container that
also contains the outfit viewer, to appear below it!
I was motivated because I realized I forgot the Customize More button
so now I'm building it lol
The loading indicator *should* fade in after two seconds, to avoid a
flash of a loading indicator when the page loads quickly - but in some
circumstances it wouldn't delay:
1. Visit an item page. (It delays correctly the first time!)
2. Click "Infinite Closet", then click a link to another item page.
3. The loading indicator appears immediately, because this time the
web component JS is already loaded, so the `outfit-layer` elements
enter `:state(loading)` *immediately*. The element starts at
`opacity: 1`, and the delay doesn't matter, because it was never at
anything else.
In this change, we have the `outfit-viewer` web component take on a
`:state(after-first-frame)`, after a `setTimeout(0)` resolves. That
enables the loading state CSS to *never* apply on the first frame, but
then sometimes kick in on the *second* frame, so that the element is
correctly perceived as "transitioning" from hidden to visible, and the
two-second delay will apply.
When I run `bin/deploy:precompile` on the previous version, I get an
error from libsass that `vw` and `vh` are incompatible units. I don't
get this error in development, only when compiling for production.
My inference is:
1. For the production build, Sass is trying to preprocess even non-SASS
files, maybe to help minify them?
2. In Sass, their `min()` existed before CSS's `min()`, so it's
treating it Like That, and returning a reasonable-in-some-cases-but-
not-here error that `min(100vw, 100vh)` can't be *precomputed*.
Anyway, wrapping it in `calc()` isn't a *problem*, and helps the Sass
compiler not try to precompute it, so. Okay!
https://github.com/sass/node-sass/issues/2815#issuecomment-575926329
We call it enough times on this page, and it *does* have a SQL query,
that I want to cache it! (Also I want to make it fewer species queries
if I can tbh…)
For static image layers, this was *always* logging that we failed to
send the frame a "pause" message. Which, like, of course!
It makes sense to log the notable circumstance where we send a message
we *expect* to arrive, but the frame isn't loaded yet. But if there's
just no frame, ignore it and don't bother to say so.
We add a new `use_responsive_design` helper, for pages to opt into this
new CSS—mostly just because like… it's *worse* to apply these styles
for pages that don't expect it 😅
And then, I fix up a couple things on the item page (including in the
general items layout) to match!
I'm doing this because the species face picker layout is going to want
some responsive awareness, and I want to be doing that from the start!
Here, I remember the trick I learned when building the outfit viewer:
web components are great for making sure stuff stays initialized well
in a Turbo environment!
The problem was, after submitting the form and getting a new preview
loaded via Turbo, the part where we remove `inert` would get undone.
Additionally, this script only loads *once* per session, so if you
Turbo-nav to a different item then that part of the page never ran.
Instead, we use web components to remove the attributes on mount, then
again if they're ever reapplied by Idiomorph.
We mark the options as `inert` and `aria-hidden` while the JS is still
loading—and if the `noscript` tag tells us it's never coming, it covers
up the picker with a brief explainer!
The basics are working great! There's a few known missing things though:
- Add reasonable noscript behavior
- Disable options where there's no valid appearance
- Lay it out actually _good_, instead of just images dumped there
Adapting what the Impress 2020 UI does, but in Ruby instead!
I feel like this is case is really starting to show the power of doing
this stuff in Rails instead of via an API… we can *really* take
advantage of our models and our handy idioms at all points. This is
just so much less *code* than this feature takes in Node + GraphQL +
React.
We used to use this to determine what color to show by default on the
item page preview for, like, Maraquan-specific items. Now, we infer it
from our actual customization data, rather than these heuristics!
There's still a database field for `Item#manual_special_color_id`. We
can still read and write this from the support UI, and Impress 2020
still slightly uses it from the homepage, so I'm not removing from the
database right now.
The `build_on_pet_types` helper used to be reused on the items page, to
generate the list of species to display. We don't use it anymore, so
simplify and remove!
I'm about to reimplement the more-robust version of what this used to
be: how the item page used to say "sometimes" after certain zones in
the occupied list.
Now, we're going to do parity with 2020, and list the actual species!
I like that this takes away the weird `#sometimes` method on the `Zone`
class, which was always an odd hack for just this small thing.
We had this issue on Impress 2020 and I fixed it over there too. I guess
it went less noticed here on Classic, because it's a more
progressively-enhanced site in general (and this failure case is an
interesting argument for that architecture! lol).
On Impress 2020, I wasn't sure if the "waits for the document to load"
behavior of the `defer` attribute was necessary to the script, so I
chose to keep `defer` but move it _after_ the other scripts.
This time, I dug in a bit more, and found a Plausible author saying
that the choice was kinda arbitrary; and another person who had the
same issue as me, who said they switched to `async` and it worked well.
So, that's what we're doing now, too!
https://github.com/plausible/analytics/discussions/1907#discussioncomment-2754499
Closes#2, after making some tweaks to the PR to fit how JS templating
works here. Thanks @dice!!
I had to move `petThumbnailUrl` out of the closure, because this script
does a cute thing of having separate variable scopes for the separate
areas of the page—but this is used by two of them. Arguably it could
make sense to like, put this all in one larger shared IIFE closure that
wraps both of them, to preserve some of this code's intention of
avoiding adding to the global namespace on this page, but like.
*It's fine.*
Co-Authored-By: Steve C <diceroll123@gmail.com>
