This used to be the behavior, and the site has plenty of graceful
fallbacks for it, I just forgot this one when doing Rails upgrades!
Note that the impress-2020 stuff is *not* as graceful about this, so
the wardrobe page won't show the pet until the color is in the DB. Ah
well, still an improvement!
Okay right, the wardrobe-2020 app treats `state` as a bit of an
override thing, and `pose` is the main canonical field for how a pet
looks. We were missing a few pieces here:
1. After loading a pet, we weren't including the `pose` field in the
initial query string for the wardrobe URL, but we _were_ including
the `state` field, so the outfit would get set up with a conflicting
pet state ID vs pose.
2. When saving an outfit, we weren't taking the `state` field into
account at all. This could cause the saved outfit to not quite match
how it actually looked in-app, because the default pet state for
that species/color/pose trio could be different; and regardless, the
outfit state would come back with `appearanceId` set to `null`,
which wouldn't match the local outfit state, which would trigger an
infinite loop.
Here, we complete the round-trip of the `state` field, from pet loading
to outfit saving to the outfit data that comes back after saving!
Now that DTI 2020 has been deployed without references to the
translations tables, we can stop keeping them in sync!
Next step is to drop the tables and be done with them altogether! (I
have a backup of the public data for this too, as does this repo!)
Two motivations here:
1. Unconverted pets should no longer exist on Neopets.com (and we
especially don't expect new ones), so this logic helps no one.
2. The Baby Pteri keeps getting overridden to be marked as Unconverted
because it has only one asset, which is incorrect.
A decent heuristic for a bygone era, goodbye!
Yay it works(*)! But two major missing pieces:
- Outfit saving doesn't persist it at all
- Item compatibility is unaffected: items will still appear in search
and in the preview, even when they don't fit anymore.
To help with space, I'm just showing the word "Nostalgic" (or "???" if
it's from a series we don't recognize, this is hardcoded by ID), and
trusting that from context it will be obvious that it's the "Nostalgic
Faerie" case or whatever. (Moreover, in both the button and the select
we're omitting the species name, by similar reasoning!)
Note that this _still_ doesn't actually apply the style to the outfit
whatsoever; this is all just local state as we're continuing to play
with UI concepts. Actually applying it is probably next though! (Though
there's a couple more UI things I want to do, like some affordances to
clarify that a Style is applied and that Expression changes won't work.)
Something in the Rails loader doesn't like that I have both a gem and
a lib folder named `RocketAMF`, I think? It'll often work for the first
pet load request, then on subsequent ones say `Envelope` is not
defined, I'm guessing because it scrapped the gem's module in favor of
mine?
Idk, let's just simplify all this by making our own module. I feel like
this old lib could use an overhaul and simplification anyway, but this
will do for now!
Like in 0dca538, this is preliminary work for being able to drop the
`zone_translations` table! We're copying the field over first, to be
able to migrate DTI 2020 safely before dropping anything.
Non-English languages haven't been supported on Neopets for a while, so
I'd like to remove this extra cross-cutting complexity, especially
since it's now inconsistent with the real site anyway!
The main motivation is that I'd like to do this for items too, because
I have a hunch that all the complexity of `globalize` to read
`item.name` is part of what's making large user lists so slow to
render: lots of little objects getting created down the stack, and
needing to be garbage-collected later.
I'm not sure that's why! But I figure removing this complexity is a
simplicity win anyway, so let's do it!
Note that this doesn't *finish* the migration, it just starts it! The
`Species::Translation` and `Color::Translation` models still exist, and
still have their data, and not all references to them are scrubbed yet.
I especially don't want to delete the backing tables until both DTI and
DTI 2020 are ready for it!
So this change will someday be paired with another change to actually
drop the tables - after backing up the data for future records just in
case, of course!
If your first wanted list was created before your first owned list,
then `false` would come before `true` in the keys of
`current_user_lists`.
I both fixed this to be more consistent at the model level, because who
likes unpredictable behavior? But also downstream at the view I
hardcoded that true should come before false, because that's a UI
concern that I want to be encoded in the view regardless of what's
upstream.
It was a bit tricky to figure out the right API for this, since I'm
looking ahead to the possibility of splitting these across multiple
pages with more detail, like we do in DTI 2020.
What I like about this API is that the caller gets to apply, or not
apply, whatever scopes they want to the underlying hanger set (like
`includes` or `order`), without violating the usual syntax by e.g.
passing it as a parameter to a method.
Ahh I see, the way we got away with not having a `trading` scope before
was a weird metaprogramming `{owned/wanted}_trading` situation. Okay,
let's trash that in favor of our new stuff! And that helps us bulk the
queries too which is nice.
In impress-2020, we do a big slow query to figure out which users have
been active in trades recently. Now, we cache that timestamp on the
User model.
This won't have any immediate effect; it's to clear the way for Classic
DTI to receive the better trade ratios feature people like from 2020.
I also added some unit testing infra because I finally wanted it! for
all the ways you can trigger this timestamp lol
Note too that this is a bit of an unusually complex migration, but my
hope is that the batching and query structure and such helps it run
surprisingly fast! 🤞
So this was a slightly wrong error message, what was happening was:
1. Trying to load the image hash for this pet, by looking them up at
https://pets.neopets.com/cpn/PET_NAME/1/1.png and seeing what URL it
redirects to.
2. But pets.neopets.com was rejecting our User-Agent string, which
would've been just "Ruby", since we hadn't set it otherwise. I guess
that's an explicitly banned string?
I also found that the kind of more-helpful User-Agent string I like to
write was being rejected, and I could only get it to accept something
very simple? So that's what we're using now, I guess!!
Building toward replacing more of the 2020 data sources! I think this is
an endpoint that benefits from bulk loading, esp with the way the item
page previews work. I also like taking the concept of "canonical" out of
the GQL interface, and instead just loading for each of the 50 species
and letting the client decide. (And then it can fast-swap between them!)
The models folder is a bit confusingly large, these are more mixins and
kinda clutter it. Push them off into `lib`, I think!
I think they used to be in models mainly because Rails used to handle
`lib` differently with autoloading, and it made for a worse dev
experience. Now it's all the same, though!
We haven't used the mall spider in this app in forever (I guess we even
deleted the code at some point?), but there was some vestigial stuff
left. Goodbye!
Just moving more stuff over! I modernized Item's `as_json` method while
I was here. (Note that I removed the NC/own/want fields, because I
think the only other place this method is still called from is the
quick-add feature on the closet lists page, and I think it doesn't use
these fields to do anything: updating the page is basically a full-page
reload, done sneakily.)
There was a time when I used an old proxy server to try to fix mixed
content issues, and I eventually removed it but never took the tendrils
out from the code.
We probably _should_ figure out how to secure these URLs! But until
then, we may as well simplify the code.
I changed my mind again! At first I wanted to make the special case
clearer, and to be able to more strongly assert that the species is
not null. But now I'm like… eh, there's code that references `body.id`
that has no reason _not_ to work in the all-bodies case… let's just
keep the types more consistent, I think.
