Skip to content

[08] Reload item enchants and random props#89

Open
alseif0x wants to merge 5 commits into
3.4.3from
issue-20-dc1-dc2-reload-item-props
Open

[08] Reload item enchants and random props#89
alseif0x wants to merge 5 commits into
3.4.3from
issue-20-dc1-dc2-reload-item-props

Conversation

@alseif0x

@alseif0x alseif0x commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Refs #20

QA pending: do not merge until relog/capture validation, CI, and Codex reviewer verdict are complete. This is a partial #20 slice and must not close the issue.

Summary:

  • Implements the D-C1/D-C2 slice: reload item enchantments and random properties on relog.
  • Extends item equipment/bag SQL loads and runtime item create serialization.
  • Leaves D-C3, D-C4, D-C5/D-C6, D-C8, and D-C9 for later PRs.

Validation reported locally:

  • PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo test -p wow-world loaded_item_instance_fields --lib
  • cargo test -p wow-packet item_create_serializes_random_properties_and_context --lib
  • cargo test -p wow-database char_sql_has_correct_placeholders --lib
  • PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo check -p world-server
  • cargo fmt --all -- --check
  • git diff --check

Manual QA pending: equip enchanted/random-prop item, relog, compare item state and create packets vs C++.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ac0cd2789

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/wow-world/src/handlers/character.rs
@alseif0x

alseif0x commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

@codex review\n\nPushed e4d1f67 for the review thread. Loaded top-level equipped items now run the represented ApplyEnchantment path after inventory hydration, including canonical duration registration and ItemEnchantTimeUpdate emission for duration enchants. Anchored to C++ Player::_LoadInventory -> _ApplyAllItemMods -> ApplyEnchantment.

alseif0x commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

QA validation checklist:

  • Equip an item with a visible/permanent enchant, relog, and confirm the enchant visual/effect remains applied.
  • If available, apply a temporary weapon enchant, relog, and confirm the temporary enchant still exists with a sane duration and is not duplicated.
  • Equip an item with random property/suffix data, relog, and confirm tooltip/stats still match the item.
  • Keep an enchanted item in bags, relog, then equip it; bag-loaded enchants should not incorrectly apply until equipped.
  • Verify normal login, attack/cast, and logout still work.

Note: CI/reviewer status must be green on the current head before this is merge-ready.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e4d1f670ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/wow-world/src/handlers/character.rs
Comment thread crates/wow-world/src/session.rs
@alseif0x

alseif0x commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Pushed 21ad0de to address the actionable comments: item load now mirrors C++ SetItemRandomProperties before DB enchant string parsing so random property/suffix slots are rebuilt when enchantments is empty, and loaded equipped enchantments now apply represented effect actions in addition to duration updates.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21ad0de4de

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/wow-world/src/session.rs
Comment thread crates/wow-world/src/handlers/character.rs Outdated
Comment thread crates/wow-world/src/handlers/character.rs Outdated
@alseif0x

alseif0x commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

Review fixes pushed in 36068b5.\n\nAddressed current Codex feedback:\n- Permanent enchant visuals are now computed into login VisibleItems from SpellItemEnchantment.db2 via the loaded item model.\n- Loaded enchant replay now applies state before login CREATE but queues duration/stat/visible UpdateObject packets until after send_login_sequence, so referenced item/player objects already exist client-side.\n- Equipped enchant replay now includes profession slots PROFESSION_SLOT_START..PROFESSION_SLOT_END as well as normal equipment slots.\n\nValidated locally:\n- PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo test -p wow-world loaded_item_instance_fields --lib\n- PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo test -p wow-world loaded_item_slots_apply_equipped_enchantments_for_equipment_and_profession_like_cpp --lib\n- PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo test -p wow-world loaded_equipped_item_enchantments --lib\n- PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo test -p wow-world current_player_item_enchantment --lib\n- cargo fmt --all -- --check\n- git diff --check\n- PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo check -p world-server\n\n@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36068b5ee5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/wow-world/src/handlers/character.rs Outdated
Comment thread crates/wow-world/src/session.rs
Comment thread crates/wow-world/src/session.rs
@alseif0x

alseif0x commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Pushed d0fad19d for the current review feedback:

  • Loaded equipped enchant replay now runs after set_player_skill_records_like_cpp, so required-skill enchants see the loaded skill values before applying plans.
  • Random property/suffix slots are rebuilt when the DB enchantment payload is absent or a valid all-zero string; non-empty DB enchantments still override the synthesized random slots.
  • Loaded enchant effect replay now records real effect actions in the outcome and in represented item-bonus action history. Stat-like actions still update represented state; non-live actions such as CastEquipSpell/UpdateDamageDoneMods are exposed as unrepresented_effect_actions instead of being silently dropped.
  • Player registry and cross-session player CREATE now use the same loaded visible-item helper, so permanent enchant visuals are present for nearby clients during login/create, not only as a later self update.

Validation:

  • PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo test -p wow-world loaded_item_instance_fields --lib
  • PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo test -p wow-world loaded_equipped_item_enchantments --lib
  • PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo test -p wow-world loaded_player_visible_items_for_create_includes_loaded_enchant_visual_like_cpp --lib
  • PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo test -p wow-world loaded_item_slots_apply_equipped_enchantments_for_equipment_and_profession_like_cpp --lib
  • PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo test -p wow-world current_player_item_enchantment --lib
  • cargo fmt --all -- --check
  • git diff --check
  • PROTOC=/home/cdmonio/.local/protoc/bin/protoc cargo check -p world-server

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: d0fad19d55

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant