Dinonuggie added#152
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@utils/claim.ts`:
- Around line 180-230: processClaim currently performs separate reads and writes
(client.db.user.getUserAttr, addUserAttr, setUserAttr) so concurrent requests
can both pass the cooldown check and double-credit dinonuggies; make the entire
read-check-write sequence atomic by using a DB transaction or a per-user lock
around processClaim's logic (wrap the calls that read dinonuggiesLastClaimed,
dinonuggiesClaimStreak, dinonuggies and the subsequent addUserAttr/setUserAttr
updates) so only one execution for a given uid can perform the cooldown check
and updates at a time; implement this by using your DB client's transaction API
(or a mutex keyed by uid) to enclose the cooldown branch and the
reward/broken_streak branches, ensuring you roll back on failure and return the
same ClaimResult semantics from processClaim.
- Around line 202-205: The broken-streak branch stores the raw value returned by
getBaseAmount(uid, 0) which can be fractional, causing a mismatch with normal
claims that use Math.ceil via getAmount; fix it by rounding the computed amount
before persisting—call Math.ceil on the value returned from getBaseAmount (or
reuse getAmount to produce the final rounded result) and then pass that rounded
value into client.db.user.addUserAttr(uid, 'dinonuggies', roundedAmount) and use
the same rounded value when updating 'dinonuggiesLastClaimed' logic so persisted
and displayed values match.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 94cdf75d-abfd-409b-8c83-64b0f0123122
📒 Files selected for processing (9)
.env.examplecommands/claim.tssite_src/Assets/Images/game-dinonuggie.avifsite_src/Assets/Images/game-dinonuggie.webpsite_src/bot-bridge.tssite_src/pages/games.tssite_src/pages/games/claim.tssite_src/server.tsutils/claim.ts
| async function processClaim(client: any, uid: string): Promise<ClaimResult> { | ||
| const now = Date.now(); | ||
| const lastClaimedInt = await client.db.user.getUserAttr(uid, 'dinonuggiesLastClaimed'); | ||
| const lastClaimed = lastClaimedInt || null; | ||
| const diff = lastClaimed ? now - lastClaimed : DAY_LENGTH; | ||
|
|
||
| const streak = await client.db.user.getUserAttr(uid, 'dinonuggiesClaimStreak'); | ||
| const dinonuggies = await client.db.user.getUserAttr(uid, 'dinonuggies'); | ||
| const bekiLevel = await client.db.user.getUserAttr(uid, 'bekiLevel'); | ||
| const cooldown = getBekiCooldown(bekiLevel); | ||
|
|
||
| if (diff < cooldown * HOUR_LENGTH) { | ||
| const selected = BEKI_COOLDOWN_RESPONSES[Math.floor(Math.random() * BEKI_COOLDOWN_RESPONSES.length)]; | ||
| return { | ||
| status: 'cooldown', | ||
| title: selected.title, | ||
| gifUrl: selected.gifUrl, | ||
| hoursRemaining: cooldown - diff / HOUR_LENGTH, | ||
| cooldown, | ||
| }; | ||
| } | ||
|
|
||
| if (diff > 2 * DAY_LENGTH) { | ||
| const amount = await getBaseAmount(client, uid, 0); | ||
| await client.db.user.addUserAttr(uid, 'dinonuggies', amount); | ||
| await client.db.user.setUserAttr(uid, 'dinonuggiesLastClaimed', now); | ||
| await client.db.user.setUserAttr(uid, 'dinonuggiesClaimStreak', 1); | ||
| return { | ||
| status: 'broken_streak', | ||
| amount, | ||
| previousDinonuggies: dinonuggies, | ||
| previousStreak: streak, | ||
| }; | ||
| } | ||
|
|
||
| const reward = await getAmount(client, uid, streak); | ||
| await client.db.user.addUserAttr(uid, 'dinonuggies', reward.amount); | ||
| await client.db.user.setUserAttr(uid, 'dinonuggiesLastClaimed', now); | ||
| await client.db.user.addUserAttr(uid, 'dinonuggiesClaimStreak', 1); | ||
| return { | ||
| status: 'success', | ||
| amount: reward.amount, | ||
| title: reward.title, | ||
| imageUrl: reward.imageUrl, | ||
| colour: reward.colour, | ||
| footer: reward.footer, | ||
| thumbnail: reward.thumbnail, | ||
| previousDinonuggies: dinonuggies, | ||
| previousStreak: streak, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Make the claim mutation atomic.
processClaim() does the cooldown check and then updates balance / last-claimed / streak in separate DB calls. Because both commands/claim.ts and site_src/server.ts now funnel through this helper, two near-simultaneous requests for the same user can both pass the checks and both credit dinonuggies before dinonuggiesLastClaimed is updated. Please wrap this in a transaction or a per-user lock so the whole read/check/write sequence is serialized.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/claim.ts` around lines 180 - 230, processClaim currently performs
separate reads and writes (client.db.user.getUserAttr, addUserAttr, setUserAttr)
so concurrent requests can both pass the cooldown check and double-credit
dinonuggies; make the entire read-check-write sequence atomic by using a DB
transaction or a per-user lock around processClaim's logic (wrap the calls that
read dinonuggiesLastClaimed, dinonuggiesClaimStreak, dinonuggies and the
subsequent addUserAttr/setUserAttr updates) so only one execution for a given
uid can perform the cooldown check and updates at a time; implement this by
using your DB client's transaction API (or a mutex keyed by uid) to enclose the
cooldown branch and the reward/broken_streak branches, ensuring you roll back on
failure and return the same ClaimResult semantics from processClaim.
| if (diff > 2 * DAY_LENGTH) { | ||
| const amount = await getBaseAmount(client, uid, 0); | ||
| await client.db.user.addUserAttr(uid, 'dinonuggies', amount); | ||
| await client.db.user.setUserAttr(uid, 'dinonuggiesLastClaimed', now); |
There was a problem hiding this comment.
Round broken-streak rewards before persisting them.
On Line 203 the broken-streak path credits raw getBaseAmount(...), while normal claims always round via Math.ceil(...) inside getAmount(). If any multiplier produces a fractional base, this branch stores fractional dinonuggies and the web UI will display a rounded value that does not match what was actually persisted.
Suggested fix
- const amount = await getBaseAmount(client, uid, 0);
+ const amount = Math.ceil(await getBaseAmount(client, uid, 0));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (diff > 2 * DAY_LENGTH) { | |
| const amount = await getBaseAmount(client, uid, 0); | |
| await client.db.user.addUserAttr(uid, 'dinonuggies', amount); | |
| await client.db.user.setUserAttr(uid, 'dinonuggiesLastClaimed', now); | |
| if (diff > 2 * DAY_LENGTH) { | |
| const amount = Math.ceil(await getBaseAmount(client, uid, 0)); | |
| await client.db.user.addUserAttr(uid, 'dinonuggies', amount); | |
| await client.db.user.setUserAttr(uid, 'dinonuggiesLastClaimed', now); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/claim.ts` around lines 202 - 205, The broken-streak branch stores the
raw value returned by getBaseAmount(uid, 0) which can be fractional, causing a
mismatch with normal claims that use Math.ceil via getAmount; fix it by rounding
the computed amount before persisting—call Math.ceil on the value returned from
getBaseAmount (or reuse getAmount to produce the final rounded result) and then
pass that rounded value into client.db.user.addUserAttr(uid, 'dinonuggies',
roundedAmount) and use the same rounded value when updating
'dinonuggiesLastClaimed' logic so persisted and displayed values match.
1 participant
in memory of our inactive member bekers
@coderabbitai review you lil shit
Summary by CodeRabbit