Add altStyleId to PetAppearance type and resolvers #2

Open
dice wants to merge 2 commits from dice/impress-2020:add-altStyleId into main
2 changed files with 46 additions and 5 deletions

View file

@ -33,9 +33,12 @@ const resolvers = {
return null;
}
const speciesId = customPetData.custom_pet.species_id;
const baseColorId = customPetData.custom_pet.color_id;
const petType = await petTypeBySpeciesAndColorLoader.load({
speciesId: customPetData.custom_pet.species_id,
colorId: customPetData.custom_pet.color_id,
speciesId,
colorId: baseColorId,
});
const petStates = petType
? await petStatesForPetTypeLoader.load(petType.id)
@ -52,8 +55,33 @@ const resolvers = {
.sort((a, b) => Number(a) - Number(b))
.join(",");
petState = petStates.find((ps) => ps.swfAssetIds === swfAssetIdsString);
// Neopets may omit these fields entirely; treat "missing" as null.
const altStyleId = customPetData.custom_pet.alt_style;
const altColorId = customPetData.custom_pet.alt_color;
if (petState) {
return { id: petState.id };
return { id: petState.id, altStyleId, altColorId };
}
// If that didn't work, and Neopets reported an alt color, try looking up
// the same exact-assets pet state on the species+altColor pair.
if (altColorId != null && altColorId !== baseColorId) {
const altColorPetType = await petTypeBySpeciesAndColorLoader.load({
speciesId,
colorId: altColorId,
});
if (altColorPetType) {
const altColorPetStates = await petStatesForPetTypeLoader.load(
altColorPetType.id,
);
const altColorPetState = altColorPetStates.find(
(ps) => ps.swfAssetIds === swfAssetIdsString,
);
if (altColorPetState) {
return { id: altColorPetState.id, altStyleId, altColorId };
}
}
Review

I don't really follow what's going on in this block. In what cases would the alt color connect to the pet state? My understanding is they're totally disconnected from one another. (That's how we architected it on our end, anyway.)

I don't really follow what's going on in this block. In what cases would the alt color connect to the pet state? My understanding is they're totally disconnected from one another. (That's how we architected it on our end, anyway.)
}
// Next, look for a pet state matching the same pose. (This can happen if
@ -71,7 +99,7 @@ const resolvers = {
`because it matches pose ${pose}. Actual pet state for these ` +
`assets not found: ${swfAssetIdsString}`,
);
return { id: petState.id };
return { id: petState.id, altStyleId, altColorId };
}
}
@ -85,7 +113,7 @@ const resolvers = {
`as an UNKNOWN fallback pose. Actual pet state for these ` +
`assets not found: ${swfAssetIdsString}`,
);
return { id: petState.id };
return { id: petState.id, altStyleId, altColorId };
}
// If we still don't have a pet state, raise an error. (This can happen

View file

@ -78,6 +78,18 @@ const typeDefs = gql`
"""
id: ID!
"""
The ID of the alt style used to render this appearance, if any.
Null when this appearance is not using an alt style.
"""
altStyleId: ID
"""
The ID of the alt color used to render this appearance, if any.
Null when this appearance is not using an alt style.
"""
altColorId: ID
species: Species!
color: Color!
pose: Pose!
@ -273,6 +285,7 @@ const resolvers = {
},
PetAppearance: {
altColorId: ({ altColorId }) => (altColorId),
Review

I suppose we should either have this for both altStyleId and altColorId, or for neither. (I think maybe if we omit the resolvers it just does this exact operation by default? I'll check that before merging.)

I suppose we should either have this for both `altStyleId` and `altColorId`, or for neither. (I think maybe if we omit the resolvers it just does this exact operation by default? I'll check that before merging.)
id: ({ id, altStyleId }) => {
if (altStyleId != null) {
return `${id}-with-alt-style-${altStyleId}`;