From 9eaee4a2d4cdf1a956c3710a3a307e8f0c383ecb Mon Sep 17 00:00:00 2001 From: Emi Matchu Date: Sun, 3 Nov 2024 12:05:37 -0800 Subject: [PATCH] Refactor item modeling Simpler, more encapsulated, and fixes the pending jank stuff in the tests! --- app/models/item.rb | 125 +--------------------------------------- app/models/pet.rb | 36 +++++++----- spec/models/pet_spec.rb | 9 +-- 3 files changed, 24 insertions(+), 146 deletions(-) diff --git a/app/models/item.rb b/app/models/item.rb index 8be75718..653cdaae 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -404,33 +404,6 @@ class Item < ApplicationRecord return PetType.all if compatible_body_ids.include?(0) PetType.where(body_id: compatible_body_ids) end - - def handle_assets! - if @parent_swf_asset_relationships_to_update && @current_body_id - new_swf_asset_ids = @parent_swf_asset_relationships_to_update.map(&:swf_asset_id) - rels = ParentSwfAssetRelationship.arel_table - swf_assets = SwfAsset.arel_table - - # If a relationship used to bind an item and asset for this body type, - # but doesn't in this sample, the two have been unbound. Delete the - # relationship. - ids_to_delete = self.parent_swf_asset_relationships. - select(rels[:id]). - joins(:swf_asset). - where(rels[:swf_asset_id].not_in(new_swf_asset_ids)). - where(swf_assets[:body_id].in([@current_body_id, 0])). - map(&:id) - - unless ids_to_delete.empty? - ParentSwfAssetRelationship.where(:id => ids_to_delete).delete_all - end - - @parent_swf_asset_relationships_to_update.each do |rel| - rel.save! - rel.swf_asset.save! - end - end - end def body_specific? # If there are species support IDs (it's not empty), the item is @@ -438,7 +411,7 @@ class Item < ApplicationRecord explicitly_body_specific? || !species_support_ids.empty? end - def add_origin_registry_info(info, locale) + def add_origin_registry_info(info) # bear in mind that numbers from registries are floats species_support_strs = info['species_support'] || [] self.species_support_ids = species_support_strs.map(&:to_i) @@ -457,16 +430,6 @@ class Item < ApplicationRecord self.zones_restrict = info['zones_restrict'] end - def pending_swf_assets - @parent_swf_asset_relationships_to_update.inject([]) do |all_swf_assets, relationship| - all_swf_assets << relationship.swf_asset - end - end - - def parent_swf_asset_relationships_to_update=(rels) - @parent_swf_asset_relationships_to_update = rels - end - # NOTE: Adding the JSON serializer makes `as_json` treat this like a model # instead of like a hash, so you can target its children with things like # the `include` option. This feels clunky though, I wish I had something a @@ -638,90 +601,4 @@ class Item < ApplicationRecord items end - - def self.collection_from_pet_type_and_registries(pet_type, info_registry, asset_registry, scope=Item.all) - # bear in mind that registries are arrays with many nil elements, - # due to how the parser works - - # Collect existing items - items = {} - item_ids = [] - info_registry.each do |item_id, info| - if info && info[:is_compatible] - item_ids << item_id.to_i - end - end - - # Collect existing relationships - existing_relationships_by_item_id_and_swf_asset_id = {} - existing_items = scope.where(id: item_ids). - includes(:parent_swf_asset_relationships) - existing_items.each do |item| - items[item.id] = item - relationships_by_swf_asset_id = {} - item.parent_swf_asset_relationships.each do |relationship| - relationships_by_swf_asset_id[relationship.swf_asset_id] = relationship - end - existing_relationships_by_item_id_and_swf_asset_id[item.id] = - relationships_by_swf_asset_id - end - - # Collect existing assets - swf_asset_ids = [] - asset_registry.each do |asset_id, asset_data| - swf_asset_ids << asset_id.to_i if asset_data - end - existing_swf_assets = SwfAsset.object_assets.includes(:zone). - where(remote_id: swf_asset_ids) - existing_swf_assets_by_remote_id = {} - existing_swf_assets.each do |swf_asset| - existing_swf_assets_by_remote_id[swf_asset.remote_id] = swf_asset - end - - # With each asset in the registry, - relationships_by_item_id = {} - asset_registry.each do |asset_id, asset_data| - if asset_data - # Build and update the item - item_id = asset_data[:obj_info_id].to_i - next unless item_ids.include?(item_id) # skip incompatible (Uni Bug) - item = items[item_id] - unless item - item = Item.new - item.id = item_id - items[item_id] = item - end - item.add_origin_registry_info info_registry[item.id.to_s], I18n.default_locale - item.current_body_id = pet_type.body_id - - # Build and update the SWF - swf_asset_remote_id = asset_data[:asset_id].to_i - swf_asset = existing_swf_assets_by_remote_id[swf_asset_remote_id] - unless swf_asset - swf_asset = SwfAsset.new - swf_asset.remote_id = swf_asset_remote_id - end - swf_asset.origin_object_data = asset_data - swf_asset.origin_pet_type = pet_type - swf_asset.item = item - - # Build and update the relationship - relationship = existing_relationships_by_item_id_and_swf_asset_id[item.id][swf_asset.id] rescue nil - unless relationship - relationship = ParentSwfAssetRelationship.new - relationship.parent = item - end - relationship.swf_asset = swf_asset - relationships_by_item_id[item_id] ||= [] - relationships_by_item_id[item_id] << relationship - end - end - - # Set up the relationships to be updated on item save - relationships_by_item_id.each do |item_id, relationships| - items[item_id].parent_swf_asset_relationships_to_update = relationships - end - - items.values - end end diff --git a/app/models/pet.rb b/app/models/pet.rb index 189794bf..f15154e8 100644 --- a/app/models/pet.rb +++ b/app/models/pet.rb @@ -39,17 +39,8 @@ class Pet < ApplicationRecord before_validation do pet_type.save! @pet_state.save! if @pet_state - - if @items - @items.each do |item| - item.save! if item.changed? - item.handle_assets! - end - end - - if @alt_style - @alt_style.save! - end + @alt_style.save! if @alt_style + (@items || []).each(&:save!) end def self.load(name, **options) @@ -125,9 +116,14 @@ class Pet < ApplicationRecord end def items - @items ||= Item.collection_from_pet_type_and_registries( - pet_type, @object_info_registry, @object_asset_registry - ) + @items ||= begin + @object_info_registry.map do |id, item_data| + Item.find_or_initialize_by(id:).tap do |item| + item.add_origin_registry_info item_data + item.swf_assets = item_assets_for id + end + end + end end private @@ -141,6 +137,18 @@ class Pet < ApplicationRecord end end + def item_assets_for(item_id) + all_infos = @object_asset_registry.values + infos = all_infos.select { |a| a[:obj_info_id].to_i == item_id.to_i } + infos.map do |asset_data| + remote_id = asset_data[:asset_id].to_i + SwfAsset.find_or_initialize_by(type: "object", remote_id:).tap do |swf_asset| + swf_asset.origin_pet_type = pet_type + swf_asset.origin_object_data = asset_data + end + end + end + def alt_style_assets raise UnexpectedDataFormat if @custom_pet[:biology_by_zone].empty? assets_from_biology(@custom_pet[:biology_by_zone]) diff --git a/spec/models/pet_spec.rb b/spec/models/pet_spec.rb index bd311f7c..55441286 100644 --- a/spec/models/pet_spec.rb +++ b/spec/models/pet_spec.rb @@ -271,18 +271,11 @@ RSpec.describe Pet, type: :model do end context "its item assets" do - # TODO: I wish item assets were set up before saving. - # Once we change this, we can un-mark some tests as pending. - before { pet.save! } - let(:assets_by_item) { pet.items.to_h { |item| [item.id, item.swf_assets.to_a] } } subject(:item_assets) { assets_by_item.values.flatten(1) } let(:asset_ids) { item_assets.map(&:remote_id) } - they("are all new") do - pending("Currently, pets must be saved before assets are assigned.") - should all be_new_record - end + they("are all new") { should all be_new_record } they("match the expected IDs") do expect(asset_ids).to contain_exactly(16933, 108567, 410722) end