Add altStyleId to PetAppearance type and resolvers #2

dice · 2026-01-03T19:46:19-08:00

dice commented

2026-01-03 19:46:19 -08:00

First-time contributor

No description provided.

dice added 1 commit 2026-01-03 19:46:19 -08:00

Add altStyleId to PetAppearance type and resolvers a0a7cfba01

dice added 1 commit 2026-01-05 01:56:29 -08:00

Add altColorId support to PetAppearance 8c1de5e901

matchu requested changes 2026-01-07 15:46:50 -08:00

matchu left a comment

Makes sense to me overall! I have a few questions about details. I also imagine, if we were being real serious, we'd want to expand this into a whole AltStyle GraphQL type… but I'm also comfy just doing whatever.

Also though, I'm curious what your use case is, and if I can address it more directly for ya. I know I put you through all this GraphQL business back when it all popped off, and I wonder if it might be good to keep simplifying our way off of it and into endpoints that just do what you need.

Makes sense to me overall! I have a few questions about details. I also imagine, if we were being real serious, we'd want to expand this into a whole `AltStyle` GraphQL type… but I'm also comfy just doing whatever. Also though, I'm curious what your use case is, and if I can address it more directly for ya. I know I put you through all this GraphQL business back when it all popped off, and I wonder if it might be good to keep simplifying our way off of it and into endpoints that just do what you need.

src/server/types/Pet.js

					
				@ -57,0 +81,4 @@

				          if (altColorPetState) {

				            return { id: altColorPetState.id, altStyleId, altColorId };

				          }

				        }

matchu commented

2026-01-07 15:41:27 -08:00

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.)

src/server/types/PetAppearance.js

					
				@ -273,6 +285,7 @@ const resolvers = {

				  },

				  PetAppearance: {

				    altColorId: ({ altColorId }) => (altColorId),

matchu commented

2026-01-07 15:42:20 -08:00

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.)

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u add-altStyleId:dice-add-altStyleId

git checkout dice-add-altStyleId

Merge

Merge the changes and update on Forgejo.

git checkout main

git merge --no-ff dice-add-altStyleId

git checkout main

git merge --ff-only dice-add-altStyleId

git checkout dice-add-altStyleId

git rebase main

git checkout main

git merge --no-ff dice-add-altStyleId

git checkout main

git merge --squash dice-add-altStyleId

git checkout main

git merge --ff-only dice-add-altStyleId

git checkout main

git merge dice-add-altStyleId

git push origin main

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: OpenNeo/impress-2020#2