feat: add Items and Item Templates support - #490
kirre-bylund wants to merge 4 commits into
Conversation
Adds SDK support for the new inventory system (internally "Assets 2.0"),
publicly named Items and Item Templates.
Game API endpoints (base player/inventory/v1):
- GET / ListItemTemplates
- GET /my ListPlayerItems
- GET /{id} GetPlayerItem
- DELETE /{id} DeletePlayerItem
- POST /{id}/consume ConsumePlayerItem
- POST /{id}/split SplitPlayerItemStack
- POST /merge MergePlayerItemStacks
Notes:
- List responses use LootLockerExtendedPagination (the backend returns an
extended offset pagination shape, not the simple page/per_page/total).
- metadata is typed as LootLockerMetadataEntry[].
- consumable is a nullable bool so "no filter" remains reachable; the backend
only applies the filter when the query value is non-empty.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Fix nullable count serialization and address the requested metadata, documentation, and test coverage gaps.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Adds Unity SDK support for Items and Item Templates inventory APIs.
Changes:
- Added item/template DTOs, requests, responses, and pagination.
- Added list, retrieve, delete, consume, split, and merge APIs.
- Registered inventory endpoints and public SDK methods.
| File | Summary |
|---|---|
Runtime/Game/Requests/ItemRequest.cs |
Implements item models and API requests; address nullable count serialization, add the Unity .meta file, and add JSON coverage. |
Runtime/Game/LootLockerSDKManager.cs |
Exposes public item APIs; align pagination documentation with callable signatures and add endpoint tests. |
Runtime/Client/LootLockerEndPoints.cs |
Defines the new Items endpoint routes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Add ShouldSerializecount() to LootLockerConsumeItemRequest so an unset count is
omitted entirely. This removes the Newtonsoft/ZeroDep asymmetry (only ZeroDep
skips nulls) and avoids sending {"count":null}, which cannot decode into uint64.
- Correct the consume count documentation: the backend defaults to 1, not the
whole stack. To consume an entire stack, pass the item's current count.
- Remove the incorrect "Defaults to 1." / "Defaults to 25." claims from the
page/perPage docs on ListItemTemplates and ListPlayerItems; both are required.
- Add the missing ItemRequest.cs.meta file.
- Add inventory and item_template to LootLockerMetadataSources and correct the
misleading item comment (item is the legacy source, inventory is Assets 2.0).
- Add 7 serialization/deserialization tests covering the new item endpoints.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
I'm missing functional testing of this feature. We should add some (and use the admin api parts to do the required set up of course, I've attached the current api spec for relevant endpoints). admin api.json |
kirre-bylund
left a comment
kirre-bylund
left a comment
There was a problem hiding this comment.
A bunch of things that need fixing. The same probably goes for the unreal and unreal server implementations as well.
Address PR review feedback on the Items / Item Templates DTOs: - Remove game_id from item and item template DTOs; the SDK always operates in a game context so the field is redundant. - Type item_type as LootLockerItemType instead of string. - Type created_at / updated_at as DateTime (updated_at nullable, since the backend omits it when unset). - Type granted item `type` as the new LootLockerRewardKind enum. - Add `source` to LootLockerItemSortField. - Only append query parameters when they carry a meaningful value, so unset filters are not sent as empty/zero values. - Inline the seven item methods from the LootLockerAPIManager partial class into LootLockerSDKManager, matching the convention used by the rest of the SDK. - Update the item JSON tests for the new types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add PlayMode coverage for the seven item endpoints against a live backend, using the admin API for setup (create template with a public audience, grant to the session player). - Add admin endpoint constants for the item template and player inventory routes. - Add a LootLockerTestItems helper mirroring the go-backend admin routes rather than the published ApiDog spec, which is stale for this feature: the list endpoint returns `templates` (not `items`), grant returns `id` (not `inventory_id`), consume requires a body and returns a count (not a bool), and the player inventory listing uses `template_name` and has no `deletable`. - Add ItemTests with 10 cases covering list templates, list player items (with and without filters), get, partial and whole-stack consume, split, merge, delete, and the forbidden delete of a non-deletable item. - Only the happy-path test carries LootLockerCIFast; the rest are LootLockerCI only. Also correct the ConsumePlayerItem doc comment: a null count consumes a single item, not the whole stack, matching the backend default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Functional tests added in 28760ee.
Only the happy-path template test carries Verified against a live local backend: 15/15 passed (the 10 above plus 5 pre-existing cases matched by the filter). One thing worth flagging from that run: the local |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved moderate issues remain in the public API and item filter test coverage.
Review effort: Lite
Findings: 1
Open (1)
Resolved since last review (3)
Files not reviewed (3)
- Runtime/Game/Requests/ItemRequest.cs.meta: Generated file
- Tests/LootLockerTestUtils/LootLockerTestConfigurationItem.cs.meta: Generated file
- Tests/LootLockerTests/PlayMode/ItemTests.cs.meta: Generated file
| asset = 7, | ||
| item = 8, // This is the source for asset instances (player inventory items), while the "asset" source is for the asset in general | ||
| item = 8, // This is the source for legacy items, while the "inventory" source is for item instances | ||
| inventory = 9, // This is the source for item instances (player inventory items) |
There was a problem hiding this comment.
Shouldn't this be named something else then?
An inventory is something that contains items in my mind.
There was a problem hiding this comment.
I'm with Johannes here. We spent quite a bit of time coming up with the name Item Templates to avoid confusion, so I really don't think we should use other names in the SDK
Summary
Adds SDK support for the new inventory system, publicly named Items and Item Templates.
Endpoints added (base
player/inventory/v1)/ListItemTemplates/myListPlayerItems/{id}GetPlayerItem/{id}DeletePlayerItem/{id}/consumeConsumePlayerItem/{id}/splitSplitPlayerItemStack/mergeMergePlayerItemStacksNomenclature
Template/item_template_id).Inventory/inventory_id).Public SDK names use "Item"/"Item Template" to avoid colliding with the legacy Inventory/Asset APIs. Wire field names are preserved in the DTOs.
Implementation notes
LootLockerExtendedPagination. The backend returns an extended offset shape (per_page, offset, total, last_page, current_page, next_page, prev_page, errors), not the simplepage/per_page/total.LootLockerMetadataEntry[](the wire shape is an array of{key, value, type, access, tags}).consumablefilter: exposed asbool?so "no filter" remains reachable. The backend only applies the filter when the query value is non-empty, so always sendingtrue/falsewould silently force a filter.granted[]on consume matches the backendGrantResultshape:{source_id, count, type, name, code}.item_typeisLootLockerItemType,granted[].typeisLootLockerRewardKind, and timestamps areDateTime/DateTime?.game_idis omitted — it is redundant in a game-context SDK.ConsumePlayerItem: omittingcountconsumes one item, not the whole stack. To consume an entire stack, pass the item's current count.LootLockerSDKManager.cs; there is no partial-class indirection.Functional tests
Tests/LootLockerTests/PlayMode/ItemTests.csadds 10 PlayMode tests covering the full lifecycle, with all setup performed through the admin API (Tests/LootLockerTestUtils/LootLockerTestConfigurationItem.cs):Items_ListItemTemplates_ReturnsCreatedTemplate—LootLockerCIFastItems_ListPlayerItems_ReturnsGrantedItemItems_ListPlayerItems_WithFilters_ReturnsFilteredItemsItems_GetPlayerItem_ReturnsItemWithTemplateItems_ConsumePlayerItem_PartialStack_DecrementsCountItems_ConsumePlayerItem_WholeStack_RemovesItemItems_DeletePlayerItem_RemovesItemItems_DeletePlayerItem_NotDeletable_ReturnsForbiddenItems_SplitPlayerItemStack_CreatesSecondStackItems_MergePlayerItemStacks_CombinesCountsOnly the happy-path template test carries
LootLockerCIFast; the rest areLootLockerCI.Verification
go-backendsource rather than the ApiDog spec, which was stale for several of these endpoints.Related