OpenNeo/impress
OpenNeo/impress
1
0
Fork 1
Code Pull requests Activity

Reintroduce some of our modeling refactors, without touching items

Browse source 
Okay so, when we reverted a buncha stuff in e3d196f, it was in response
to a bug where item modeling data was getting deleted. And I was tired,
and just took a big simple hammer to it of reverting all the modeling
refactors.

Here, we reintroduce *some* of them: the biology ones before the item
bug. And tests still pass, and in fact I can un-pending some of them!

I might also try to reapply the change where we extract it all into a
new file, but without the item parts.

```shell
git cherry-pick --no-commit 13ceec8fcc
git cherry-pick --no-commit f81415d327
git cherry-pick --no-commit c03e7446e3
git cherry-pick --no-commit 52ca41dbff
```
This commit is contained in:
Emi Matchu 2024-11-10 11:36:23 -08:00
parent e4e81f0694
commit 54b25ef08e
5 changed files with 116 additions and 147 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
app/models/alt_style.rb
View file

@ -74,15 +74,6 @@ 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

163
app/models/pet.rb
View file

@ -4,66 +4,15 @@ class Pet < ApplicationRecord
attr_reader :items, :pet_state, :alt_style
def load!(timeout: nil)
viewer_data = Neopets::CustomPets.fetch_viewer_data(name, timeout:)
use_viewer_data(viewer_data)
viewer_data_hash = Neopets::CustomPets.fetch_viewer_data(name, timeout:)
use_viewer_data(ViewerData.new(viewer_data_hash))
end
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])
self.pet_type = viewer_data.pet_type
@pet_state = viewer_data.pet_state
@alt_style = viewer_data.alt_style
@items = viewer_data.items
end
def wardrobe_query
@ -89,11 +38,8 @@ class Pet < ApplicationRecord
before_validation do
pet_type.save!
if @pet_state
@pet_state.save!
@pet_state.handle_assets!
end
@pet_state.save! if @pet_state
if @items
@items.each do |item|
item.save! if item.changed?
@ -113,5 +59,98 @@ class Pet < ApplicationRecord
end
class UnexpectedDataFormat < RuntimeError;end
# A representation of a Neopets::CustomPets viewer data response, translated
# to DTI's database models!
class ViewerData
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 UnexpectedDataFormat unless @custom_pet[:species_id]
raise UnexpectedDataFormat unless @custom_pet[:color_id]
raise 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 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).sort.join(",")
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 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 ||= Item.collection_from_pet_type_and_registries(
pet_type, @object_info_registry, @object_asset_registry
)
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 alt_style_assets
raise UnexpectedDataFormat if @custom_pet[:biology_by_zone].empty?
assets_from_biology(@custom_pet[:biology_by_zone])
end
def assets_from_biology(biology)
raise 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
end

66
app/models/pet_state.rb
View file

@ -6,8 +6,7 @@ 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,
:autosave => false
has_many :parent_swf_asset_relationships, :as => :parent
has_many :swf_assets, :through => :parent_swf_asset_relationships
belongs_to :pet_type
@ -15,8 +14,6 @@ class PetState < ApplicationRecord
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, -> {
@ -111,71 +108,10 @@ class PetState < ApplicationRecord
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
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
# A helper for the `pose=` method.

13
app/models/pet_type.rb
View file

@ -57,6 +57,14 @@ 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
@ -71,11 +79,6 @@ 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

12
spec/models/pet_spec.rb
View file

@ -36,7 +36,7 @@ RSpec.describe Pet, type: :model do
let(:asset_ids) { biology_assets.map(&:remote_id) }
they("are all new") { should all be_new_record }
pending("match the expected IDs (before saving)") do
they("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
@ -44,7 +44,7 @@ RSpec.describe Pet, type: :model do
expect(asset_ids).to contain_exactly(10083, 11613, 14187, 14189)
end
they("are saved when saving the pet") { pet.save!; should all be_persisted }
pending("have the expected asset metadata (before saving)") do
they("have the expected asset metadata (before saving)") do
should contain_exactly(
a_record_matching(
type: "biology",
@ -187,11 +187,11 @@ RSpec.describe Pet, type: :model do
biology_assets.select(&:new_record?).map(&:remote_id)
}
pending("are partially new, partially existing") do
they("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
pending("match the expected IDs (before saving)") do
they("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
@ -199,7 +199,7 @@ RSpec.describe Pet, type: :model do
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 }
pending("have the expected asset metadata (before saving)") do
they("have the expected asset metadata (before saving)") do
should contain_exactly(
a_record_matching(
type: "biology",
@ -289,7 +289,7 @@ RSpec.describe Pet, type: :model do
let(:asset_ids) { biology_assets.map(&:remote_id) }
they("are all new") { should all be_new_record }
pending("match the expected IDs (before saving)") do
they("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