Skip to content

feat: implement custom pets in Openclaude#1011

Draft
devNull-bootloader wants to merge 3 commits intoGitlawb:mainfrom
devNull-bootloader:feat/custom-pets
Draft

feat: implement custom pets in Openclaude#1011
devNull-bootloader wants to merge 3 commits intoGitlawb:mainfrom
devNull-bootloader:feat/custom-pets

Conversation

@devNull-bootloader
Copy link
Copy Markdown
Contributor

This pull request introduces a new structure for supporting user-created "custom pets" within the application, including type definitions, validation logic, and integration into the application state. The most significant changes are the addition of new types and validation utilities for custom pets, as well as updates to the application state to store and manage these pets.

Custom Pet Types and Validation Utilities:

  • Added customPetTypes.ts with comprehensive type definitions for custom pets, including stats, rarity, traits, and custom replies, as well as validation functions to ensure data integrity when creating or loading custom pets.
  • Exported the new custom pet types (CustomPet, CustomPetBones, CustomPetSoul, StoredCustomPets) from types.ts for use elsewhere in the codebase.

Application State Integration:

  • Updated the AppState type to include customPets (an array of user-created pets) and equippedCustomPetId (the ID of the currently active custom pet).
  • Modified the default application state in getDefaultAppState() to initialize customPets as an empty array and equippedCustomPetId as undefined.
  • Imported the CustomPet type in AppStateStore.ts to support the new state properties.

@gnanam1990
Copy link
Copy Markdown
Collaborator

Thanks for the contribution, @devNull-bootloader, and welcome. I confirmed src/buddy/ is a real subsystem (the companion / sprite / isBuddyEnabled feature flag), so extending it is a sensible direction in principle. Before I do a deep code review, three questions I'd like to get aligned on first:

  1. Buddy roadmap alignment. Is a custom-pet creation wizard something the buddy subsystem owners want? The existing companion.ts / CompanionSprite.tsx code is a fairly opinionated single-companion design; layering a user-customizable pet system on top of it is a meaningful design step. cc @kevincodex1 / @anandh8x — would appreciate a quick yes/no on whether this should be merged or stays out of scope.

  2. @inquirer/prompts as a new dependency. The codebase uses Ink for interactive UI (the prompt input, command UI, etc.). Adding @inquirer/prompts ^4.3.1 introduces a second prompt UX with a different look and behavior, and a new top-level dep. Is there a reason Ink doesn't work for this wizard? If we can stay on Ink, that'd keep the surface area smaller.

  3. Tests. 642 lines added across 5 files, no test coverage. The validation utilities in customPetTypes.ts (the isValid* predicates) and the wizard's input parsing are exactly the kind of things that benefit from unit tests. Existing src/buddy/ files don't have many tests either, but new code adding ~500 lines of validation/parsing should probably come with a customPetTypes.test.ts at minimum.

Happy to do a full code review once 1 and 2 are settled. Thanks again.

Copy link
Copy Markdown
Collaborator

@Vasanthdev2004 Vasanthdev2004 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Targeted maintainer triage of current head c7c9ad597e16e3b34619ce63c5d1341f738263ad; not a full code review because the PR is draft and CI is failing before tests run.

Verdict: Needs changes

Blocking issue:

  1. CI fails during bun install --frozen-lockfile with lockfile had changes, but lockfile is frozen. This PR changes package.json, so bun.lock needs to be regenerated/committed against current main before the test suite can even run.

What I checked:

  • Changed files are limited to the custom pet types/wizard/state surface plus package.json.
  • The current failing job does not reach the unit tests; it stops at dependency installation.

Please rebase onto current main, commit the updated lockfile if the dependency change is intentional, and rerun CI. After that, the actual custom-pet state/validation flow can get a real review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants