Update modeling bug announcement, now that things are working again

Also, while we're here! To restore the lost data, I:

1. Downloaded this scheduled public data backup, which was taken
   thankfully the day before we updated modeling code!
   https://impress.openneo.net/public-data/2024-11-03T08_15_02Z-scheduled.sql.gz
2. Trimmed it just to the section about the `parents_swf_assets` table:
   dropping it, then rebuilding it from scratch.
3. Ran this modified backup SQL dump on the production server.
4. Ran the code from `db/migrate/20241001052510_add_cached_fields_to_items.rb`
   to bring items' cached fields back into the correct state.

I also had to fix some errors in the item data that prevented some
items from passing the latest validations:

```rb
Item.where(rarity: "").update_all(rarity: "???")
Item.where(description: "").update_all(description: "???")
Item.where(zones_restrict: "").update_all(zones_restrict: "00000
00000000000000000000000000000000000000000000000")
```

app/helpers/outfits_helper.rb

@@ -1,5 +1,5 @@
 module OutfitsHelper
-  LAST_DAY_OF_ANNOUNCEMENT = Date.parse("2024-10-21")
+  LAST_DAY_OF_ANNOUNCEMENT = Date.parse("2024-11-08")
   def show_announcement?
     Date.today <= LAST_DAY_OF_ANNOUNCEMENT
   end

app/models/alt_style.rb

@@ -74,6 +74,15 @@ class AltStyle < ApplicationRecord
     Item.appearances_for(items, self, ...)
   end
 
+  def biology=(biology)
+    # TODO: This is very similar to what `PetState` does, but like… much much
+    # more compact? Idk if I'm missing something, or if I was just that much
+    # more clueless back when I wrote it, lol 😅
+    self.swf_assets = biology.values.map do |asset_data|
+      SwfAsset.from_biology_data(self.body_id, asset_data)
+    end
+  end
+
   # At time of writing, most batches of Alt Styles thumbnails used a simple
   # pattern for the item thumbnail URL, but that's not always the case anymore.
   # For now, let's keep using this format as the default value when creating a

app/models/item.rb

@@ -404,6 +404,33 @@ 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
@@ -411,7 +438,7 @@ class Item < ApplicationRecord
     explicitly_body_specific? || !species_support_ids.empty?
   end
 
-  def add_origin_registry_info(info)
+  def add_origin_registry_info(info, locale)
     # 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)
@@ -430,6 +457,16 @@ 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
@@ -601,4 +638,90 @@ 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

app/models/pet.rb

@@ -4,15 +4,66 @@ class Pet < ApplicationRecord
   attr_reader :items, :pet_state, :alt_style
 
   def load!(timeout: nil)
-    viewer_data_hash = Neopets::CustomPets.fetch_viewer_data(name, timeout:)
-    use_modeling_snapshot(ModelingSnapshot.new(viewer_data_hash))
+    viewer_data = Neopets::CustomPets.fetch_viewer_data(name, timeout:)
+    use_viewer_data(viewer_data)
   end
 
-  def use_modeling_snapshot(snapshot)
-    self.pet_type = snapshot.pet_type
-    @pet_state = snapshot.pet_state
-    @alt_style = snapshot.alt_style
-    @items = snapshot.items
+  def use_viewer_data(viewer_data)
+    pet_data = viewer_data[:custom_pet]
+
+    raise UnexpectedDataFormat unless pet_data[:species_id]
+    raise UnexpectedDataFormat unless pet_data[:color_id]
+    raise UnexpectedDataFormat unless pet_data[:body_id]
+
+    has_alt_style = pet_data[:alt_style].present?
+
+    self.pet_type = PetType.find_or_initialize_by(
+      species_id: pet_data[:species_id].to_i,
+      color_id: pet_data[:color_id].to_i
+    )
+
+    begin
+      new_image_hash = Neopets::CustomPets.fetch_image_hash(self.name)
+    rescue => error
+      Rails.logger.warn "Failed to load image hash: #{error.full_message}"
+    end
+    self.pet_type.image_hash = new_image_hash if new_image_hash.present?
+
+    # With an alt style, `body_id` in the biology data refers to the body ID of
+    # the *alt* style, not the usual pet type. (We have `original_biology` for
+    # *some* of the pet type's situation, but not it's body ID!)
+    #
+    # So, in the alt style case, don't update `body_id` - but if this is our
+    # first time seeing this pet type and it doesn't *have* a `body_id` yet,
+    # let's not be creating it without one. We'll need to model it without the
+    # alt style first. (I don't bother with a clear error message though 😅)
+    self.pet_type.body_id = pet_data[:body_id] unless has_alt_style
+    if self.pet_type.body_id.nil?
+      raise UnexpectedDataFormat,
+        "can't process alt style on first occurrence of pet type"
+    end
+
+    pet_state_biology = has_alt_style ? pet_data[:original_biology] :
+      pet_data[:biology_by_zone]
+    raise UnexpectedDataFormat if pet_state_biology.empty?
+    pet_state_biology[0] = nil # remove effects if present
+    @pet_state = self.pet_type.add_pet_state_from_biology! pet_state_biology
+
+    if has_alt_style
+      raise UnexpectedDataFormat unless pet_data[:alt_color]
+      raise UnexpectedDataFormat if pet_data[:biology_by_zone].empty?
+
+      @alt_style = AltStyle.find_or_initialize_by(id: pet_data[:alt_style].to_i)
+      @alt_style.assign_attributes(
+        color_id: pet_data[:alt_color].to_i,
+        species_id: pet_data[:species_id].to_i,
+        body_id: pet_data[:body_id].to_i,
+        biology: pet_data[:biology_by_zone],
+      )
+    end
+
+    @items = Item.collection_from_pet_type_and_registries(self.pet_type,
+      viewer_data[:object_info_registry], viewer_data[:object_asset_registry])
   end
 
   def wardrobe_query
@@ -38,9 +89,21 @@ class Pet < ApplicationRecord
 
   before_validation do
     pet_type.save!
-    @pet_state.save! if @pet_state
-    @alt_style.save! if @alt_style
-    (@items || []).each(&:save!)
+    if @pet_state
+      @pet_state.save!
+      @pet_state.handle_assets!
+    end
+    
+    if @items
+      @items.each do |item|
+        item.save! if item.changed?
+        item.handle_assets!
+      end
+    end
+
+    if @alt_style
+      @alt_style.save!
+    end
   end
 
   def self.load(name, **options)

app/models/pet/modeling_snapshot.rb

@@ -1,107 +0,0 @@
-# A representation of a Neopets::CustomPets viewer data response, translated
-# to DTI's database models!
-class Pet::ModelingSnapshot
-  def initialize(viewer_data_hash)
-    @custom_pet = viewer_data_hash[:custom_pet]
-    @object_info_registry = viewer_data_hash[:object_info_registry]
-    @object_asset_registry = viewer_data_hash[:object_asset_registry]
-  end
-
-  def pet_type
-    @pet_type ||= begin
-      raise Pet::UnexpectedDataFormat unless @custom_pet[:species_id]
-      raise Pet::UnexpectedDataFormat unless @custom_pet[:color_id]
-      raise Pet::UnexpectedDataFormat unless @custom_pet[:body_id]
-
-      @custom_pet => {species_id:, color_id:}
-      PetType.find_or_initialize_by(species_id:, color_id:).tap do |pet_type|
-        # Apply the pet's body ID to the pet type, unless it's wearing an alt
-        # style, in which case ignore it, because it's the *alt style*'s body ID.
-        # (This can theoretically cause a problem saving a new pet type when
-        # there's an alt style too!)
-        pet_type.body_id = @custom_pet[:body_id] unless @custom_pet[:alt_style]
-        if pet_type.body_id.nil?
-          raise Pet::UnexpectedDataFormat,
-            "can't process alt style on first occurrence of pet type"
-        end
-
-        # Try using this pet for the pet type's thumbnail, but don't worry
-        # if it fails.
-        begin
-          pet_type.consider_pet_image(@custom_pet[:name])
-        rescue => error
-          Rails.logger.warn "Failed to load pet image: #{error.full_message}"
-        end
-      end
-    end
-  end
-
-  def pet_state
-    @pet_state ||= begin
-      swf_asset_ids = biology_assets.map(&:remote_id)
-      pet_type.pet_states.find_or_initialize_by(swf_asset_ids:).tap do |pet_state|
-        pet_state.swf_assets = biology_assets
-      end
-    end
-  end
-
-  def alt_style
-    @alt_style ||= begin
-      return nil unless @custom_pet[:alt_style]
-      raise Pet::UnexpectedDataFormat unless @custom_pet[:alt_color]
-
-      id = @custom_pet[:alt_style].to_i
-      AltStyle.find_or_initialize_by(id:).tap do |alt_style|
-        alt_style.assign_attributes(
-          color_id: @custom_pet[:alt_color].to_i,
-          species_id: @custom_pet[:species_id].to_i,
-          body_id: @custom_pet[:body_id].to_i,
-          swf_assets: alt_style_assets,
-        )
-      end
-    end
-  end
-
-  def items
-    @items ||= @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
-
-  private
-
-  def biology_assets
-    @biology_assets ||= begin
-      biology = @custom_pet[:alt_style].present? ?
-        @custom_pet[:original_biology] :
-        @custom_pet[:biology_by_zone]
-      assets_from_biology(biology)
-    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 Pet::UnexpectedDataFormat if @custom_pet[:biology_by_zone].empty?
-    @alt_style_assets ||= assets_from_biology(@custom_pet[:biology_by_zone])
-  end
-
-  def assets_from_biology(biology)
-    raise Pet::UnexpectedDataFormat if biology.empty?
-    body_id = @custom_pet[:body_id].to_i
-    biology.values.map { |b| SwfAsset.from_biology_data(body_id, b) }
-  end
-end

app/models/pet_state.rb

@@ -6,16 +6,17 @@ class PetState < ApplicationRecord
   has_many :contributions, :as => :contributed,
     :inverse_of => :contributed # in case of duplicates being merged
   has_many :outfits
-  has_many :parent_swf_asset_relationships, :as => :parent
+  has_many :parent_swf_asset_relationships, :as => :parent,
+    :autosave => false
   has_many :swf_assets, :through => :parent_swf_asset_relationships
 
-  serialize :swf_asset_ids, coder: Serializers::IntegerSet, type: Array
-
   belongs_to :pet_type
 
   delegate :species_id, :species, :color_id, :color, to: :pet_type
 
   alias_method :swf_asset_ids_from_association, :swf_asset_ids
+  
+  attr_writer :parent_swf_asset_relationships_to_update
 
   # A simple ordering that tries to bring reliable pet states to the front.
   scope :emotion_order, -> {
@@ -94,16 +95,85 @@ class PetState < ApplicationRecord
     end
   end
 
+  def sort_swf_asset_ids!
+    self.swf_asset_ids = swf_asset_ids_array.sort.join(',')
+  end
+
+  def swf_asset_ids
+    self['swf_asset_ids']
+  end
+
+  def swf_asset_ids_array
+    swf_asset_ids.split(',').map(&:to_i)
+  end
+
+  def swf_asset_ids=(ids)
+    self['swf_asset_ids'] = ids
+  end
+
+  def handle_assets!
+    @parent_swf_asset_relationships_to_update.each do |rel|
+      rel.swf_asset.save!
+      rel.save!
+    end
+  end
+
   def to_param
     "#{id}-#{pose.split('_').map(&:capitalize).join('-')}"
   end
 
-  # Because our column is named `swf_asset_ids`, we need to ensure writes to
-  # it go to the attribute, and not the thing ActiveRecord does of finding the
-  # relevant `swf_assets`.
-  # TODO: Consider renaming the column to `cached_swf_asset_ids`?
-  def swf_asset_ids=(new_swf_asset_ids)
-    write_attribute(:swf_asset_ids, new_swf_asset_ids)
+  def self.from_pet_type_and_biology_info(pet_type, info)
+    swf_asset_ids = []
+    info.each do |zone_id, asset_info|
+      if zone_id.present? && asset_info
+        swf_asset_ids << asset_info[:part_id].to_i
+      end
+    end
+    swf_asset_ids_str = swf_asset_ids.sort.join(',')
+    if pet_type.new_record?
+      pet_state = self.new :swf_asset_ids => swf_asset_ids_str
+    else
+      pet_state = self.find_or_initialize_by(
+        pet_type_id: pet_type.id,
+        swf_asset_ids: swf_asset_ids_str
+      )
+    end
+    existing_swf_assets = SwfAsset.biology_assets.includes(:zone).
+      where(remote_id: swf_asset_ids)
+    existing_swf_assets_by_id = {}
+    existing_swf_assets.each do |swf_asset|
+      existing_swf_assets_by_id[swf_asset.remote_id] = swf_asset
+    end
+    existing_relationships_by_swf_asset_id = {}
+    unless pet_state.new_record?
+      pet_state.parent_swf_asset_relationships.each do |relationship|
+        existing_relationships_by_swf_asset_id[relationship.swf_asset_id] = relationship
+      end
+    end
+    pet_state.pet_type = pet_type # save the second case from having to look it up by ID
+    relationships = []
+    info.each do |zone_id, asset_info|
+      if zone_id.present? && asset_info
+        swf_asset_id = asset_info[:part_id].to_i
+        swf_asset = existing_swf_assets_by_id[swf_asset_id]
+        unless swf_asset
+          swf_asset = SwfAsset.new
+          swf_asset.remote_id = swf_asset_id
+        end
+        swf_asset.origin_biology_data = asset_info
+        swf_asset.origin_pet_type = pet_type
+        relationship = existing_relationships_by_swf_asset_id[swf_asset.id]
+        unless relationship
+          relationship ||= ParentSwfAssetRelationship.new
+          relationship.parent = pet_state
+          relationship.swf_asset_id = swf_asset.id
+        end
+        relationship.swf_asset = swf_asset
+        relationships << relationship
+      end
+    end
+    pet_state.parent_swf_asset_relationships_to_update = relationships
+    pet_state
   end
 
   private

app/models/pet_type.rb

@@ -57,14 +57,6 @@ class PetType < ApplicationRecord
     basic_image_hash || self['image_hash'] || 'deadbeef'
   end
 
-  def consider_pet_image(pet_name)
-    # If we already have a basic image hash, don't worry about it!
-    return if basic_image_hash?
-
-    # Otherwise, use this as the new image hash for this pet type.
-    self.image_hash = Neopets::CustomPets.fetch_image_hash(pet_name)
-  end
-
   def possibly_new_color
     self.color || Color.new(id: self.color_id)
   end
@@ -79,6 +71,11 @@ class PetType < ApplicationRecord
                    species_human_name: possibly_new_species.human_name)
   end
 
+  def add_pet_state_from_biology!(biology)
+    pet_state = PetState.from_pet_type_and_biology_info(self, biology)
+    pet_state
+  end
+
   def canonical_pet_state
     # For consistency (randomness is always scary!), we use the PetType ID to
     # determine which gender to prefer, if it's not built into the color. That

app/views/outfits/new.html.haml

@@ -6,18 +6,19 @@
 
 - if show_announcement?
   %section.announcement
-    = image_tag "about/announcement.png", width: 70, height: 70,
-      srcset: {"about/announcement@2x.png": "2x"}
+    = image_tag "/images/error-grundo.png", width: 70, height: 70,
+      srcset: {"/images/error-grundo.png": "2x"}
     .content
       %p
         %strong
-          🎃
-          = link_to "New pet styles are out today!", alt_styles_path
-        If you've seen one we don't have yet, please model it by entering the
-        pet's name in the box below. Thank you!!
+          Oops, sorry for the bugs recently!
+        For the first time in One Million Years, we made some changes to our
+        modeling code—and it looks like we goofed it, and started gradually
+        losing some data!
       %p
-        By the way, we had a bug where modeling new styles wasn't working for a
-        little while. Fixed now! 🤞
+        We've restored a backup from before we made these changes, so most
+        everything is back in order! None of your personal data was affected.
+        Sorry for the disruption, and hope everyone is doing okay! 💜
 
 #outfit-forms
   #pet-preview

spec/fixtures/colors.yml

@@ -1,3 +1,6 @@
+blue:
+  id: 8
+  name: blue
 purple:
   id: 57
   name: purple

spec/fixtures/species.yml

@@ -1,3 +1,6 @@
+acara:
+  id: 1
+  name: acara
 blumaroo:
   id: 3
   name: blumaroo

spec/models/pet_spec.rb

@@ -36,11 +36,52 @@ RSpec.describe Pet, type: :model do
           let(:asset_ids) { biology_assets.map(&:remote_id) }
 
           they("are all new") { should all be_new_record }
-          they("match the expected IDs") do
+          pending("match the expected IDs (before saving)") do
+            expect(asset_ids).to contain_exactly(10083, 11613, 14187, 14189)
+          end
+          they("match the expected IDs (after saving)") do
+            pet.save! # TODO: Remove this test once the above passes.
             expect(asset_ids).to contain_exactly(10083, 11613, 14187, 14189)
           end
           they("are saved when saving the pet") { pet.save!; should all be_persisted }
-          they("have the expected asset metadata") do
+          pending("have the expected asset metadata (before saving)") do
+            should contain_exactly(
+              a_record_matching(
+                type: "biology",
+                remote_id: 10083,
+                zone_id: 37,
+                url: "https://images.neopets.com/cp/bio/swf/000/000/010/10083_8a1111a13f.swf",
+                manifest_url: "https://images.neopets.com/cp/bio/data/000/000/010/10083_8a1111a13f/manifest.json",
+                zones_restrict: "0000000000000000000000000000000000000000000000000000",
+              ),
+              a_record_matching(
+                type: "biology",
+                remote_id: 11613,
+                zone_id: 15,
+                url: "https://images.neopets.com/cp/bio/swf/000/000/011/11613_f7d8d377ab.swf",
+                manifest_url: "https://images.neopets.com/cp/bio/data/000/000/011/11613_f7d8d377ab/manifest.json",
+                zones_restrict: "0000000000000000000000000000000000000000000000000000",
+              ),
+              a_record_matching(
+                type: "biology",
+                remote_id: 14187,
+                zone_id: 34,
+                url: "https://images.neopets.com/cp/bio/swf/000/000/014/14187_0e65c2082f.swf",
+                manifest_url: "https://images.neopets.com/cp/bio/data/000/000/014/14187_0e65c2082f/manifest.json",
+                zones_restrict: "0000000000000000000000000000000000000000000000000000",
+              ),
+              a_record_matching(
+                type: "biology",
+                remote_id: 14189,
+                zone_id: 33,
+                url: "https://images.neopets.com/cp/bio/swf/000/000/014/14189_102e4991e9.swf",
+                manifest_url: "https://images.neopets.com/cp/bio/data/000/000/014/14189_102e4991e9/manifest.json",
+                zones_restrict: "0000000000000000000000000000000000000000000000000000",
+              )
+            )
+          end
+          they("have the expected asset metadata (after saving)") do
+            pet.save! # TODO: Remove this test once the above passes.
             should contain_exactly(
               a_record_matching(
                 type: "biology",
@@ -89,7 +130,7 @@ RSpec.describe Pet, type: :model do
           it("already exists") { should be_persisted }
           it("is the same as before") { should eq pet.pet_type }
           it "is not changed when saving the pet" do
-            expect { new_pet.save! }.not_to change { pet_type.attributes }
+            new_pet.save!; expect(pet_type.previous_changes).to be_empty
           end
         end
 
@@ -99,7 +140,7 @@ RSpec.describe Pet, type: :model do
           it("already exists") { should be_persisted }
           it("is the same as before") { should eq pet.pet_state }
           it "is not changed when saving the pet" do
-            expect { new_pet.save! }.not_to change { pet_state.attributes }
+            new_pet.save!; expect(pet_state.previous_changes).to be_empty
           end
         end
 
@@ -109,7 +150,7 @@ RSpec.describe Pet, type: :model do
           they("already exist") { should all be_persisted }
           they("are the same as before") { should eq pet.pet_state.swf_assets }
           they("are not changed when saving the pet") do
-            expect { new_pet.save! }.not_to change { biology_assets.map(&:attributes) }
+            new_pet.save!; expect(biology_assets.map(&:previous_changes)).to all be_empty
           end
         end
       end
@@ -124,7 +165,7 @@ RSpec.describe Pet, type: :model do
           it("already exists") { should be_persisted }
           it("is the same as before") { should eq pet.pet_type }
           it "is not changed when saving the pet" do
-            expect { new_pet.save! }.not_to change { pet_type.attributes }
+            new_pet.save!; expect(pet_type.previous_changes).to be_empty
           end
         end
 
@@ -146,15 +187,56 @@ RSpec.describe Pet, type: :model do
               biology_assets.select(&:new_record?).map(&:remote_id)
             }
 
-            they("are partially new, partially existing") do
+            pending("are partially new, partially existing") do
               expect(persisted_asset_ids).to contain_exactly(10083, 11613)
               expect(new_asset_ids).to contain_exactly(10448, 10451)
             end
-            they("match the expected IDs") do
+            pending("match the expected IDs (before saving)") do
+              expect(asset_ids).to contain_exactly(10083, 11613, 10448, 10451)
+            end
+            they("match the expected IDs (after saving)") do
+              new_pet.save! # TODO: Remove this test once the above passes.
               expect(asset_ids).to contain_exactly(10083, 11613, 10448, 10451)
             end
             they("are saved when saving the pet") { new_pet.save!; should all be_persisted }
-            they("have the expected asset metadata") do
+            pending("have the expected asset metadata (before saving)") do
+              should contain_exactly(
+                a_record_matching(
+                  type: "biology",
+                  remote_id: 10083,
+                  zone_id: 37,
+                  url: "https://images.neopets.com/cp/bio/swf/000/000/010/10083_8a1111a13f.swf",
+                  manifest_url: "https://images.neopets.com/cp/bio/data/000/000/010/10083_8a1111a13f/manifest.json",
+                  zones_restrict: "0000000000000000000000000000000000000000000000000000",
+                ),
+                a_record_matching(
+                  type: "biology",
+                  remote_id: 11613,
+                  zone_id: 15,
+                  url: "https://images.neopets.com/cp/bio/swf/000/000/011/11613_f7d8d377ab.swf",
+                  manifest_url: "https://images.neopets.com/cp/bio/data/000/000/011/11613_f7d8d377ab/manifest.json",
+                  zones_restrict: "0000000000000000000000000000000000000000000000000000",
+                ),
+                a_record_matching(
+                  type: "biology",
+                  remote_id: 10448,
+                  zone_id: 34,
+                  url: "https://images.neopets.com/cp/bio/swf/000/000/010/10448_0b238e79e2.swf",
+                  manifest_url: "https://images.neopets.com/cp/bio/data/000/000/010/10448_0b238e79e2/manifest.json",
+                  zones_restrict: "0000000000000000000000000000000000000000000000000000",
+                ),
+                a_record_matching(
+                  type: "biology",
+                  remote_id: 10451,
+                  zone_id: 33,
+                  url: "https://images.neopets.com/cp/bio/swf/000/000/010/10451_cd4a8a8e47.swf",
+                  manifest_url: "https://images.neopets.com/cp/bio/data/000/000/010/10451_cd4a8a8e47/manifest.json",
+                  zones_restrict: "0000000000000000000000000000000000000000000000000000",
+                )
+              )
+            end
+            they("have the expected asset metadata (after saving)") do
+              new_pet.save! # TODO: Remove this test once the above passes.
               should contain_exactly(
                 a_record_matching(
                   type: "biology",
@@ -207,7 +289,11 @@ RSpec.describe Pet, type: :model do
         let(:asset_ids) { biology_assets.map(&:remote_id) }
 
         they("are all new") { should all be_new_record }
-        they("match the expected IDs") do
+        pending("match the expected IDs (before saving)") do
+          expect(asset_ids).to contain_exactly(331, 332, 333, 23760, 23411)
+        end
+        they("match the expected IDs (after saving)") do
+          pet.save! # TODO: Remove this test once the above passes.
           expect(asset_ids).to contain_exactly(331, 332, 333, 23760, 23411)
         end
         they("are saved when saving the pet") { pet.save!; should all be_persisted }
@@ -216,13 +302,14 @@ RSpec.describe Pet, type: :model do
       describe "its items" do
         subject(:items) { pet.items }
         let(:item_ids) { items.map(&:id) }
+        let(:compatible_body_ids) { items.to_h { |i| [i.id, i.compatible_body_ids] } }
 
         they("are all new") { should all be_new_record }
         they("match the expected IDs") do
           expect(item_ids).to contain_exactly(39552, 53874, 71706)
         end
         they("are saved when saving the pet") { pet.save! ; should all be_persisted }
-        they("have the expected item metadata") do
+        they("have the expected item metadata (without even saving first)") do
           should contain_exactly(
             a_record_matching(
               id: 39552,
@@ -268,6 +355,14 @@ RSpec.describe Pet, type: :model do
             ),
           )
         end
+        they("should be marked compatible with this pet's body ID") do
+          pet.save!
+          expect(compatible_body_ids).to eq(
+            39552 => [47],
+            53874 => [47],
+            71706 => [0],
+          )
+        end
       end
 
       context "its item assets" do
@@ -276,11 +371,50 @@ RSpec.describe Pet, type: :model do
         let(:asset_ids) { item_assets.map(&:remote_id) }
 
         they("are all new") { should all be_new_record }
-        they("match the expected IDs") do
+        pending("match the expected IDs (before saving)") do
+          expect(asset_ids).to contain_exactly(16933, 108567, 410722)
+        end
+        they("match the expected IDs (after saving)") do
+          pet.save! # TODO: Remove this test once the above passes.
           expect(asset_ids).to contain_exactly(16933, 108567, 410722)
         end
         they("are saved when saving the pet") { pet.save! ; should all be_persisted }
-        they("match the expected metadata") do
+        pending("match the expected metadata (before saving)") do
+          expect(assets_by_item).to match(
+            39552 => a_collection_containing_exactly(
+              a_record_matching(
+                type: "object",
+                remote_id: 16933,
+                zone_id: 35,
+                url: "https://images.neopets.com/cp/items/swf/000/000/016/16933_0833353c4f.swf",
+                manifest_url: "https://images.neopets.com/cp/items/data/000/000/016/16933_0833353c4f/manifest.json?v=1706",
+                zones_restrict: "",
+              )
+            ),
+            53874 => a_collection_containing_exactly(
+              a_record_matching(
+                type: "object",
+                remote_id: 108567,
+                zone_id: 23,
+                url: "https://images.neopets.com/cp/items/swf/000/000/108/108567_ee88141325.swf",
+                manifest_url: "https://images.neopets.com/cp/items/data/000/000/108/108567_ee88141325/manifest.json?v=1706",
+                zones_restrict: "",
+              )
+            ),
+            71706 => a_collection_containing_exactly(
+              a_record_matching(
+                type: "object",
+                remote_id: 410722,
+                zone_id: 3,
+                url: "https://images.neopets.com/cp/items/swf/000/000/410/410722_3bcd2f5e11.swf",
+                manifest_url: "https://images.neopets.com/cp/items/data/000/000/410/410722_3bcd2f5e11/manifest.json?v=1706",
+                zones_restrict: "",
+              )
+            ),
+          )
+        end
+        they("match the expected metadata (after saving)") do
+          pet.save! # TODO: Remove this test after the above passes.
           expect(assets_by_item).to match(
             39552 => a_collection_containing_exactly(
               a_record_matching(
@@ -326,7 +460,7 @@ RSpec.describe Pet, type: :model do
           it("already exists") { should be_persisted }
           it("is the same as before") { should eq pet.pet_type }
           it "is not changed when saving the pet" do
-            expect { new_pet.save! }.not_to change { pet_type.attributes }
+            new_pet.save!; expect(pet_type.previous_changes).to be_empty
           end
         end
 
@@ -336,7 +470,7 @@ RSpec.describe Pet, type: :model do
           it("already exists") { should be_persisted }
           it("is the same as before") { should eq pet.pet_state }
           it "is not changed when saving the pet" do
-            expect { new_pet.save! }.not_to change { pet_state.attributes }
+            new_pet.save!; expect(pet_state.previous_changes).to be_empty
           end
         end
 
@@ -346,7 +480,7 @@ RSpec.describe Pet, type: :model do
           they("already exist") { should all be_persisted }
           they("are the same as before") { should eq pet.pet_state.swf_assets }
           they("are not changed when saving the pet") do
-            expect { new_pet.save! }.not_to change { biology_assets.map(&:attributes) }
+            new_pet.save!; expect(biology_assets.map(&:previous_changes)).to all be_empty
           end
         end
 
@@ -356,7 +490,7 @@ RSpec.describe Pet, type: :model do
           they("already exist") { should all be_persisted }
           they("are the same as before") { should eq pet.items }
           they("are not changed when saving the pet") do
-            expect { new_pet.save! }.not_to change { items.map(&:attributes) }
+            new_pet.save!; expect(items.map(&:previous_changes)).to all be_empty
           end
         end
 
@@ -366,7 +500,26 @@ RSpec.describe Pet, type: :model do
           they("already exist") { should all be_persisted }
           they("are the same as before") { should eq pet.items.map(&:swf_assets).flatten(1) }
           they("are not changed when saving the pet") do
-            expect { new_pet.save! }.not_to change { item_assets.map(&:attributes) }
+            new_pet.save!; expect(item_assets.map(&:previous_changes)).to all be_empty
+          end
+        end
+      end
+
+      context "when modeled a second time, but as a Blue Acara" do
+        before { pet.save! }
+        subject(:new_pet) { Pet.load("matts_bat:acara") }
+
+        describe "its items" do
+          subject(:items) { new_pet.items }
+          let(:compatible_body_ids) { items.to_h { |i| [i.id, i.compatible_body_ids] } }
+
+          they("should be marked compatible with both pets' body IDs") do
+            new_pet.save!
+            expect(compatible_body_ids).to eq(
+              39552 => [47, 93],
+              53874 => [47, 93],
+              71706 => [0],
+            )
           end
         end
       end
@@ -432,7 +585,7 @@ RSpec.describe Pet, type: :model do
             it("already exists") { should be_persisted }
             it("is the same as before") { should eq pet.alt_style }
             it "is not changed when saving the pet" do
-              expect { new_pet.save! }.not_to change { alt_style.attributes }
+              new_pet.save!; expect(alt_style.previous_changes).to be_empty
             end
 
             describe "its assets" do
@@ -441,12 +594,20 @@ RSpec.describe Pet, type: :model do
               they("already exist") { should all be_persisted }
               they("are the same as before") { should eq pet.alt_style.swf_assets }
               they("are not changed when saving the pet") do
-                expect { new_pet.save! }.not_to change { assets.map(&:attributes) }
+                new_pet.save!; expect(assets.map(&:previous_changes)).to all be_empty
               end
             end
           end
         end
       end
     end
+
+    context "when modeling is disabled" do
+      before { allow(Rails.configuration).to receive(:modeling_enabled) { false } }
+
+      pending("raises an error") do
+        expect { Pet.load("matts_bat") }.to raise_error(Pet::ModelingDisabled)
+      end
+    end
   end
 end

spec/support/mocks/custom_pets/matts_bat:acara.json

@@ -0,0 +1,194 @@
+{
+  "dti_comment": "This is matts_bat, hand-modified to be a Blue Acara wearing the same items.",
+  "custom_pet": {
+    "name": "matts_bat",
+    "owner": "matchu1993",
+    "slot": 1.0,
+    "scale": 0.5,
+    "muted": true,
+    "body_id": 93.0,
+    "species_id": 1.0,
+    "color_id": 8.0,
+    "alt_style": false,
+    "alt_color": 8.0,
+    "style_closet_id": null,
+    "biology_by_zone": {
+      "30": {
+        "part_id": 32185.0,
+        "zone_id": 30.0,
+        "asset_url": "https://images.neopets.com/cp/bio/swf/000/000/032/32185_dc8f076ae3.swf",
+        "manifest": "https://images.neopets.com/cp/bio/data/000/000/032/32185_dc8f076ae3/manifest.json",
+        "zones_restrict": "0000000000000000000000000000000000000000000000000000"
+      },
+      "15": {
+        "part_id": 2425.0,
+        "zone_id": 15.0,
+        "asset_url": "https://images.neopets.com/cp/bio/swf/000/000/002/2425_501f596cef.swf",
+        "manifest": "https://images.neopets.com/cp/bio/data/000/000/002/2425_501f596cef/manifest.json",
+        "zones_restrict": "0000000000000000000000000000000000000000000000000000"
+      },
+      "5": {
+        "part_id": 2426.0,
+        "zone_id": 5.0,
+        "asset_url": "https://images.neopets.com/cp/bio/swf/000/000/002/2426_898928db88.swf",
+        "manifest": "https://images.neopets.com/cp/bio/data/000/000/002/2426_898928db88/manifest.json",
+        "zones_restrict": "0000000000000000000000000000000000000000000000000000"
+      },
+      "38": {
+        "part_id": 2427.0,
+        "zone_id": 38.0,
+        "asset_url": "https://images.neopets.com/cp/bio/swf/000/000/002/2427_f12853f18a.swf",
+        "manifest": "https://images.neopets.com/cp/bio/data/000/000/002/2427_f12853f18a/manifest.json",
+        "zones_restrict": "0000000000000000000000000000000000000000000000000000"
+      },
+      "34": {
+        "part_id": 19157.0,
+        "zone_id": 34.0,
+        "asset_url": "https://images.neopets.com/cp/bio/swf/000/000/019/19157_f2e42f30e9.swf",
+        "manifest": "https://images.neopets.com/cp/bio/data/000/000/019/19157_f2e42f30e9/manifest.json",
+        "zones_restrict": "0000000000000000000000000000000000000000000000000000"
+      },
+      "33": {
+        "part_id": 18945.0,
+        "zone_id": 33.0,
+        "asset_url": "https://images.neopets.com/cp/bio/swf/000/000/018/18945_45623865d6.swf",
+        "manifest": "https://images.neopets.com/cp/bio/data/000/000/018/18945_45623865d6/manifest.json",
+        "zones_restrict": "0000000000000000000000000000000000000000000000000000"
+      }
+    },
+    "equipped_by_zone": {
+      "35": {
+        "asset_id": 16931.0,
+        "zone_id": 35.0,
+        "closet_obj_id": 2549145.0
+      },
+      "23": {
+        "asset_id": 108565.0,
+        "zone_id": 23.0,
+        "closet_obj_id": 16955628.0
+      },
+      "3": {
+        "asset_id": 410722.0,
+        "zone_id": 3.0,
+        "closet_obj_id": 17147987.0
+      }
+    },
+    "original_biology": [
+
+    ]
+  },
+  "closet_items": {
+    "2549145": {
+      "closet_obj_id": 2549145.0,
+      "obj_info_id": 39552.0,
+      "applied_to": "matts_bat",
+      "is_wishlist": false,
+      "expiration": "N/A"
+    },
+    "16955628": {
+      "closet_obj_id": 16955628.0,
+      "obj_info_id": 53874.0,
+      "applied_to": "matts_bat",
+      "is_wishlist": false,
+      "expiration": "N/A"
+    },
+    "17147987": {
+      "closet_obj_id": 17147987.0,
+      "obj_info_id": 71706.0,
+      "applied_to": "matts_bat",
+      "is_wishlist": false,
+      "expiration": "N/A"
+    }
+  },
+  "object_info_registry": {
+    "39552": {
+      "obj_info_id": 39552.0,
+      "assets_by_zone": {
+        "35": 16931.0
+      },
+      "zones_restrict": "0000000000000000000000000000000000000000000000000000",
+      "is_compatible": true,
+      "is_paid": true,
+      "thumbnail_url": "https://images.neopets.com/items/mall_springyeyeglasses.gif",
+      "name": "Springy Eye Glasses",
+      "description": "Hey, keep your eyes in your head!",
+      "category": "Clothes",
+      "type": "Clothes",
+      "rarity": "Artifact",
+      "rarity_index": 500.0,
+      "price": 0.0,
+      "weight_lbs": 1.0,
+      "species_support": [
+        3.0
+      ],
+      "converted": true
+    },
+    "53874": {
+      "obj_info_id": 53874.0,
+      "assets_by_zone": {
+        "23": 108565.0
+      },
+      "zones_restrict": "0000000000000000000000000000000000000000000000000000",
+      "is_compatible": true,
+      "is_paid": false,
+      "thumbnail_url": "https://images.neopets.com/items/clo_404_shirt.gif",
+      "name": "404 Shirt",
+      "description": "When Neopets is down, the shirt comes on!",
+      "category": "Clothes",
+      "type": "Clothes",
+      "rarity": "Rare",
+      "rarity_index": 88.0,
+      "price": 1701.0,
+      "weight_lbs": 1.0,
+      "species_support": [
+        3.0
+      ],
+      "converted": true
+    },
+    "71706": {
+      "obj_info_id": 71706.0,
+      "assets_by_zone": {
+        "3": 410722.0
+      },
+      "zones_restrict": "0000000000000000000000000000000000000000000000000000",
+      "is_compatible": true,
+      "is_paid": false,
+      "thumbnail_url": "https://images.neopets.com/items/gif_roof_onthe_fg.gif",
+      "name": "On the Roof Background",
+      "description": "Who is that on the roof?! Could it be...?",
+      "category": "Special",
+      "type": "Mystical Surroundings",
+      "rarity": "Special",
+      "rarity_index": 101.0,
+      "price": 0.0,
+      "weight_lbs": 1.0,
+      "species_support": [
+
+      ],
+      "converted": true
+    }
+  },
+  "object_asset_registry": {
+    "16931": {
+      "asset_id": 16931.0,
+      "zone_id": 35.0,
+      "asset_url": "https://images.neopets.com/cp/items/swf/000/000/016/16931_aa01126387.swf",
+      "obj_info_id": 39552.0,
+      "manifest": "https://images.neopets.com/cp/items/data/000/000/016/16931_aa01126387/manifest.json?v=1706"
+    },
+    "108565": {
+      "asset_id": 108565.0,
+      "zone_id": 23.0,
+      "asset_url": "https://images.neopets.com/cp/items/swf/000/000/108/108565_80d238dbaf.swf",
+      "obj_info_id": 53874.0,
+      "manifest": "https://images.neopets.com/cp/items/data/000/000/108/108565_80d238dbaf/manifest.json?v=1706"
+    },
+    "410722": {
+      "asset_id": 410722.0,
+      "zone_id": 3.0,
+      "asset_url": "https://images.neopets.com/cp/items/swf/000/000/410/410722_3bcd2f5e11.swf",
+      "obj_info_id": 71706.0,
+      "manifest": "https://images.neopets.com/cp/items/data/000/000/410/410722_3bcd2f5e11/manifest.json?v=1706"
+    }
+  }
+}