Flash Talisman: ANY_KAMI shape for self-use#2353
Open
kanyaveral wants to merge 3 commits intomainfrom
Open
Conversation
Introduce ANY_KAMI for-shape so Flash Talisman (11412) works through both KamiUseItemSystem (self-buff) and KamiCastItemSystem (enemy cast). - items.csv: Enemy_Kami → Any_Kami, add BYPASS_BONUS_RESET flag - LibItem.sol: add verifyForShapeOr() helper - KamiCastItemSystem: accept ENEMY_KAMI or ANY_KAMI - KamiUseItemSystem: accept KAMI or ANY_KAMI - CastItemButton/UseItemButton: include ANY_KAMI in inventory filters
kanyaveral
requested review from
ExistentialEnso,
JirAcheron,
Xenofluxx,
chlwys,
deeterbleater and
hubur
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThe changes extend the item casting and usage system to support both ENEMY_KAMI and ANY_KAMI item shapes. Client-side buttons now aggregate and reorder items by type, while contract systems introduce dual-shape verification and enforce authorization-based state checks for item casting. Changes
Sequence Diagram(s)sequenceDiagram
participant Button as CastItemButton/<br/>UseItemButton
participant System as KamiCastItemSystem/<br/>KamiUseItemSystem
participant LibItem as LibItem Library
Button->>Button: Filter & aggregate<br/>ENEMY_KAMI + ANY_KAMI items
Button->>System: Call with item selection
alt Unauthorized Caster (KamiCastItemSystem)
System->>System: Check authorization
System->>System: Enforce HARVESTING state
end
System->>LibItem: verifyForShapeOr(...<br/>"ENEMY_KAMI"/"ANY_KAMI")
LibItem->>LibItem: Retrieve item shape<br/>via LibFor.get
LibItem->>LibItem: Compare shape<br/>against both options
LibItem-->>System: Shape verified ✓
System->>System: Continue with remaining<br/>validations & execution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
ExistentialEnso
previously approved these changes
Feb 11, 2026
Contributor
ExistentialEnso
left a comment
ExistentialEnso
left a comment
There was a problem hiding this comment.
lgtm! pretty straightforward but an incredibly useful feature.
kanyaveral
dismissed
ExistentialEnso’s stale review
The base branch was changed.
Xenofluxx
previously approved these changes
Feb 13, 2026
- enemy kamis must be in HARVESTING state to be targeted by cast items - own kamis (via ANY_KAMI items) can still be targeted while RESTING or HARVESTING - prevents spirit glue and similar items from being used on resting enemy kamis
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/contracts/src/libraries/LibItem.sol (1)
298-304: Improve dual-shape revert message clarity.Validation is correct, but the revert reason only mentions the first accepted shape. Including both accepted shapes will make debugging/operator support clearer.
Proposed patch
function verifyForShapeOr( IUintComp components, uint32 index, string memory a, string memory b ) public view { string memory shape = LibFor.get(components, genID(index)); - if (!shape.eq(a) && !shape.eq(b)) revert(LibString.concat("not for ", a)); + if (!shape.eq(a) && !shape.eq(b)) { + revert(LibString.concat("not for ", LibString.concat(a, LibString.concat(" or ", b)))); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/contracts/src/libraries/LibItem.sol` around lines 298 - 304, The revert message in verifyForShapeOr currently only mentions the first accepted shape; update the revert to include both accepted shapes by building a clear message (e.g., "not for <a> or <b>") before reverting. Locate function verifyForShapeOr which calls LibFor.get(components, genID(index)) and checks shape.eq(a) and shape.eq(b); replace the LibString.concat call used in revert with a concatenation that includes both a and b (using LibString.concat or existing string utilities) so the revert contains both accepted shapes for clearer debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/contracts/src/systems/KamiCastItemSystem.sol`:
- Around line 26-29: The log at the own/enemy ownership branch is hardcoded to
"ENEMY_KAMI" and mislabels own-target casts; update the logging logic in
KamiCastItemSystem.sol to choose the label based on
LibKami.checkAccount(components, targetID, accID) (use "ENEMY_KAMI" when that
check is false and a distinct "OWN_KAMI" or equivalent label when true) and
ensure any subsequent behavior that relies on that log string uses the new
conditional value; adjust the place where verifyState("HARVESTING") is called to
remain unchanged but ensure the log selection is conditional around that call.
---
Nitpick comments:
In `@packages/contracts/src/libraries/LibItem.sol`:
- Around line 298-304: The revert message in verifyForShapeOr currently only
mentions the first accepted shape; update the revert to include both accepted
shapes by building a clear message (e.g., "not for <a> or <b>") before
reverting. Locate function verifyForShapeOr which calls LibFor.get(components,
genID(index)) and checks shape.eq(a) and shape.eq(b); replace the
LibString.concat call used in revert with a concatenation that includes both a
and b (using LibString.concat or existing string utilities) so the revert
contains both accepted shapes for clearer debugging.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/client/src/app/components/library/buttons/actions/CastItemButton.tsxpackages/client/src/app/components/library/buttons/actions/UseItemButton.tsxpackages/contracts/src/libraries/LibItem.solpackages/contracts/src/systems/KamiCastItemSystem.solpackages/contracts/src/systems/KamiUseItemSystem.sol
Comment on lines
+26
to
+29
| // enemy kamis must be harvesting (exposed on node) | ||
| if (!LibKami.checkAccount(components, targetID, accID)) { | ||
| LibKami.verifyState(components, targetID, "HARVESTING"); | ||
| } |
Contributor
There was a problem hiding this comment.
Fix target-shape logging after introducing own-target cast path.
The new ownership branch allows non-enemy targets, but Line 46 still logs "ENEMY_KAMI" unconditionally. This misclassifies own-target casts.
Proposed patch
- // enemy kamis must be harvesting (exposed on node)
- if (!LibKami.checkAccount(components, targetID, accID)) {
+ bool isOwnTarget = LibKami.checkAccount(components, targetID, accID);
+
+ // enemy kamis must be harvesting (exposed on node)
+ if (!isOwnTarget) {
LibKami.verifyState(components, targetID, "HARVESTING");
}
@@
- LibItem.logUse(components, accID, itemIndex, 1, "ENEMY_KAMI");
+ LibItem.logUse(components, accID, itemIndex, 1, isOwnTarget ? "KAMI" : "ENEMY_KAMI");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/contracts/src/systems/KamiCastItemSystem.sol` around lines 26 - 29,
The log at the own/enemy ownership branch is hardcoded to "ENEMY_KAMI" and
mislabels own-target casts; update the logging logic in KamiCastItemSystem.sol
to choose the label based on LibKami.checkAccount(components, targetID, accID)
(use "ENEMY_KAMI" when that check is false and a distinct "OWN_KAMI" or
equivalent label when true) and ensure any subsequent behavior that relies on
that log string uses the new conditional value; adjust the place where
verifyState("HARVESTING") is called to remain unchanged but ensure the log
selection is conditional around that call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
ANY_KAMIfor-shape so Flash Talisman can be used on your own Kami (self-buff) in addition to enemy casting. Currently players work around this by casting from an alt account.verifyForShapeOr()helper that accepts two valid shapesENEMY_KAMIorANY_KAMIKAMIorANY_KAMIANY_KAMIitems in inventory filtersDeployment requirements
Before or alongside merging:
Enemy_Kami→Any_Kami, addBYPASS_BONUS_RESETflagreviseItemsfor index 11412 to update ForComponent + flags on-chainSummary by CodeRabbit
Release Notes
New Features
Bug Fixes