From 010f64264f86d8c22ef0f7e98696e8409449154b Mon Sep 17 00:00:00 2001 From: Matchu Date: Sat, 11 Nov 2023 07:14:48 -0800 Subject: [PATCH 1/9] Replace swf_assets#index with item_appearances#index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preparing a better endpoint for wardrobe-2020 to use! I deleted the now-unused swf_assets#index endpoint, and replaced it with an "appearances" concept that isn't exactly reflected in the database models but is a _lot_ easier for clients to work with imo. Note that this was a big part of the motivation for the recent `manifest_url` work—in this draft, I'm probably gonna have the client request the manifest, rather than use impress-2020's trick of caching it in the database! There's a bit of a perf penalty, but I think that's a simpler starting point, and I have a hunch I'll be able to make up the perf difference once we have the impress-media-server managing more of these responsibilities. --- .../item_appearances_controller.rb | 6 ++ app/controllers/swf_assets_controller.rb | 56 ------------------- app/models/item.rb | 33 +++++++---- app/models/swf_asset.rb | 38 ++++++++----- app/models/zone.rb | 4 ++ config/routes.rb | 11 +--- 6 files changed, 58 insertions(+), 90 deletions(-) create mode 100644 app/controllers/item_appearances_controller.rb diff --git a/app/controllers/item_appearances_controller.rb b/app/controllers/item_appearances_controller.rb new file mode 100644 index 00000000..be8edabb --- /dev/null +++ b/app/controllers/item_appearances_controller.rb @@ -0,0 +1,6 @@ +class ItemAppearancesController < ApplicationController + def index + @item = Item.find(params[:item_id]) + render json: @item.appearances + end +end diff --git a/app/controllers/swf_assets_controller.rb b/app/controllers/swf_assets_controller.rb index 8713837f..9229c207 100644 --- a/app/controllers/swf_assets_controller.rb +++ b/app/controllers/swf_assets_controller.rb @@ -1,60 +1,4 @@ class SwfAssetsController < ApplicationController - def index - if params[:item_id] - item = Item.find(params[:item_id]) - @swf_assets = item.swf_assets.includes_depth - if params[:body_id] - @swf_assets = @swf_assets.fitting_body_id(params[:body_id]) - else - if item.special_color - @swf_assets = @swf_assets.fitting_color(item.special_color) - else - @swf_assets = @swf_assets.fitting_standard_body_ids - end - json = @swf_assets.all.group_by(&:body_id) - end - elsif params[:pet_type_id] && params[:item_ids] - pet_type = PetType.find(params[:pet_type_id]) - - @swf_assets = SwfAsset.object_assets.includes_depth. - fitting_body_id(pet_type.body_id). - for_item_ids(params[:item_ids]). - with_parent_ids - json = @swf_assets.map { |a| a.as_json(:parent_id => a.parent_id.to_i, :for => 'wardrobe') } - elsif params[:pet_state_id] - @swf_assets = PetState.find(params[:pet_state_id]).swf_assets. - includes_depth.all - pet_state_id = params[:pet_state_id].to_i - json = @swf_assets.map { |a| a.as_json(:parent_id => pet_state_id, :for => 'wardrobe') } - elsif params[:pet_type_id] - @swf_assets = PetType.find(params[:pet_type_id]).pet_states.emotion_order - .first.swf_assets.includes_depth - elsif params[:ids] - @swf_assets = [] - if params[:ids][:biology] - @swf_assets += SwfAsset.includes_depth.biology_assets.where(:remote_id => params[:ids][:biology]).all - end - if params[:ids][:object] - @swf_assets += SwfAsset.includes_depth.object_assets.where(:remote_id => params[:ids][:object]).all - end - elsif params[:body_id] && params[:item_ids] - # DEPRECATED in favor of pet_type_id and item_ids - swf_assets = SwfAsset.arel_table - @swf_assets = SwfAsset.includes_depth.object_assets. - select('swf_assets.*, parents_swf_assets.parent_id'). - fitting_body_id(params[:body_id]). - for_item_ids(params[:item_ids]) - json = @swf_assets.map { |a| a.as_json(:parent_id => a.parent_id.to_i, :for => 'wardrobe') } - end - if @swf_assets - @swf_assets = @swf_assets.all unless @swf_assets.is_a? Array - json = @swf_assets unless json - else - json = nil - end - render :json => json - end - def show @swf_asset = SwfAsset.find params[:id] render :json => @swf_asset diff --git a/app/models/item.rb b/app/models/item.rb index a842e53e..fff00c66 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -511,17 +511,30 @@ class Item < ApplicationRecord def parent_swf_asset_relationships_to_update=(rels) @parent_swf_asset_relationships_to_update = rels end - - def needed_translations - translatable_locales = Set.new(I18n.locales_with_neopets_language_code) - translated_locales = Set.new(translations.map(&:locale)) - translatable_locales - translated_locales - end - def method_cached?(method_name) - # No methods are cached on a full item. This is for duck typing with item - # proxies. - false + Appearance = Struct.new(:body_id, :swf_assets) + def appearances + all_swf_assets = swf_assets.to_a + + # If there are no assets yet, there are no appearances. + return [] if all_swf_assets.empty? + + # Get all SWF assets, and separate the ones that fit everyone (body_id=0). + swf_assets_by_body_id = all_swf_assets.group_by(&:body_id) + swf_assets_for_all_bodies = swf_assets_by_body_id.delete(0) || [] + + # If there are no body-specific assets, return one appearance for them all. + if swf_assets_by_body_id.empty? + return Appearance.new(0, swf_assets_for_all_bodies) + end + + # Otherwise, create an appearance for each real (nonzero) body ID. We don't + # generally expect body_id = 0 and body_id != 0 to mix, but if they do, + # uhh, let's merge the body_id = 0 ones in? + swf_assets_by_body_id.map do |body_id, body_specific_assets| + swf_assets_for_body = body_specific_assets + swf_assets_for_all_bodies + Appearance.new(body_id, swf_assets_for_body) + end end def self.all_by_ids_or_children(ids, swf_assets) diff --git a/app/models/swf_asset.rb b/app/models/swf_asset.rb index dc8a763c..89179685 100644 --- a/app/models/swf_asset.rb +++ b/app/models/swf_asset.rb @@ -96,22 +96,30 @@ class SwfAsset < ApplicationRecord end def as_json(options={}) - json = { - :id => remote_id, - :type => type, - :depth => depth, - :body_id => body_id, - :zone_id => zone_id, - :zones_restrict => zones_restrict, - :is_body_specific => body_specific?, - # Now that we don't proactively convert images anymore, let's just always - # say `has_image: true` when sending data to the frontend, so it'll use the - # new URLs anyway! - :has_image => true, - :images => images + super({ + only: [:id], + methods: [:zone, :restricted_zones, :urls] + }.merge(options)) + end + + def urls + { + swf: url, + png: image_url, + manifest: manifest_url, } - json[:parent_id] = options[:parent_id] if options[:parent_id] - json + end + + def restricted_zone_ids + [].tap do |ids| + zones_restrict.chars.each_with_index do |bit, index| + ids << index + 1 if bit == "1" + end + end + end + + def restricted_zones + Zone.where(id: restricted_zone_ids) end def body_specific? diff --git a/app/models/zone.rb b/app/models/zone.rb index 887fba03..cf4050d1 100644 --- a/app/models/zone.rb +++ b/app/models/zone.rb @@ -18,6 +18,10 @@ class Zone < ActiveRecord::Base } scope :for_items, -> { where(arel_table[:type_id].gt(1)) } + def as_json(options={}) + super({only: [:id, :depth, :label]}.merge(options)) + end + def uncertain_label @sometimes ? "#{label} sometimes" : label end diff --git a/config/routes.rb b/config/routes.rb index 0ed310c7..3d02b1e6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -7,25 +7,18 @@ OpenneoImpressItems::Application.routes.draw do get '/wardrobe' => redirect('/outfits/new') get '/start/:color_name/:species_name' => 'outfits#start' - # DEPRECATED - get '/bodies/:body_id/swf_assets.json' => 'swf_assets#index', :as => :body_swf_assets - - get '/items/:item_id/swf_assets.json' => 'swf_assets#index', :as => :item_swf_assets - get '/items/:item_id/bodies/:body_id/swf_assets.json' => 'swf_assets#index', :as => :item_swf_assets_for_body_id - get '/pet_types/:pet_type_id/swf_assets.json' => 'swf_assets#index', :as => :pet_type_swf_assets - get '/pet_types/:pet_type_id/items/swf_assets.json' => 'swf_assets#index', :as => :item_swf_assets_for_pet_type - get '/pet_states/:pet_state_id/swf_assets.json' => 'swf_assets#index', :as => :pet_state_swf_assets get '/species/:species_id/color/:color_id/pet_type.json' => 'pet_types#show' resources :contributions, :only => [:index] resources :items, :only => [:index, :show] do + resources :appearances, controller: 'item_appearances', only: [:index] collection do get :needed end end resources :outfits, :only => [:show, :create, :update, :destroy] resources :pet_attributes, :only => [:index] - resources :swf_assets, :only => [:index, :show] do + resources :swf_assets, :only => [:show] do collection do get :links end From 6c31094c2d7b1fa5f9307a48b8edaafbc469efee Mon Sep 17 00:00:00 2001 From: Matchu Date: Sat, 11 Nov 2023 07:21:13 -0800 Subject: [PATCH 2/9] Add known_glitches to appearances JSON --- app/models/swf_asset.rb | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/app/models/swf_asset.rb b/app/models/swf_asset.rb index 89179685..67af14a6 100644 --- a/app/models/swf_asset.rb +++ b/app/models/swf_asset.rb @@ -97,7 +97,7 @@ class SwfAsset < ApplicationRecord def as_json(options={}) super({ - only: [:id], + only: [:id, :known_glitches], methods: [:zone, :restricted_zones, :urls] }.merge(options)) end @@ -110,6 +110,17 @@ class SwfAsset < ApplicationRecord } end + def known_glitches + self[:known_glitches].split(',') + end + + def known_glitches=(new_known_glitches) + if new_known_glitches.is_a? Array + new_known_glitches = new_known_glitches.join(',') + end + self[:known_glitches] = new_known_glitches + end + def restricted_zone_ids [].tap do |ids| zones_restrict.chars.each_with_index do |bit, index| From 133895b213121b4f1a2409c2f6a1058658a1f976 Mon Sep 17 00:00:00 2001 From: Matchu Date: Sat, 11 Nov 2023 07:54:56 -0800 Subject: [PATCH 3/9] Refactor appearances data to use a "body" object This is more similar to what impress-2020 does, I was working on the wardrobe-2020 code and took some inspiration! The body has an ID and a species, or is the string "all". --- app/models/item.rb | 9 ++++++--- app/models/species.rb | 8 +++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/models/item.rb b/app/models/item.rb index fff00c66..bb74e42c 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -512,7 +512,8 @@ class Item < ApplicationRecord @parent_swf_asset_relationships_to_update = rels end - Appearance = Struct.new(:body_id, :swf_assets) + Body = Struct.new(:id, :species) + Appearance = Struct.new(:body, :swf_assets) def appearances all_swf_assets = swf_assets.to_a @@ -525,7 +526,7 @@ class Item < ApplicationRecord # If there are no body-specific assets, return one appearance for them all. if swf_assets_by_body_id.empty? - return Appearance.new(0, swf_assets_for_all_bodies) + return Appearance.new(:all, swf_assets_for_all_bodies) end # Otherwise, create an appearance for each real (nonzero) body ID. We don't @@ -533,7 +534,9 @@ class Item < ApplicationRecord # uhh, let's merge the body_id = 0 ones in? swf_assets_by_body_id.map do |body_id, body_specific_assets| swf_assets_for_body = body_specific_assets + swf_assets_for_all_bodies - Appearance.new(body_id, swf_assets_for_body) + species = Species.with_body_id(body_id).first! + body = Body.new(body_id, species) + Appearance.new(body, swf_assets_for_body) end end diff --git a/app/models/species.rb b/app/models/species.rb index 2bbbbbc0..d20e1726 100644 --- a/app/models/species.rb +++ b/app/models/species.rb @@ -1,5 +1,6 @@ class Species < ApplicationRecord translates :name + has_many :pet_types scope :alphabetical, -> { st = Species::Translation.arel_table @@ -12,6 +13,11 @@ class Species < ApplicationRecord where(st[:name].matches(sanitize_sql_like(name))) } + scope :with_body_id, -> body_id { + pt = PetType.arel_table + joins(:pet_types).where(pt[:body_id].eq(body_id)).limit(1) + } + # TODO: Should we consider replacing this at call sites? This used to be # built into the globalize gem but isn't anymore! def self.find_by_name(name) @@ -19,7 +25,7 @@ class Species < ApplicationRecord end def as_json(options={}) - {:id => id, :name => human_name} + super({only: [:id, :name], methods: [:human_name]}.merge(options)) end def human_name From eaab0b1922fe7db56a7e9048d812a23f699ca2df Mon Sep 17 00:00:00 2001 From: Matchu Date: Sat, 11 Nov 2023 07:58:50 -0800 Subject: [PATCH 4/9] Oops, return an array in the all-bodies case I was returning just the body itself, oops! For consistency with the general return value, it should be a one-element array. --- app/models/item.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/item.rb b/app/models/item.rb index bb74e42c..4cbcb324 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -526,7 +526,7 @@ class Item < ApplicationRecord # If there are no body-specific assets, return one appearance for them all. if swf_assets_by_body_id.empty? - return Appearance.new(:all, swf_assets_for_all_bodies) + return [Appearance.new(:all, swf_assets_for_all_bodies)] end # Otherwise, create an appearance for each real (nonzero) body ID. We don't From fe6264b20af818496198f2e13f74bcdc31fcf227 Mon Sep 17 00:00:00 2001 From: Matchu Date: Sat, 11 Nov 2023 08:13:07 -0800 Subject: [PATCH 5/9] Use an actual body with ID "0", instead of the string "all" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- app/models/item.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/item.rb b/app/models/item.rb index 4cbcb324..18e9a5b2 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -526,7 +526,8 @@ class Item < ApplicationRecord # If there are no body-specific assets, return one appearance for them all. if swf_assets_by_body_id.empty? - return [Appearance.new(:all, swf_assets_for_all_bodies)] + body = Body.new(0, nil) + return [Appearance.new(body, swf_assets_for_all_bodies)] end # Otherwise, create an appearance for each real (nonzero) body ID. We don't From 5e25e0bda6f93d1fdc8930a0ee7e2217bda918c0 Mon Sep 17 00:00:00 2001 From: Matchu Date: Sat, 11 Nov 2023 08:15:10 -0800 Subject: [PATCH 6/9] Load item compatibility data from the Rails app, not impress-2020 Ok progress! We've moved the info about what bodies this fits, and what zones it occupies, into a Rails API endpoint that we now load at the same time as the other data! We'll keep moving more over, too! --- .../ItemPage/SpeciesFacesPicker.js | 2 +- .../wardrobe-2020/ItemPageOutfitPreview.js | 52 ++++++++--------- app/javascript/wardrobe-2020/loaders/items.js | 57 +++++++++++++++++++ .../wardrobe-2020/loaders/outfits.js | 2 +- 4 files changed, 82 insertions(+), 31 deletions(-) create mode 100644 app/javascript/wardrobe-2020/loaders/items.js diff --git a/app/javascript/wardrobe-2020/ItemPage/SpeciesFacesPicker.js b/app/javascript/wardrobe-2020/ItemPage/SpeciesFacesPicker.js index 57496ee0..8838ed68 100644 --- a/app/javascript/wardrobe-2020/ItemPage/SpeciesFacesPicker.js +++ b/app/javascript/wardrobe-2020/ItemPage/SpeciesFacesPicker.js @@ -60,7 +60,7 @@ function SpeciesFacesPicker({ ); const allBodiesAreCompatible = compatibleBodies.some( - (body) => body.representsAllBodies, + (body) => body.id === "0", ); const compatibleBodyIds = compatibleBodies.map((body) => body.id); diff --git a/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js b/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js index 5e83ea20..e85b55ab 100644 --- a/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js +++ b/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js @@ -30,6 +30,7 @@ import { } from "./components/useOutfitAppearance"; import { useOutfitPreview } from "./components/OutfitPreview"; import { logAndCapture, useLocalStorage } from "./util"; +import { useItemAppearances } from "./loaders/items"; function ItemPageOutfitPreview({ itemId }) { const idealPose = React.useMemo( @@ -99,6 +100,13 @@ function ItemPageOutfitPreview({ itemId }) { const [initialPreferredSpeciesId] = React.useState(preferredSpeciesId); const [initialPreferredColorId] = React.useState(preferredColorId); + const { + data: itemAppearancesData, + loading: loadingAppearances, + error: errorAppearances, + } = useItemAppearances(itemId); + const itemAppearances = itemAppearancesData || []; + // Start by loading the "canonical" pet and item appearance for the outfit // preview. We'll use this to initialize both the preview and the picker. // @@ -128,20 +136,6 @@ function ItemPageOutfitPreview({ itemId }) { id label } - compatibleBodiesAndTheirZones { - body { - id - representsAllBodies - species { - id - name - } - } - zones { - id - label - } - } canonicalAppearance( preferredSpeciesId: $preferredSpeciesId preferredColorId: $preferredColorId @@ -192,10 +186,7 @@ function ItemPageOutfitPreview({ itemId }) { }, ); - const compatibleBodies = - data?.item?.compatibleBodiesAndTheirZones?.map(({ body }) => body) || []; - const compatibleBodiesAndTheirZones = - data?.item?.compatibleBodiesAndTheirZones || []; + const compatibleBodies = itemAppearances?.map(({ body }) => body) || []; // If there's only one compatible body, and the canonical species's name // appears in the item name, then this is probably a species-specific item, @@ -203,10 +194,12 @@ function ItemPageOutfitPreview({ itemId }) { // model it. const isProbablySpeciesSpecific = compatibleBodies.length === 1 && - !compatibleBodies[0].representsAllBodies && - (data?.item?.name || "").includes( - data?.item?.canonicalAppearance?.body?.canonicalAppearance?.species?.name, - ); + compatibleBodies[0] !== "all" && + (data?.item?.name || "") + .toLowerCase() + .includes( + data?.item?.canonicalAppearance?.body?.canonicalAppearance?.species?.name.toLowerCase(), + ); const couldProbablyModelMoreData = !isProbablySpeciesSpecific; // TODO: Does this double-trigger the HTTP request with SpeciesColorPicker? @@ -257,7 +250,7 @@ function ItemPageOutfitPreview({ itemId }) { const borderColor = useColorModeValue("green.700", "green.400"); const errorColor = useColorModeValue("red.600", "red.400"); - const error = errorGQL || errorValids; + const error = errorGQL || errorAppearances || errorValids; if (error) { return {error.message}; } @@ -350,6 +343,7 @@ function ItemPageOutfitPreview({ itemId }) { // Wait for us to start _requesting_ the appearance, and _then_ // for it to load, and _then_ check compatibility. !loadingGQL && + !loadingAppearances && !appearance.loading && petState.isValid && !isCompatible && ( @@ -386,13 +380,13 @@ function ItemPageOutfitPreview({ itemId }) { compatibleBodies={compatibleBodies} couldProbablyModelMoreData={couldProbablyModelMoreData} onChange={onChange} - isLoading={loadingGQL || loadingValids} + isLoading={loadingGQL || loadingAppearances || loadingValids} /> - {compatibleBodiesAndTheirZones.length > 0 && ( + {itemAppearances.length > 0 && ( )} @@ -526,13 +520,13 @@ function PlayPauseButton({ isPaused, onClick }) { ); } -function ItemZonesInfo({ compatibleBodiesAndTheirZones, restrictedZones }) { +function ItemZonesInfo({ itemAppearances, restrictedZones }) { // Reorganize the body-and-zones data, into zone-and-bodies data. Also, we're // merging zones with the same label, because that's how user-facing zone UI // generally works! const zoneLabelsAndTheirBodiesMap = {}; - for (const { body, zones } of compatibleBodiesAndTheirZones) { - for (const zone of zones) { + for (const { body, swfAssets } of itemAppearances) { + for (const { zone } of swfAssets) { if (!zoneLabelsAndTheirBodiesMap[zone.label]) { zoneLabelsAndTheirBodiesMap[zone.label] = { zoneLabel: zone.label, diff --git a/app/javascript/wardrobe-2020/loaders/items.js b/app/javascript/wardrobe-2020/loaders/items.js new file mode 100644 index 00000000..fb12f70e --- /dev/null +++ b/app/javascript/wardrobe-2020/loaders/items.js @@ -0,0 +1,57 @@ +import { useQuery } from "@tanstack/react-query"; + +export function useItemAppearances(id, options = {}) { + return useQuery({ + ...options, + queryKey: ["items", String(id)], + queryFn: () => loadItemAppearances(id), + }); +} + +async function loadItemAppearances(id) { + const res = await fetch(`/items/${encodeURIComponent(id)}/appearances.json`); + + if (!res.ok) { + throw new Error( + `loading item appearances failed: ${res.status} ${res.statusText}`, + ); + } + + return res.json().then(normalizeItemAppearances); +} + +function normalizeItemAppearances(appearances) { + return appearances.map((appearance) => ({ + body: normalizeBody(appearance.body), + swfAssets: appearance.swf_assets.map((asset) => ({ + id: String(asset.id), + knownGlitches: asset.known_glitches, + zone: normalizeZone(asset.zone), + restrictedZones: asset.restricted_zones.map((z) => normalizeZone(z)), + urls: { + swf: asset.urls.swf, + png: asset.urls.png, + manifest: asset.urls.manifest, + }, + })), + })); +} + +function normalizeBody(body) { + if (String(body.id) === "0") { + return { id: "0" }; + } + + return { + id: String(body.id), + species: { + id: String(body.species.id), + name: body.species.name, + humanName: body.species.humanName, + }, + }; +} + +function normalizeZone(zone) { + return { id: String(zone.id), label: zone.label, depth: zone.depth }; +} diff --git a/app/javascript/wardrobe-2020/loaders/outfits.js b/app/javascript/wardrobe-2020/loaders/outfits.js index ac73ab6c..2063e7c3 100644 --- a/app/javascript/wardrobe-2020/loaders/outfits.js +++ b/app/javascript/wardrobe-2020/loaders/outfits.js @@ -1,6 +1,6 @@ import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; -export function useSavedOutfit(id, options) { +export function useSavedOutfit(id, options = {}) { return useQuery({ ...options, queryKey: ["outfits", String(id)], From c6cb61ef38d3283ce1dbdaab7ed1461be95fbf4a Mon Sep 17 00:00:00 2001 From: Matchu Date: Sat, 11 Nov 2023 08:24:08 -0800 Subject: [PATCH 7/9] Remove no-op item.thumbnail.secure_url 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. --- app/helpers/contribution_helper.rb | 2 +- app/models/image.rb | 14 -------------- app/models/item.rb | 10 ---------- app/views/items/_item_link.html.haml | 2 +- app/views/items/show.html.haml | 2 +- app/views/outfits/new.html.haml | 4 ++-- 6 files changed, 5 insertions(+), 29 deletions(-) delete mode 100644 app/models/image.rb diff --git a/app/helpers/contribution_helper.rb b/app/helpers/contribution_helper.rb index da8a77d7..eb01776f 100644 --- a/app/helpers/contribution_helper.rb +++ b/app/helpers/contribution_helper.rb @@ -19,7 +19,7 @@ module ContributionHelper :item_link => link) output = translate("contributions.contributed_description.main.#{main_key}_html", :item_description => description) - output << image_tag(item.thumbnail.secure_url) if show_image + output << image_tag(item.thumbnail_url) if show_image output else translate('contributions.contributed_description.parents.item.blank') diff --git a/app/models/image.rb b/app/models/image.rb deleted file mode 100644 index 75f027aa..00000000 --- a/app/models/image.rb +++ /dev/null @@ -1,14 +0,0 @@ -class Image - attr_reader :insecure_url, :secure_url - - def initialize(insecure_url, secure_url) - @insecure_url = insecure_url - @secure_url = secure_url - end - - def self.from_insecure_url(insecure_url) - # TODO: We used to use a "Camo" server for this, but we don't anymore. - # Replace this with actual logic to actually secure the URLs! - Image.new insecure_url, insecure_url - end -end diff --git a/app/models/item.rb b/app/models/item.rb index 18e9a5b2..a6893cd4 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -413,16 +413,6 @@ class Item < ApplicationRecord modeled_body_ids.size.to_f / predicted_body_ids.size end - def thumbnail - if thumbnail_url.present? - url = thumbnail_url - else - url = ActionController::Base.helpers.asset_path( - "broken_item_thumbnail.gif") - end - @thumbnail ||= Image.from_insecure_url(url) - end - def as_json(options={}) json = { :description => description, diff --git a/app/views/items/_item_link.html.haml b/app/views/items/_item_link.html.haml index 27fb65ef..e50aafd3 100644 --- a/app/views/items/_item_link.html.haml +++ b/app/views/items/_item_link.html.haml @@ -1,4 +1,4 @@ = link_to item_path(item) do - = image_tag item.thumbnail.secure_url, :alt => item.description, :title => item.description + = image_tag item.thumbnail_url, :alt => item.description, :title => item.description %span.name= item.name = nc_icon_for(item) diff --git a/app/views/items/show.html.haml b/app/views/items/show.html.haml index 8dfc521b..4cb64ce9 100644 --- a/app/views/items/show.html.haml +++ b/app/views/items/show.html.haml @@ -2,7 +2,7 @@ - canonical_path @item %header#item-header - = image_tag @item.thumbnail.secure_url, :id => 'item-thumbnail' + = image_tag @item.thumbnail_url, :id => 'item-thumbnail' %div %h2#item-name= @item.name = nc_icon_for(@item) diff --git a/app/views/outfits/new.html.haml b/app/views/outfits/new.html.haml index f49c7e83..3031052d 100644 --- a/app/views/outfits/new.html.haml +++ b/app/views/outfits/new.html.haml @@ -88,7 +88,7 @@ - @newest_unmodeled_items.each do |item| - localized_cache "items/#{item.id} modeling_progress updated_at=#{item.updated_at.to_i}" do %li{'data-item-id' => item.id} - = link_to image_tag(item.thumbnail.secure_url), item, :class => 'image-link' + = link_to image_tag(item.thumbnail_url), item, :class => 'image-link' = link_to item, :class => 'header' do %h2= item.name %span.meter{style: "width: #{@newest_unmodeled_items_predicted_modeled_ratio[item]*100}%"} @@ -101,7 +101,7 @@ - @newest_modeled_items.each do |item| %li.object = link_to item, title: item.name, alt: item.name do - = image_tag item.thumbnail.secure_url + = image_tag item.thumbnail_url = nc_icon_for(item) From a31039c4c8cea2d5c4f574a7f1dee49d5d907ca0 Mon Sep 17 00:00:00 2001 From: Matchu Date: Sat, 11 Nov 2023 08:41:31 -0800 Subject: [PATCH 8/9] Load item page restricted zones data from Rails app, not impress-2020 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.) --- .../item_appearances_controller.rb | 4 +- .../wardrobe-2020/ItemPageOutfitPreview.js | 9 ++--- app/javascript/wardrobe-2020/loaders/items.js | 37 ++++++++++--------- app/models/item.rb | 25 ++----------- 4 files changed, 30 insertions(+), 45 deletions(-) diff --git a/app/controllers/item_appearances_controller.rb b/app/controllers/item_appearances_controller.rb index be8edabb..2c72f9fc 100644 --- a/app/controllers/item_appearances_controller.rb +++ b/app/controllers/item_appearances_controller.rb @@ -1,6 +1,8 @@ class ItemAppearancesController < ApplicationController def index @item = Item.find(params[:item_id]) - render json: @item.appearances + render json: @item.as_json( + only: [:id], methods: [:appearances, :restricted_zones] + ) end end diff --git a/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js b/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js index e85b55ab..ffe6ea25 100644 --- a/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js +++ b/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js @@ -105,7 +105,8 @@ function ItemPageOutfitPreview({ itemId }) { loading: loadingAppearances, error: errorAppearances, } = useItemAppearances(itemId); - const itemAppearances = itemAppearancesData || []; + const itemAppearances = itemAppearancesData?.appearances ?? []; + const restrictedZones = itemAppearancesData?.restrictedZones ?? []; // Start by loading the "canonical" pet and item appearance for the outfit // preview. We'll use this to initialize both the preview and the picker. @@ -132,10 +133,6 @@ function ItemPageOutfitPreview({ itemId }) { item(id: $itemId) { id name - restrictedZones { - id - label - } canonicalAppearance( preferredSpeciesId: $preferredSpeciesId preferredColorId: $preferredColorId @@ -387,7 +384,7 @@ function ItemPageOutfitPreview({ itemId }) { {itemAppearances.length > 0 && ( )} diff --git a/app/javascript/wardrobe-2020/loaders/items.js b/app/javascript/wardrobe-2020/loaders/items.js index fb12f70e..1387e39b 100644 --- a/app/javascript/wardrobe-2020/loaders/items.js +++ b/app/javascript/wardrobe-2020/loaders/items.js @@ -4,11 +4,11 @@ export function useItemAppearances(id, options = {}) { return useQuery({ ...options, queryKey: ["items", String(id)], - queryFn: () => loadItemAppearances(id), + queryFn: () => loadItemAppearancesData(id), }); } -async function loadItemAppearances(id) { +async function loadItemAppearancesData(id) { const res = await fetch(`/items/${encodeURIComponent(id)}/appearances.json`); if (!res.ok) { @@ -17,24 +17,27 @@ async function loadItemAppearances(id) { ); } - return res.json().then(normalizeItemAppearances); + return res.json().then(normalizeItemAppearancesData); } -function normalizeItemAppearances(appearances) { - return appearances.map((appearance) => ({ - body: normalizeBody(appearance.body), - swfAssets: appearance.swf_assets.map((asset) => ({ - id: String(asset.id), - knownGlitches: asset.known_glitches, - zone: normalizeZone(asset.zone), - restrictedZones: asset.restricted_zones.map((z) => normalizeZone(z)), - urls: { - swf: asset.urls.swf, - png: asset.urls.png, - manifest: asset.urls.manifest, - }, +function normalizeItemAppearancesData(data) { + return { + appearances: data.appearances.map((appearance) => ({ + body: normalizeBody(appearance.body), + swfAssets: appearance.swf_assets.map((asset) => ({ + id: String(asset.id), + knownGlitches: asset.known_glitches, + zone: normalizeZone(asset.zone), + restrictedZones: asset.restricted_zones.map((z) => normalizeZone(z)), + urls: { + swf: asset.urls.swf, + png: asset.urls.png, + manifest: asset.urls.manifest, + }, + })), })), - })); + restrictedZones: data.restricted_zones.map((z) => normalizeZone(z)), + }; } function normalizeBody(body) { diff --git a/app/models/item.rb b/app/models/item.rb index a6893cd4..8e8aae06 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -414,27 +414,10 @@ class Item < ApplicationRecord end def as_json(options={}) - json = { - :description => description, - :id => id, - :name => name, - :thumbnail_url => thumbnail.secure_url, - :zones_restrict => zones_restrict, - :rarity_index => rarity_index, - :nc => nc? - } - - # Set owned and wanted keys, unless explicitly told not to. (For example, - # item proxies don't want us to bother, since they'll override.) - unless options.has_key?(:include_hanger_status) - options[:include_hanger_status] = true - end - if options[:include_hanger_status] - json[:owned] = owned? - json[:wanted] = wanted? - end - - json + super({ + only: [:id, :name, :description, :thumbnail_url, :rarity_index], + methods: [:zones_restrict], + }.merge(options)) end before_create do From b54be0ab5c47f3f05ab4f3c024d333b6e93f24cf Mon Sep 17 00:00:00 2001 From: Matchu Date: Sat, 11 Nov 2023 08:50:32 -0800 Subject: [PATCH 9/9] Load item name from Rails app in item preview page This is just a silly lil single-field migration! --- app/javascript/wardrobe-2020/ItemPageOutfitPreview.js | 11 +++++------ app/javascript/wardrobe-2020/loaders/items.js | 1 + 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js b/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js index ffe6ea25..9d6e5b47 100644 --- a/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js +++ b/app/javascript/wardrobe-2020/ItemPageOutfitPreview.js @@ -105,6 +105,7 @@ function ItemPageOutfitPreview({ itemId }) { loading: loadingAppearances, error: errorAppearances, } = useItemAppearances(itemId); + const itemName = itemAppearancesData?.name ?? ""; const itemAppearances = itemAppearancesData?.appearances ?? []; const restrictedZones = itemAppearancesData?.restrictedZones ?? []; @@ -132,7 +133,6 @@ function ItemPageOutfitPreview({ itemId }) { ) { item(id: $itemId) { id - name canonicalAppearance( preferredSpeciesId: $preferredSpeciesId preferredColorId: $preferredColorId @@ -189,14 +189,13 @@ function ItemPageOutfitPreview({ itemId }) { // appears in the item name, then this is probably a species-specific item, // and we should adjust the UI to avoid implying that other species could // model it. + const speciesName = + data?.item?.canonicalAppearance?.body?.canonicalAppearance?.species?.name ?? + ""; const isProbablySpeciesSpecific = compatibleBodies.length === 1 && compatibleBodies[0] !== "all" && - (data?.item?.name || "") - .toLowerCase() - .includes( - data?.item?.canonicalAppearance?.body?.canonicalAppearance?.species?.name.toLowerCase(), - ); + itemName.toLowerCase().includes(speciesName.toLowerCase()); const couldProbablyModelMoreData = !isProbablySpeciesSpecific; // TODO: Does this double-trigger the HTTP request with SpeciesColorPicker? diff --git a/app/javascript/wardrobe-2020/loaders/items.js b/app/javascript/wardrobe-2020/loaders/items.js index 1387e39b..4b796bce 100644 --- a/app/javascript/wardrobe-2020/loaders/items.js +++ b/app/javascript/wardrobe-2020/loaders/items.js @@ -22,6 +22,7 @@ async function loadItemAppearancesData(id) { function normalizeItemAppearancesData(data) { return { + name: data.name, appearances: data.appearances.map((appearance) => ({ body: normalizeBody(appearance.body), swfAssets: appearance.swf_assets.map((asset) => ({