W3 Trapping: add owned/collected critters summary by TolakaD · Pull Request #309 · Sludging/idleon-efficiency

TolakaD · 2026-03-10T15:24:46Z

Add total of owned critters to a trapping page;
Add number of critters that are going to be collected;
Add critters that are going to be collected per trap (in hover view);
Make cells with critters to wrap during resizing instead of overlapping;
Refactor traps.tsx file to use TrapBoxRepo for trap times and bonuses instead of hard-coded values;
Refactor traps.tsx file to use ItemDetailRepo for number of traps per trap set type instead of hard-coded values;
Introduce ResourceCountTile in base components for critters summary tiles. Potentially can be reused for cooking spices.

…toring

vercel · 2026-03-10T15:24:51Z

@TolakaD is attempting to deploy a commit to the Idleon Efficiency Team on Vercel.

A member of the Team first needs to authorize it.

vercel · 2026-03-12T00:28:33Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
idleon-efficiency	Ready	Preview, Comment	Mar 14, 2026 5:08am

Sludging

Left some comments, mostly not blocking but I'd like to maintain some conventions so keen to see if you are happy to address them yourselves or want me to look at it post merge as a follow-up PR.

Sludging · 2026-03-12T02:43:21Z

+    const critters = initCritterRepo();
+    const critterCounts = {
+        regular: critters.map(c => ({
+            id: c.id,
+            count: storage?.amountInStorage(c.id) ?? 0,
+            location: c.data.location,
+        })),
+        shiny: critters.map(c => ({
+            id: c.data.shiny,
+            count: storage?.amountInStorage(c.data.shiny) ?? 0,
+            location: c.data.location,
+        })),
+    };
+
+    const totalRewards = new Map<string, number>();
+    playerTraps
+        .flat()
+        .filter((trap) => trap?.placed)
+        .forEach((trap) => {
+            const current = totalRewards.get(trap.critterName) ?? 0;
+            totalRewards.set(trap.critterName, current + (trap.critters ?? 0));
+        });
+
+    const trapRewards = Array.from(totalRewards.entries())
+        .map(([name, count]) => ({ name, count }))
+        .sort((a, b) => a.name.localeCompare(b.name));


The codebase follows a convention where page components (app/*/content.tsx) pull pre-calculated data from the domain — they aim (unless really inconvinent or depending on user behavior) to not do aggregations or init*Repo() directly.

So as an example for this code portion:

Repo data should be initialized in the domain layer and surfaced through the store, not re-called every render

Critter count aggregation and trap reward totals - this kind of logic belongs in the domain layer so the component just pulls the result from theData.

Thanks for explanation. Fixed by moving repo's initialization and calculation logic to the traps.tsx file.

Sludging · 2026-03-12T02:44:29Z

@@ -0,0 +1,37 @@
+import { Box, Text } from 'grommet';


ResourceCountTile is very similar to the existing ResourceDisplay component at
components/account/shared/ResourceDisplay.tsx.

What do you think about rather than introducing a new component, promoting ResourceDisplay from
components/account/shared/ to components/base/ (it's generic enough to live there) and reusing
it for the critter/reward tiles.

If it needs small additions like the border/background styling, those can be added as props or handled by the caller wrapping it in a Box.

Good point, didn't know what I can reuse for this data. Fixed by removing new component and switching to ResourceDisplay component. Didn't promote it to components/base though, not to expand the scope of this PR.
I would rather open a separate PR for that.

Sludging · 2026-03-12T02:45:43Z

-        {duration: 2419e3, qty: 550, shiny: 1150}
-    ],
-];
+const trapBoxBySetIndex = Object.fromEntries(


Is there a reason you went with module level constants for trapIdByName and trapBoxBySetIndex?

The more common pattern is to set the values as class properties (similar to how Sneaking, Farming, etc. do it).

Having them as class properties will lead to recalculating the same static data over and over again, this is why I've left them at the module level. Note that previously they were at module level as well which made sense for me :)

We only initialize our domain classes once (at least .. in theory lol) but that's a good point overall. Happy to keep it like that.

Sludging · 2026-03-12T02:49:28Z

+    .map(trapBox => [trapBox.index, trapBox.data.times])
+) as Record<number, TrapBoxTimeModel[]>;

 export class Trap {


To do some of the work I suggested you might need to create a "wrapper" Traps class that inside it holds the traps information (the current data returned) + the new fields.

It's more work (and most of it busy work) compare to what you have done but I think it would be a nice challenge + helps align a misaligned old code to newer conventions.

What do you think? you ok with picking that up?

Fixed. I didn't create a new class but refactored the existing Traps class in traps.tsx.
I hope the change addresses what you've meant, please let me know otherwise.

Oh I forgot there was already a Traps domain class 😆

Yeah the changes look good, good work!

Sludging · 2026-03-12T02:50:36Z

+                <Box 
+                    border={{ side: 'top', color: 'grey-1', size: '1px' }}
+                    margin={{ top: 'small' }}
+                    pad={{ top: 'small' }}
+                    align="end"
+                >
+                    <Text margin={{ bottom: "small" }}>Trap Rewards</Text>
+                    <TrapRewardsSummary entries={trapRewards} />
+                </Box>


I feel this is hidden at the bottom, what do you think making it top right (i.e. right above the table and below the owned critters section)?

I think it will be more visible and more aligned with other newer pages (like grimoire/tesseract etc)

Tried and it looks not good in that place. Grimoire/tesseract have pages have cards and this style fits nicely there, but we have a table here :) I've put it between owned critters and traps table, please let me know what you think.

Thanks for trying, and yeah left side between the two looks fine to me as well!

Sludging

Thanks for working through my requested changes and for delivering on this feature, I'll merge it in now as I'm happy with how it shaped up!

W3 Trapping: add owned/collected critters summary and traps.tsx refac…

69db017

…toring

TolakaD requested a review from Sludging as a code owner March 10, 2026 15:24

vercel Bot deployed to Preview March 12, 2026 00:31 View deployment

Sludging reviewed Mar 12, 2026

View reviewed changes

Fix PR comments

3b8e6ba

vercel Bot deployed to Preview March 14, 2026 05:08 View deployment

Sludging approved these changes Mar 14, 2026

View reviewed changes

Sludging merged commit a8c1a1e into Sludging:main Mar 14, 2026
3 of 4 checks passed

Conversation

TolakaD commented Mar 10, 2026

Uh oh!

vercel Bot commented Mar 10, 2026

Uh oh!

vercel Bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sludging left a comment

Choose a reason for hiding this comment

Uh oh!

Sludging Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sludging left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel Bot commented Mar 12, 2026 •

edited

Loading

Sludging Mar 12, 2026 •

edited

Loading