Feature: Restrict Jutsu by Specific Items #995
Feature: Restrict Jutsu by Specific Items #995milanig-creator wants to merge 11 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Jutsu JSON field Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/UI
participant JutsuAPI as Jutsu Router (server)
participant ItemAPI as Item Router (server)
participant Lib as train.ts
participant DB as Database
Client->>JutsuAPI: POST /startTraining (jutsuId)
JutsuAPI->>DB: fetch jutsu & user
JutsuAPI->>ItemAPI: fetchUserItems(userId)
ItemAPI->>DB: query user items + item relations
DB-->>ItemAPI: user items with relations
ItemAPI-->>JutsuAPI: user items (with relations)
JutsuAPI->>Lib: canTrainJutsu(jutsu, userWithAllItems)
Lib->>Lib: checkJutsuItems(requiredItemIds, requireEquipped?)
Lib-->>JutsuAPI: boolean (canTrain)
JutsuAPI-->>Client: 200 OK / 400 Forbidden
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 review |
✅ Actions performedReview triggered.
|
![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
🤖 Fix all issues with AI agents
In `@app/drizzle/schema.ts`:
- Line 1321: The json column definition for requiredItemIds in schema.ts is
missing the Drizzle default, causing mismatch with the migration; update the
column declaration requiredItemIds:
json("requiredItemIds").$type<string[]>().notNull() to include .default([])
(matching the pattern used by requiredSkillIds) so Drizzle will apply an
empty-array default on inserts.
In `@app/src/layout/EditContent.tsx`:
- Around line 582-625: displayValue can be undefined which makes Textarea switch
between uncontrolled and controlled; update the render inside FormField in
EditContent.tsx to default displayValue to an empty string (and for array fields
default to [] before join) so Textarea always receives a string: change the
displayValue calculation (and any use of (field.value as string[]) and
(field.value as string)) to use nullish coalescing (e.g. (field.value as
string[] ?? []).join("\n") and (field.value as string) ?? "") and leave Textarea
value={displayValue} as-is to keep it controlled; identifiers: FormField render,
isArrayField, displayValue, handleChange, Textarea.
🧹 Nitpick comments (1)
app/src/app/jutsus/page.tsx (1)
274-282: Remove unnecessaryas anycast —requiredItemIdsis already properly typed.The
Jutsutype is inferred from the drizzle schema (which definesrequiredItemIds: json("requiredItemIds").$type<string[]>()), so theas anycast at lines 276–277 is redundant and bypasses TypeScript's type safety. The field can be accessed directly.♻️ Suggested refactor
if (!checkJutsuItems(uj.jutsu, userItems, true)) { + const requiredItemIds = uj.jutsu.requiredItemIds; const hasRequiredItems = - Array.isArray((uj.jutsu as any).requiredItemIds) && - (uj.jutsu as any).requiredItemIds.length > 0; + Array.isArray(requiredItemIds) && requiredItemIds.length > 0; if (hasRequiredItems) { warning = "Missing required item(s) for this jutsu. Item must be equipped."; } else { warning = `No ${uj.jutsu.jutsuWeapon.toLowerCase()} weapon equipped.`; } }
milanig-creator
added
new feature
📦 Next.js Bundle Analysis for tnrThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
1 similar comment
📦 Next.js Bundle Analysis for tnrThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
Greptile OverviewConfidence Score: 4/5
Important Files Changed
Flowchartflowchart TD
A[Content Staff: Edit Jutsu] --> B[Input Item IDs via Textarea]
B --> C{Validate Items Exist}
C -->|Invalid| D[Error: Item Not Found]
C -->|Valid| E[Save requiredItemIds to DB]
F[Player: Train Jutsu] --> G{Check canTrainJutsu}
G --> H{Has Required Items?}
H -->|Check Ownership| I{Owns All Items?}
I -->|No| J[Error: Missing Items]
I -->|Yes| K[Allow Training]
L[Player: Equip Jutsu] --> M{Check canTrainJutsu}
M --> N{Has Required Items?}
N -->|Check Equipped| O{All Items Equipped?}
O -->|No| P[Error: Items Not Equipped]
O -->|Yes| Q[Allow Equip]
R[Player: Unequip Item] --> S{Any Equipped Jutsu Requires Item?}
S -->|Yes| T[Error: Cannot Unequip]
S -->|No| U[Allow Unequip]
V[Player: Sell Item] --> W{Any Equipped Jutsu Requires Item?}
W -->|Yes| X[Error: Cannot Sell]
W -->|No| Y[Allow Sale]
Z[Combat: Use Jutsu] --> AA{checkJutsuItems with requireEquipped=true}
AA -->|Failed| AB[Block Jutsu Usage]
AA -->|Passed| AC[Allow Jutsu Usage]
Last reviewed commit: 068f530 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export const checkJutsuItems = ( | ||
| jutsu: Jutsu, | ||
| userItems: UserItemWithItem[] | undefined, | ||
| userItems: UserItem[] | undefined, | ||
| requireEquipped = true, | ||
| userItemsWithItem?: UserItemWithItem[], | ||
| ) => { | ||
| const requiredItemIds = jutsu.requiredItemIds; | ||
| const hasRequiredItems = | ||
| Array.isArray(requiredItemIds) && requiredItemIds.length > 0; | ||
|
|
||
| // If specific required items are defined (AND semantics - ALL items required) | ||
| if (hasRequiredItems) { | ||
| if (requireEquipped) { | ||
| // For combat/usage: require ALL items to be equipped | ||
| const allItemsEquipped = requiredItemIds.every((reqId: string) => | ||
| userItems?.some((ui) => ui.itemId === reqId && ui.equipped !== "NONE"), | ||
| ); | ||
| if (!allItemsEquipped) return false; | ||
| } else { | ||
| // For training: require ALL items to be owned (in inventory) | ||
| const allItemsOwned = requiredItemIds.every((reqId: string) => | ||
| userItems?.some((ui) => ui.itemId === reqId), | ||
| ); | ||
| if (!allItemsOwned) return false; | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| // Otherwise fall back to weapon-type requirement | ||
| if (jutsu.jutsuWeapon !== "NONE") { | ||
| const equippedItem = userItems?.find( | ||
| (useritem) => | ||
| useritem.item.weaponType === jutsu.jutsuWeapon && useritem.equipped !== "NONE", | ||
| // Prefer relations if provided; otherwise try to infer when available. If we | ||
| // cannot access weaponType, fail closed (no weapon = requirement not met). | ||
| const itemsWithRelation = | ||
| userItemsWithItem ?? | ||
| userItems?.filter((ui): ui is UserItemWithItem => "item" in ui); | ||
|
|
||
| if (!itemsWithRelation || itemsWithRelation.length === 0) return false; | ||
|
|
||
| const equippedWeapon = itemsWithRelation.find( | ||
| (ui) => ui.item.weaponType === jutsu.jutsuWeapon && ui.equipped !== "NONE", | ||
| ); | ||
| if (!equippedItem) return false; | ||
| if (!equippedWeapon) return false; | ||
| } | ||
| return true; | ||
| }; |
There was a problem hiding this comment.
The function signature accepts both UserItem[] and UserItemWithItem[] but attempts type narrowing that could fail silently. When userItemsWithItem is not provided and userItems doesn't have the item relation loaded, the weapon-type check at line 200 will return false even if valid weapons are equipped.
For the weapon-type fallback logic (lines 195-208), if relations aren't available, the function fails closed. However, this could incorrectly prevent valid jutsu usage in contexts where only UserItem[] (without relations) is passed and jutsuWeapon !== "NONE".
Consider requiring userItemsWithItem when checking weapon types, or document that callers must provide items with relations when jutsuWeapon requirements exist.
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/libs/train.ts
Line: 166:210
Comment:
The function signature accepts both `UserItem[]` and `UserItemWithItem[]` but attempts type narrowing that could fail silently. When `userItemsWithItem` is not provided and `userItems` doesn't have the `item` relation loaded, the weapon-type check at line 200 will return `false` even if valid weapons are equipped.
For the weapon-type fallback logic (lines 195-208), if relations aren't available, the function fails closed. However, this could incorrectly prevent valid jutsu usage in contexts where only `UserItem[]` (without relations) is passed and `jutsuWeapon !== "NONE"`.
Consider requiring `userItemsWithItem` when checking weapon types, or document that callers must provide items with relations when `jutsuWeapon` requirements exist.
How can I resolve this? If you propose a fix, please make it concise.| const jutsuRequiringItem = equippedJutsus.find((uj) => { | ||
| if (!uj.jutsu) return false; | ||
| const requiredIds = (uj.jutsu as any).requiredItemIds; | ||
| return ( | ||
| Array.isArray(requiredIds) && requiredIds.includes(useritem.item.id) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Type assertion (uj.jutsu as any).requiredItemIds bypasses TypeScript type checking. The jutsu relation should already have the correct type from the schema.
| const jutsuRequiringItem = equippedJutsus.find((uj) => { | |
| if (!uj.jutsu) return false; | |
| const requiredIds = (uj.jutsu as any).requiredItemIds; | |
| return ( | |
| Array.isArray(requiredIds) && requiredIds.includes(useritem.item.id) | |
| ); | |
| }); | |
| const requiredIds = uj.jutsu.requiredItemIds; | |
| return Array.isArray(requiredIds) && requiredIds.includes(useritem.item.id); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/server/api/routers/item.ts
Line: 497:503
Comment:
Type assertion `(uj.jutsu as any).requiredItemIds` bypasses TypeScript type checking. The `jutsu` relation should already have the correct type from the schema.
```suggestion
const requiredIds = uj.jutsu.requiredItemIds;
return Array.isArray(requiredIds) && requiredIds.includes(useritem.item.id);
```
How can I resolve this? If you propose a fix, please make it concise.| const jutsuRequiringItem = equippedJutsus.find((uj) => { | ||
| if (!uj.jutsu) return false; | ||
| const requiredIds = (uj.jutsu as any).requiredItemIds; | ||
| return Array.isArray(requiredIds) && requiredIds.includes(useritem.item.id); | ||
| }); |
There was a problem hiding this comment.
Type assertion (uj.jutsu as any).requiredItemIds bypasses TypeScript type checking.
| const jutsuRequiringItem = equippedJutsus.find((uj) => { | |
| if (!uj.jutsu) return false; | |
| const requiredIds = (uj.jutsu as any).requiredItemIds; | |
| return Array.isArray(requiredIds) && requiredIds.includes(useritem.item.id); | |
| }); | |
| const requiredIds = uj.jutsu.requiredItemIds; | |
| return Array.isArray(requiredIds) && requiredIds.includes(useritem.item.id); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/server/api/routers/item.ts
Line: 1679:1683
Comment:
Type assertion `(uj.jutsu as any).requiredItemIds` bypasses TypeScript type checking.
```suggestion
const requiredIds = uj.jutsu.requiredItemIds;
return Array.isArray(requiredIds) && requiredIds.includes(useritem.item.id);
```
How can I resolve this? If you propose a fix, please make it concise.| render={({ field, fieldState }) => { | ||
| // Special handling for array fields: convert to/from newline-separated text | ||
| const isArrayField = Array.isArray(field.value); | ||
| const displayValue = isArrayField | ||
| ? (field.value as string[]).join("\n") | ||
| : ((field.value ?? "") as string); | ||
|
|
||
| const handleChange = (e: React.ChangeEvent<HTMLTextAreaElement>) => { | ||
| if (isArrayField) { | ||
| // Convert newline-separated text to array | ||
| const arr = e.target.value | ||
| .split("\n") | ||
| .map((s) => s.trim()) | ||
| .filter((s) => s.length > 0); | ||
| field.onChange(arr); | ||
| } else { | ||
| field.onChange(e.target.value); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The array detection logic Array.isArray(field.value) determines behavior, but this couples the UI behavior to runtime data shape rather than explicit configuration. If field.value is undefined or initially an empty string, the textarea won't convert to array format.
Consider adding an explicit isArrayField property to the FormEntry type for textarea entries that should be treated as arrays, rather than inferring from runtime value type.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/layout/EditContent.tsx
Line: 586:604
Comment:
The array detection logic `Array.isArray(field.value)` determines behavior, but this couples the UI behavior to runtime data shape rather than explicit configuration. If `field.value` is undefined or initially an empty string, the textarea won't convert to array format.
Consider adding an explicit `isArrayField` property to the FormEntry type for textarea entries that should be treated as arrays, rather than inferring from runtime value type.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
MathiasGruber
force-pushed
the
feature/restrict-jutsu-by-item
branch
from
00006ba to
a8f0385
Compare
MathiasGruber
left a comment
MathiasGruber
left a comment
There was a problem hiding this comment.
Think it needs revisiting, the textbox-approach to entering item IDs is not going to be well-received by content.
NOTE: I've rebased your branch on top of main, since I did some major refactoring that had lots of conflicts. Please check out the latest remote version 👍
| @@ -577,6 +579,50 @@ export const EditContent = < | |||
| }} | |||
| /> | |||
| )} | |||
There was a problem hiding this comment.
Please implement it similar to other db-selection features we have, where you actually get a dropdown in the admin panel:
| @@ -0,0 +1,2 @@ | |||
| -- Add requiredItemIds column to Jutsu table | |||
| ALTER TABLE `Jutsu` ADD COLUMN `requiredItemIds` json NOT NULL DEFAULT ('[]'); | |||
There was a problem hiding this comment.
When creating migrations, always do it with the make makemigrations command so that we get the journal files as well
There was a problem hiding this comment.
I was not understanding the significance of Migrations and after sitting with this one for awhile its slowly starting to make sense. I used make migrations again and I believe it was not needed. I used git hub to resolve the migration conflicts after my latest push.
Thank you for your patience.
| readonly?: boolean; | ||
| } & ( | ||
| | { type: "text" } | ||
| | { type: "textarea" } |
app/src/server/api/routers/item.ts
Outdated
| // Check if any equipped jutsu requires this item | ||
| const jutsuRequiringItem = equippedJutsus.find((uj) => { | ||
| if (!uj.jutsu) return false; | ||
| const requiredIds = (uj.jutsu as any).requiredItemIds; |
There was a problem hiding this comment.
Type assertion bypasses TypeScript. The jutsu relation should already have requiredItemIds from the schema.
| const requiredIds = (uj.jutsu as any).requiredItemIds; | |
| const requiredIds = uj.jutsu.requiredItemIds; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/server/api/routers/item.ts
Line: 500:500
Comment:
Type assertion bypasses TypeScript. The `jutsu` relation should already have `requiredItemIds` from the schema.
```suggestion
const requiredIds = uj.jutsu.requiredItemIds;
```
How can I resolve this? If you propose a fix, please make it concise.
app/src/server/api/routers/item.ts
Outdated
| }); | ||
| const jutsuRequiringItem = equippedJutsus.find((uj) => { | ||
| if (!uj.jutsu) return false; | ||
| const requiredIds = (uj.jutsu as any).requiredItemIds; |
There was a problem hiding this comment.
Type assertion bypasses TypeScript.
| const requiredIds = (uj.jutsu as any).requiredItemIds; | |
| const requiredIds = uj.jutsu.requiredItemIds; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/server/api/routers/item.ts
Line: 1686:1686
Comment:
Type assertion bypasses TypeScript.
```suggestion
const requiredIds = uj.jutsu.requiredItemIds;
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/libs/train.ts`:
- Around line 166-210: checkJutsuItems can return false when called from
canTrainJutsu because userdata.items is plain UserItem[] (no item relation) so
the type guard in checkJutsuItems (UserItemWithItem => "item" in ui) yields an
empty itemsWithRelation and weapon-type checks fail; fix by ensuring callers
pass items that include the item relation or by loading relations before
calling: update canTrainJutsu to call checkJutsuItems(jutsu, userdata.items,
false, userdata.itemsWithItem) or change the data fetch in callers to populate
the item relation on userdata.items so checkJutsuItems receives UserItemWithItem
entries when jutsu.jutsuWeapon !== "NONE".
Signed-off-by: milanig-creator <milangonzalez4@gmail.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📦 Next.js Bundle Analysis for tnrThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
| checkJutsuBloodline(jutsu, userdata) && | ||
| checkJutsuElements(jutsu, userElements) | ||
| checkJutsuElements(jutsu, userElements) && | ||
| checkJutsuItems(jutsu, userdata.items, false) // Training only needs ownership |
There was a problem hiding this comment.
userdata.items may not include the full item relations needed for weapon type checks. The function signature changed to accept UserItem[] but weapon type logic needs UserItemWithItem[]. Pass userdata.items as the 4th parameter userItemsWithItem to ensure weapon checks work correctly.
| checkJutsuItems(jutsu, userdata.items, false) // Training only needs ownership | |
| checkJutsuItems(jutsu, userdata.items, false, userdata.items) // Training only needs ownership |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/libs/train.ts
Line: 261:261
Comment:
`userdata.items` may not include the full item relations needed for weapon type checks. The function signature changed to accept `UserItem[]` but weapon type logic needs `UserItemWithItem[]`. Pass `userdata.items` as the 4th parameter `userItemsWithItem` to ensure weapon checks work correctly.
```suggestion
checkJutsuItems(jutsu, userdata.items, false, userdata.items) // Training only needs ownership
```
How can I resolve this? If you propose a fix, please make it concise.
app/src/server/api/routers/combat.ts
Outdated
| // Not if cannot train jutsu | ||
| if (battleType !== "RANKED_PVP" && battleType !== "RANKED_SPARRING") { | ||
| if (!checkJutsuItems(userjutsu.jutsu, user.items) && !user.isAi) { | ||
| if (!checkJutsuItems(userjutsu.jutsu, user.items, true) && !user.isAi) { |
There was a problem hiding this comment.
user.items may not include item relations. Pass as 4th parameter for weapon type checks:
| if (!checkJutsuItems(userjutsu.jutsu, user.items, true) && !user.isAi) { | |
| if (!checkJutsuItems(userjutsu.jutsu, user.items, true, user.items) && !user.isAi) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/server/api/routers/combat.ts
Line: 2579:2579
Comment:
`user.items` may not include item relations. Pass as 4th parameter for weapon type checks:
```suggestion
if (!checkJutsuItems(userjutsu.jutsu, user.items, true, user.items) && !user.isAi) {
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/server/api/routers/jutsu.ts (1)
842-905:⚠️ Potential issue | 🟠 MajorFiltering by
canTrainJutsublocks unequip when requirements change.Because the list is filtered before lookup, a jutsu that no longer meets requirements (e.g., newly added
requiredItemIds) is treated as missing, so users can’t unequip it. Use the full list for lookup and only gate equipping.🔧 Suggested fix (allow lookup/unequip)
- const filteredJutsus = userjutsus.filter((uj) => canTrainJutsu(uj.jutsu, userWithAllItems)); - const userjutsuObj = filteredJutsus.find((j) => j.id === input.userJutsuId); + const userjutsuObj = userjutsus.find((j) => j.id === input.userJutsuId); const isEquipped = userjutsuObj?.equipped || false; - const equippedJutsus = filteredJutsus.filter((j) => j.equipped); + const equippedJutsus = userjutsus.filter((j) => j.equipped); const curEquip = equippedJutsus.length || 0;
🤖 Fix all issues with AI agents
In `@app/src/libs/train.ts`:
- Line 21: The weapon-type requirement check currently returns !requireEquipped
when itemsWithRelation is empty, allowing weapon-locked jutsu to train with zero
items; update the logic so the bypass only applies when the user actually has
items (e.g. change the condition to require itemsWithRelation.length > 0 before
using !requireEquipped). Locate the check that uses itemsWithRelation and
requireEquipped (the training eligibility function/block) and replace the loose
falsy check with an explicit items existence check, and apply the same fix to
the corresponding block referenced around lines 166-209 in this file.
In `@app/src/server/api/routers/jutsu.ts`:
- Around line 433-463: The mapping currently picks the first match in allItems
and can return a wrong id when an input equals both an item's name and another
item's id; update the lookup logic to prefer id matches (or reject ambiguous
inputs). Modify the invalidInputs check and the mapping for
input.data.requiredItemIds so each val first tries to find an item with i.id ===
val, and only if not found falls back to i.name === val; optionally detect
ambiguity when there exist both a name-match and a different id-match and either
prefer the id-match explicitly or return an error for that val. Use the existing
symbols (input.data.requiredItemIds, itemsByName, itemsById, allItems, found) to
locate and change the checks and the map callback.
… ID over name, and ensure correct types for item checks.
|
@milanig-creator you might ned to resolve these conflicts so terr can merge |
|
@MathiasGruber when you come online can you review please. |
Signed-off-by: milanig-creator <milangonzalez4@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/app/jutsus/page.tsx`:
- Around line 313-315: checkJutsuItems currently only returns false for two
distinct failure modes (missing requiredItemIds vs wrong weapon) so the warning
message near the `if (!checkJutsuItems(uj.jutsu, userItems, true)) { warning =
\`No ${uj.jutsu.jutsuWeapon.toLowerCase()} weapon equipped.\`; }` is misleading;
change the code so you can distinguish the two cases — either update
`checkJutsuItems` to return a discriminated result (e.g., { ok: boolean, reason:
'missingRequiredItems'|'missingWeapon' } or an enum) or add a small helper
`hasRequiredItemIds(uj.jutsu, userItems)` that checks `uj.jutsu.requiredItemIds`
against `userItems`; then use that result to set the message (e.g., "Missing
required item(s)." when requiredItemIds are unmet, otherwise the existing "No
... weapon equipped."), referencing `checkJutsuItems`, `hasRequiredItemIds` /
`uj.jutsu.requiredItemIds`, and the `warning` assignment.
| if (!checkJutsuItems(uj.jutsu, userItems, true)) { | ||
| warning = `No ${uj.jutsu.jutsuWeapon.toLowerCase()} weapon equipped.`; | ||
| } |
There was a problem hiding this comment.
Warning message doesn't account for the new requiredItemIds path.
When checkJutsuItems returns false because specific requiredItemIds are not satisfied (the first branch in the function), the warning still says "No {weapon} weapon equipped.", which is misleading. Consider distinguishing the two failure modes — e.g., checking whether jutsu.requiredItemIds is populated to show a different message like "Missing required item(s)."
Suggested approach
- if (!checkJutsuItems(uj.jutsu, userItems, true)) {
- warning = `No ${uj.jutsu.jutsuWeapon.toLowerCase()} weapon equipped.`;
- }
+ if (!checkJutsuItems(uj.jutsu, userItems, true)) {
+ const hasSpecificItems =
+ Array.isArray(uj.jutsu.requiredItemIds) &&
+ uj.jutsu.requiredItemIds.length > 0;
+ warning = hasSpecificItems
+ ? "Missing required item(s) for this jutsu."
+ : `No ${uj.jutsu.jutsuWeapon.toLowerCase()} weapon equipped.`;
+ }📝 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 (!checkJutsuItems(uj.jutsu, userItems, true)) { | |
| warning = `No ${uj.jutsu.jutsuWeapon.toLowerCase()} weapon equipped.`; | |
| } | |
| if (!checkJutsuItems(uj.jutsu, userItems, true)) { | |
| const hasSpecificItems = | |
| Array.isArray(uj.jutsu.requiredItemIds) && | |
| uj.jutsu.requiredItemIds.length > 0; | |
| warning = hasSpecificItems | |
| ? "Missing required item(s) for this jutsu." | |
| : `No ${uj.jutsu.jutsuWeapon.toLowerCase()} weapon equipped.`; | |
| } |
🤖 Prompt for AI Agents
In `@app/src/app/jutsus/page.tsx` around lines 313 - 315, checkJutsuItems
currently only returns false for two distinct failure modes (missing
requiredItemIds vs wrong weapon) so the warning message near the `if
(!checkJutsuItems(uj.jutsu, userItems, true)) { warning = \`No
${uj.jutsu.jutsuWeapon.toLowerCase()} weapon equipped.\`; }` is misleading;
change the code so you can distinguish the two cases — either update
`checkJutsuItems` to return a discriminated result (e.g., { ok: boolean, reason:
'missingRequiredItems'|'missingWeapon' } or an enum) or add a small helper
`hasRequiredItemIds(uj.jutsu, userItems)` that checks `uj.jutsu.requiredItemIds`
against `userItems`; then use that result to set the message (e.g., "Missing
required item(s)." when requiredItemIds are unmet, otherwise the existing "No
... weapon equipped."), referencing `checkJutsuItems`, `hasRequiredItemIds` /
`uj.jutsu.requiredItemIds`, and the `warning` assignment.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: milanig-creator <admin@kuzushistudio.com>
| import type { CaptchaVerifySchema } from "@/validators/misc"; | ||
| import { captchaVerifySchema } from "@/validators/misc"; | ||
| import { getSearchValidator } from "@/validators/register"; | ||
| import { cn } from "../../libs/shadui"; |
There was a problem hiding this comment.
Relative import breaks convention
The rest of the codebase (71 files) imports cn using the @/libs/shadui path alias. This was changed to a relative path, breaking the project convention.
| import { cn } from "../../libs/shadui"; | |
| import { cn } from "@/libs/shadui"; |
Context Used: Context from dashboard - CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/app/traininggrounds/page.tsx
Line: 100:100
Comment:
**Relative import breaks convention**
The rest of the codebase (71 files) imports `cn` using the `@/libs/shadui` path alias. This was changed to a relative path, breaking the project convention.
```suggestion
import { cn } from "@/libs/shadui";
```
**Context Used:** Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=0827434e-cdb7-4415-b790-c54f1a8e8feb))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| {students.map((user, i) => ( | ||
| <div className="relative" key={i}> |
There was a problem hiding this comment.
Index-based React key is a regression
The original code used user.userId as the key, which is a stable, unique identifier. Changing to the array index i can cause incorrect rendering when the student list is reordered or updated. Consider reverting to the original userId-based key.
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/app/traininggrounds/page.tsx
Line: 264:265
Comment:
**Index-based React key is a regression**
The original code used `user.userId` as the key, which is a stable, unique identifier. Changing to the array index `i` can cause incorrect rendering when the student list is reordered or updated. Consider reverting to the original userId-based key.
How can I resolve this? If you propose a fix, please make it concise.| // Semantic wrapper for training checks | ||
| export const hasRequiredItemsForTraining = ( | ||
| jutsu: Jutsu, | ||
| userItems: UserItem[] | undefined, | ||
| userItemsWithItem?: UserItemWithItem[], | ||
| ) => checkJutsuItems(jutsu, userItems, false, userItemsWithItem); | ||
|
|
||
| // Semantic wrapper for combat/usage checks | ||
| export const hasRequiredItemsEquippedForUse = ( | ||
| jutsu: Jutsu, | ||
| userItems: UserItem[] | undefined, | ||
| userItemsWithItem?: UserItemWithItem[], | ||
| ) => checkJutsuItems(jutsu, userItems, true, userItemsWithItem); |
There was a problem hiding this comment.
Dead code: unused wrapper functions
hasRequiredItemsForTraining and hasRequiredItemsEquippedForUse are defined here but never imported or called anywhere in the codebase. All call sites use checkJutsuItems directly. Either remove these dead exports or refactor call sites to use them for consistency.
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/libs/train.ts
Line: 241:253
Comment:
**Dead code: unused wrapper functions**
`hasRequiredItemsForTraining` and `hasRequiredItemsEquippedForUse` are defined here but never imported or called anywhere in the codebase. All call sites use `checkJutsuItems` directly. Either remove these dead exports or refactor call sites to use them for consistency.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: app/src/app/traininggrounds/page.tsx
Line: 579:579
Comment:
**Wrong lint suppression comment format**
This project uses Biome for linting (see `biome.json`), not ESLint. The `eslint-disable-next-line` comment will have no effect. The original `biome-ignore` comment should be preserved. The same issue applies to the `eslint-disable-next-line react-hooks/exhaustive-deps` added at line 630.
How can I resolve this? If you propose a fix, please make it concise. |
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: milanig-creator <admin@kuzushistudio.com>
| if (useritem.storedAtHome) return errorResponse("Fetch at home first"); | ||
| const doEquip = slot ? useritem.equipped !== slot : useritem.equipped === "NONE"; | ||
|
|
||
| if (useritem.storedAtHome) return errorResponse("Fetch at home first"); | ||
| const doEquip = slot ? useritem.equipped !== slot : useritem.equipped === "NONE"; |
There was a problem hiding this comment.
Lines 1750-1751 and 1753-1754 are duplicated
| if (useritem.storedAtHome) return errorResponse("Fetch at home first"); | |
| const doEquip = slot ? useritem.equipped !== slot : useritem.equipped === "NONE"; | |
| if (useritem.storedAtHome) return errorResponse("Fetch at home first"); | |
| const doEquip = slot ? useritem.equipped !== slot : useritem.equipped === "NONE"; | |
| if (useritem.storedAtHome) return errorResponse("Fetch at home first"); | |
| const doEquip = slot ? useritem.equipped !== slot : useritem.equipped === "NONE"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/server/api/routers/item.ts
Line: 1750:1754
Comment:
Lines 1750-1751 and 1753-1754 are duplicated
```suggestion
if (useritem.storedAtHome) return errorResponse("Fetch at home first");
const doEquip = slot ? useritem.equipped !== slot : useritem.equipped === "NONE";
```
How can I resolve this? If you propose a fix, please make it concise.| // Only check requirements when equipping (not when unequipping) | ||
|
|
||
| // Only check requirements when equipping (not when unequipping) |
There was a problem hiding this comment.
Duplicate comment on lines 1782 and 1784
| // Only check requirements when equipping (not when unequipping) | |
| // Only check requirements when equipping (not when unequipping) | |
| // Only check requirements when equipping (not when unequipping) | |
| if (doEquip) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/server/api/routers/item.ts
Line: 1782:1784
Comment:
Duplicate comment on lines 1782 and 1784
```suggestion
// Only check requirements when equipping (not when unequipping)
if (doEquip) {
```
How can I resolve this? If you propose a fix, please make it concise.| {userReskins?.find((r) => r.jutsuId === userjutsu.jutsuId) && | ||
| !userjutsu.activeReskin && | ||
| !userjutsu.activeReskin && ( | ||
| <div className="mt-2 rounded-lg border border-blue-200 bg-blue-50 p-2"> |
There was a problem hiding this comment.
Duplicate condition !userjutsu.activeReskin on lines 568 and 569
| {userReskins?.find((r) => r.jutsuId === userjutsu.jutsuId) && | |
| !userjutsu.activeReskin && | |
| !userjutsu.activeReskin && ( | |
| <div className="mt-2 rounded-lg border border-blue-200 bg-blue-50 p-2"> | |
| {userReskins?.find((r) => r.jutsuId === userjutsu.jutsuId) && | |
| !userjutsu.activeReskin && ( |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/app/jutsus/page.tsx
Line: 567:569
Comment:
Duplicate condition `!userjutsu.activeReskin` on lines 568 and 569
```suggestion
{userReskins?.find((r) => r.jutsuId === userjutsu.jutsuId) &&
!userjutsu.activeReskin && (
```
How can I resolve this? If you propose a fix, please make it concise.3 participants
Pull Request
This PR adds support for restricting jutsu by specific required items, similar to how jutsu can already be restricted by bloodline.
Content staff can now define a list of item IDs on a jutsu. (In order to limit the amount of data returned I made this a text box with one
itemIDper line. Input is sanitized and validated server-side to ensure all provided item IDs exist before the jutsu can be saved. Players must at minimum own those items to train the jutsu, and must have them equipped in order to use the jutsu in combat.Jutsu now support a
requiredItemIdsfield. When set, the system enforces item requirements consistently across training, UI checks, and combat execution.For the user, training only checks ownership of the required items. Usage and combat require the items to be equipped.
To prevent invalid states, players are blocked from selling or unequipping items that are still required by any currently equipped jutsu. Null-saftey checks were added to fix crashes when I tried to remove jutsu from my user on my local environment.
License
By making this pull request, I confirm that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the Studie-Tech ApS organization has the copyright to use and modify my contribution for perpetuity.
Summary by CodeRabbit
New Features
Bug Fixes / Behavior