Skip to content

feat(inventory-list): add item counter#186

Open
zndxcvbn wants to merge 7 commits into
mainfrom
feat-item-counter
Open

feat(inventory-list): add item counter#186
zndxcvbn wants to merge 7 commits into
mainfrom
feat-item-counter

Conversation

@zndxcvbn

@zndxcvbn zndxcvbn commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Resolve #185.

Summary by CodeRabbit

  • New Features
    • Added configurable item-count display modes for inventory lists: hidden, per-row counts, or stacked totals (default: stacked).
    • List headers now show the item count beside the name column when enabled.
    • Counts update dynamically with list refreshes and respect active sorting and filtering.

@coderabbitai

coderabbitai Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@zndxcvbn has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 43 minutes and 41 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a9520620-9b2b-485d-b010-7b64b4404825

📥 Commits

Reviewing files that changed from the base of the PR and between 576a11f and 5d5a4ba.

📒 Files selected for processing (1)
  • source/swfsources.cmake
📝 Walkthrough

Walkthrough

Adds an item-count display feature: new config key controls display mode; TabularList computes counts per mode and filters by current name-column sort; SortedListHeader gains updateItemCount and renders a CountColumn; build sources include the new ActionScript files.

Changes

Cohort / File(s) Summary
Configuration
data/interface/skyui/config.txt
Adds [ItemList] itemCount.mode configuration key (default 2) to control item-count display mode (hidden / per-row / stacked aggregation).
List Header
source/actionscript/Common/skyui/components/list/SortedListHeader.as
Adds _itemCount state and public updateItemCount(Number); lazily creates/caches a CountColumn MovieClip, applies text format and label, toggles visibility, repositions header buttons, and adjusts buttonArea._width.
Tabular List
source/actionscript/Common/skyui/components/list/TabularList.as
Loads config.ItemList.itemCount.mode (clamped), overrides UpdateList() to compute totalItemsCount (mode-aware: count each row or sum entry.count), filters by primary name-column sort when applicable, and calls header.updateItemCount(...).
Build Sources
source/swfsources.cmake
Registers TabularList.as and SortedListHeader.as for craftingmenu and skyui_inventorylists SWF targets so the new components are compiled into those SWFs.

Sequence Diagram(s)

sequenceDiagram
    participant TabularList
    participant SortedListHeader
    participant CountColumn
    participant UI

    TabularList->>TabularList: UpdateList()
    TabularList->>TabularList: Read config.ItemList.itemCount.mode
    TabularList->>TabularList: Iterate rows, compute totalItemsCount (mode/filter aware)
    TabularList->>SortedListHeader: updateItemCount(totalItemsCount)
    SortedListHeader->>SortedListHeader: if count >= 0 create/cache CountColumn
    SortedListHeader->>CountColumn: apply text format, set label "(N)"
    SortedListHeader->>CountColumn: position based on sort state
    SortedListHeader->>SortedListHeader: adjust buttonArea._width
    CountColumn->>UI: render count next to Name column
    opt count < 0
        SortedListHeader->>CountColumn: hide CountColumn
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • doodlum
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The PR implements the core requirement from #185 (display item counter next to Name heading), but does not fully satisfy the optional enchanted-item counting functionality mentioned in the issue. Confirm whether the enchanted-item counting feature is intended to be included in this PR or deferred to a follow-up issue.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(inventory-list): add item counter' directly and clearly summarizes the main change—adding an item counter feature to the inventory list system.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the item counter feature; no out-of-scope modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 43 minutes and 41 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@source/actionscript/Common/skyui/components/list/SortedListHeader.as`:
- Around line 6-7: The header counter is initialized visible and hiding only the
clip leaves stale hitbox widths; change the default to start hidden by setting
_countColumn._visible = false (and set _itemCount to a sentinel if used for
visibility) and update any hide/show code to both toggle _countColumn._visible
and immediately reflow the header hitboxes—adjust buttonArea._width (subtract or
add _countColumn._width) or call the existing reflow/invalidate layout routine
(e.g., reflowHeaderHitboxes or invalidateLayout) after changing visibility so
the header hitboxes update immediately.

In `@source/actionscript/Common/skyui/components/list/TabularList.as`:
- Around line 87-89: The code in TabularList.as accesses this._layout and
this._layout.columnLayoutData directly (activeColIdx =
this._layout.activeColumnIndex; and checking columnLayoutData[...] against
ListLayout.COL_TYPE_NAME) without null/bounds checks; add a defensive guard at
the start of the block to ensure this._layout is non-null,
this._layout.columnLayoutData exists, and activeColIdx is a valid index before
using it (or bail/return/choose a safe default), updating references to
activeColIdx and the type check accordingly to avoid init-order null
dereferences.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8ae6250c-c709-4172-bb0b-5f6b85b694cc

📥 Commits

Reviewing files that changed from the base of the PR and between 1523d12 and e295ef7.

📒 Files selected for processing (4)
  • data/interface/skyui/config.txt
  • source/actionscript/Common/skyui/components/list/SortedListHeader.as
  • source/actionscript/Common/skyui/components/list/TabularList.as
  • source/swfsources.cmake

Comment thread source/actionscript/Common/skyui/components/list/SortedListHeader.as Outdated
Comment thread source/actionscript/Common/skyui/components/list/TabularList.as

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
source/actionscript/Common/skyui/components/list/TabularList.as (1)

87-89: ⚠️ Potential issue | 🟠 Major

Add index/entry guard before active-column type access.

Line 87-Line 88 still assume activeColIdx maps to a valid columnLayoutData entry. If the index is out of range/stale, .type can dereference undefined.

Suggested fix
-        var activeColIdx = this._layout.activeColumnIndex;
-        var isActiveColName = (this._layout.columnLayoutData[activeColIdx].type == skyui.components.list.ListLayout.COL_TYPE_NAME);
+        var activeColIdx = this._layout.activeColumnIndex;
+        var columnData = this._layout.columnLayoutData[activeColIdx];
+        if (activeColIdx == undefined || activeColIdx < 0 || columnData == undefined) {
+            this.header.updateItemCount(-1);
+            return;
+        }
+        var isActiveColName = (columnData.type == skyui.components.list.ListLayout.COL_TYPE_NAME);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@source/actionscript/Common/skyui/components/list/TabularList.as` around lines
87 - 89, The code accesses this._layout.columnLayoutData[activeColIdx].type
without verifying the index or entry exists; update the logic around
activeColIdx (used in TabularList.as) to first check that this._layout and
this._layout.columnLayoutData are defined and that activeColIdx is a valid index
(0 <= activeColIdx < columnLayoutData.length) and that
columnLayoutData[activeColIdx] is not null/undefined before reading .type, and
default isActiveColName to false when the guard fails; reference the variables
activeColIdx, _layout, columnLayoutData and the constant
ListLayout.COL_TYPE_NAME when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@source/actionscript/Common/skyui/components/list/TabularList.as`:
- Around line 87-89: The code accesses
this._layout.columnLayoutData[activeColIdx].type without verifying the index or
entry exists; update the logic around activeColIdx (used in TabularList.as) to
first check that this._layout and this._layout.columnLayoutData are defined and
that activeColIdx is a valid index (0 <= activeColIdx < columnLayoutData.length)
and that columnLayoutData[activeColIdx] is not null/undefined before reading
.type, and default isActiveColName to false when the guard fails; reference the
variables activeColIdx, _layout, columnLayoutData and the constant
ListLayout.COL_TYPE_NAME when making the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9291a660-ca9b-4eab-af0f-10f3e732b5e8

📥 Commits

Reviewing files that changed from the base of the PR and between e295ef7 and 60c1118.

📒 Files selected for processing (3)
  • data/interface/skyui/config.txt
  • source/actionscript/Common/skyui/components/list/SortedListHeader.as
  • source/actionscript/Common/skyui/components/list/TabularList.as
🚧 Files skipped from review as they are similar to previous changes (2)
  • data/interface/skyui/config.txt
  • source/actionscript/Common/skyui/components/list/SortedListHeader.as

@zndxcvbn zndxcvbn changed the title feat(item-menu): add item counter feat(inventory-list): add item counter Apr 29, 2026
@zndxcvbn zndxcvbn added deferred Will be considered at a later date labels May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deferred Will be considered at a later date

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an item counter to inventory

1 participant