Fix storage unit naming#27
Conversation
Handle the client's zero-id built-in casket naming path, initialize storage metadata, and avoid sending empty tool removal messages.
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
csgo_gc/gc_client.cpp (1)
1727-1729: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winConditionally dispatch the destroy update in
NameBaseItem.Similar to the fix applied above in
NameItem, this function unconditionally sends thek_ESOMsg_Destroymessage. SinceInventory::NameBaseItemonly populates thedestroyobject whenGetConfig().DestroyUsedItems()is true, thedestroyobject can remain empty here.Wrap the dispatch in a type-id check to prevent sending empty tool-removal messages. As per the relevant code snippets, ServerGC runs
ValidateMessageOwnerSOIDfork_ESOMsg_Destroymessages, which will fail if the object is empty.🐛 Proposed fix
SendMessageToGame(true, k_ESOMsg_Create, create); - SendMessageToGame(true, k_ESOMsg_Destroy, destroy); + if (destroy.has_type_id()) + { + SendMessageToGame(true, k_ESOMsg_Destroy, destroy); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@csgo_gc/gc_client.cpp` around lines 1727 - 1729, Update Inventory::NameBaseItem so k_ESOMsg_Destroy is dispatched only when the destroy object has a valid type ID, while continuing to send k_ESOMsg_Create unconditionally. Use the destroy object's existing type-id accessor/check to avoid sending an empty removal message.
🧹 Nitpick comments (1)
csgo_gc/inventory.cpp (1)
1227-1230: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid redundant attribute search.
Since
FindOrAddAttributeinternally callsFindAttributeas its first step, calling it right after!FindAttribute(...)results in a redundant linear scan. You can directly add the attribute here to skip the second search.♻️ Proposed refactor
- CSOEconItemAttribute *countAttribute = - FindOrAddAttribute(casket, ItemSchema::AttributeCasketItemsCount); + CSOEconItemAttribute *countAttribute = casket.add_attribute(); + countAttribute->set_def_index(ItemSchema::AttributeCasketItemsCount);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@csgo_gc/inventory.cpp` around lines 1227 - 1230, Update the casket attribute handling around AttributeCasketItemsCount to avoid calling FindOrAddAttribute immediately after FindAttribute has confirmed the attribute is absent. Directly create or add the count attribute in that branch, preserving the existing initialization and behavior without performing a second linear search.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@csgo_gc/gc_client.cpp`:
- Around line 1727-1729: Update Inventory::NameBaseItem so k_ESOMsg_Destroy is
dispatched only when the destroy object has a valid type ID, while continuing to
send k_ESOMsg_Create unconditionally. Use the destroy object's existing type-id
accessor/check to avoid sending an empty removal message.
---
Nitpick comments:
In `@csgo_gc/inventory.cpp`:
- Around line 1227-1230: Update the casket attribute handling around
AttributeCasketItemsCount to avoid calling FindOrAddAttribute immediately after
FindAttribute has confirmed the attribute is absent. Directly create or add the
count attribute in that branch, preserving the existing initialization and
behavior without performing a second linear search.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fe2e9a7-0b96-42ac-aa27-b9f30d66899b
📒 Files selected for processing (2)
csgo_gc/gc_client.cppcsgo_gc/inventory.cpp
Avoid dispatching empty base-item removal messages and eliminate a redundant casket attribute lookup.
Resolve #23.
Summary by CodeRabbit
New Features
Bug Fixes