ZO fix disc scanner in controller mode #2633
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[zzz-frontend] [Sun Jan 26 19:18:45 UTC 2025] - Deployed b99b55d to https://genshin-optimizer-prs.github.io/pr/2633/zzz-frontend (Takes 3-5 minutes after this completes to be available) [Sun Jan 26 19:27:54 UTC 2025] - Deleted deployment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/zzz/disc-scanner/src/lib/processImg.ts (2)
Line range hint
90-94: Update error message to match the conditions.The error message mentions "set effect" but the condition only checks for main stat and substats.
if (mainStatTextIndex === -1 || substatTextIndex === -1) { retProcessed.texts.push( - 'Could not detect main stat, substats or set effect.' + 'Could not detect main stat or substats.' ) return retProcessed }
Line range hint
109-114: Update second error message to match the conditions.The error message mentions "set effect" but the condition only checks for main stat and substats lengths.
if ( setLvlTexts.length === 0 || mainStatTexts.length === 0 || substatTexts.length === 0 ) { retProcessed.texts.push( - 'Could not detect main stat, substats or set effect.' + 'Could not detect level, main stat, or substats.' ) return retProcessed }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/zzz/disc-scanner/src/lib/processImg.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint
🔇 Additional comments (2)
libs/zzz/disc-scanner/src/lib/processImg.ts (2)
Line range hint
85-120: LGTM! The changes improve robustness of set effect detection.The modifications make the disc scanner more resilient by:
- Making set effect detection optional
- Adding a fallback mechanism for missing set effect text
- Continuing processing even when set effect text is not found
This aligns well with fixing the scanner in controller mode where text detection might be partial or cut off.
85-89: Add null check for whiteTexts array.The fallback logic assumes
whiteTextsis non-empty. This could lead to issues if OCR fails to detect any text.Add a defensive check:
let setEffectTextIndex = whiteTexts.findIndex((t) => t.toLowerCase().includes('set effect') ) - if (setEffectTextIndex === -1) setEffectTextIndex = whiteTexts.length - 1 + if (setEffectTextIndex === -1 && whiteTexts.length > 0) setEffectTextIndex = whiteTexts.length - 1 + else if (setEffectTextIndex === -1) setEffectTextIndex = 0
Describe your changes
Issue or discord link
Testing/validation
Checklist before requesting a review (leave this PR as draft if any part of this list is not done.)
yarn run mini-cilocally to validate format and lint.Summary by CodeRabbit