Skip to content

[Code Review Fixes] Claude Code Review & Fixes for Evolving items PR#100

Merged
Valorith merged 2 commits intocodex/evolving-item-review-fixes-masterfrom
claude/code-review-report-Fb8aZ
Mar 22, 2026
Merged

[Code Review Fixes] Claude Code Review & Fixes for Evolving items PR#100
Valorith merged 2 commits intocodex/evolving-item-review-fixes-masterfrom
claude/code-review-report-Fb8aZ

Conversation

@Valorith
Copy link
Copy Markdown
Owner

  • Fix ALL_EXP branch logic: raid/solo exp rates were unreachable when ALL_EXP was set because it short-circuited to group rate. Restructured to check raid first, then group, then solo fallback.
  • Move progression >= 100 checks inside mob null guards for SPECIFIC_MOB_RACE, SPECIFIC_ZONE_ID, and NUMBER_OF_KILLS cases to prevent unnecessary queue additions when mob is null.
  • Fix copy-paste log message in NUMBER_OF_KILLS case: was incorrectly logged as "Type 4 Specific Zone ID" instead of "Type 5 Number of Kills".
  • Guard .at("evolve") calls in SetEvolveEquipped with .contains() check to prevent std::out_of_range if called before timer is created.

https://claude.ai/code/session_01VgYPW4cYy3mzh17LKFQs58

Description

Please include a summary of the changes and the related issue (Why is this change necessary). Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Testing

Attach images and describe testing done to validate functionality.

Clients tested:

Checklist

  • I have tested my changes
  • I have performed a self-review of my code. Ensuring variables, functions and methods are named in a human-readable way, comments are added only where naming of variables, functions and methods can't give enough context.
  • I have made corresponding changes to the documentation (if applicable, if not delete this line)
  • I own the changes of my code and take responsibility for the potential issues that occur
  • If my changes make database schema changes, I have tested the changes on a local database (attach image). Updated version.h CURRENT_BINARY_DATABASE_VERSION to the new version. (Delete this if not applicable)

- Fix ALL_EXP branch logic: raid/solo exp rates were unreachable when
  ALL_EXP was set because it short-circuited to group rate. Restructured
  to check raid first, then group, then solo fallback.
- Move progression >= 100 checks inside mob null guards for
  SPECIFIC_MOB_RACE, SPECIFIC_ZONE_ID, and NUMBER_OF_KILLS cases to
  prevent unnecessary queue additions when mob is null.
- Fix copy-paste log message in NUMBER_OF_KILLS case: was incorrectly
  logged as "Type 4 Specific Zone ID" instead of "Type 5 Number of Kills".
- Guard .at("evolve") calls in SetEvolveEquipped with .contains() check
  to prevent std::out_of_range if called before timer is created.

https://claude.ai/code/session_01VgYPW4cYy3mzh17LKFQs58
@Valorith Valorith requested a review from Copilot March 22, 2026 16:19
@Valorith Valorith changed the title Fix evolving item code review findings [Code Review Fixes] Claude Code Review & Fixes for Evolving items PR Mar 22, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses several correctness and robustness issues in the evolving item system, primarily around experience subtype selection, kill/zone/race processing guards, and safer timer access.

Changes:

  • Fix ALL_EXP subtype branching so raid/group/solo experience rates are applied in the intended priority order (raid → group → solo).
  • Ensure progression-complete items are only queued when the relevant mob-dependent condition is actually processed (avoids unnecessary queue additions when mob is null).
  • Prevent std::out_of_range in SetEvolveEquipped by guarding GetTimers().at("evolve") with .contains().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
zone/client_evolving_items.cpp Corrects experience subtype branching, tightens mob-guarded progression queueing, and adjusts a kill-type log message.
common/item_instance.cpp Adds a safety guard around "evolve" timer access to avoid .at() throwing when the timer is not present.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread zone/client_evolving_items.cpp Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Valorith Valorith merged commit 749e606 into codex/evolving-item-review-fixes-master Mar 22, 2026
3 checks passed
@Valorith Valorith deleted the claude/code-review-report-Fb8aZ branch March 22, 2026 16:27
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