Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion libs/gi/localization/assets/locales/en/artifact.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
"alert": "NOTE: Artifact Scanning currently only works for <strong>ENGLISH</strong> artifacts.",
"section1": "<0>Using screenshots can dramatically decrease the amount of time you manually input in stats on the Genshin Optimizer.</0><1>Where to snip the screenshot.</1><2>In game, Open your bag, and navigate to the artifacts tab. Select the artifact you want to scan with Genshin Optimizer. <2>Only artifact from this screen can be scanned.</2></2><3>Single artifact</3><4>To take a screenshot, in Windows, the shortcut is <2>Alt + Print Screen</2>. Once you selected the region, the image is automatically included in your clipboard.</4><5>Multiple artifacts</5><6>To take advantage of batch uploads, you can use a tool like <2>PicPick</2> to create a macro to easily to screenshot multiple artifacts at once.</6><7>What to include in the screenshot.</7><8>The full genshin window, or at least a region that covers the artifact card.</8>",
"section2": "<0>Adding Screenshot to Genshin Optimizer</0><1>At this point, you should have the artifact screenshot either saved to your harddrive, or in your clipboard.</1><2>You can click on the box next to \"Browse\" to browse the files in your harddrive for multiple screenshots.</2><3>For single screenshots from the snippets, just press <2>Ctrl + V</2> to paste from your clipboard.</3><4>You should be able to see a Preview of your artifact snippet, and after waiting a few seconds, the artifact set and the substats will be filled in in the <1>Artifact Editor</1>.</4><5>Finishing the Artifact</5><6>Unfortunately, computer vision is not 100%. There will always be cases where something is not scanned properly. You should always double check the scanned artifact values! Once the artifact has been filled, Click on <2>Add Artifact</2> to finish editing the artifact.</6>"
}
},
"unactivated": "Unactivated"
},
"slot": "Slot",
"setEffectNum": "{{setNum}}-Set",
Expand Down
110 changes: 98 additions & 12 deletions libs/gi/ui/src/components/artifact/editor/SubstatInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,35 +23,59 @@ import {
Box,
Button,
ButtonGroup,
FormControlLabel,
Grid,
ListItemIcon,
ListItemText,
MenuItem,
Slider,
Typography,
} from '@mui/material'
import Checkbox from '@mui/material/Checkbox'
import { useEffect, useMemo, useState } from 'react'
import { Trans, useTranslation } from 'react-i18next'
import { PercentBadge } from '../../PercentBadge'
import { ArtifactStatWithUnit } from '../ArtifactStatKeyDisplay'
import type { RollColorKey } from '../util'

function getCorrectSubstats(
artifact: ICachedArtifact | undefined,
isUnactivatedSubstat: boolean,
substatIndex: number
) {
if (
artifact?.unactivatedSubstats &&
substatIndex === 3 &&
isUnactivatedSubstat
) {
return artifact.unactivatedSubstats[0]
} else {
return artifact?.substats[substatIndex]
}
}

export function SubstatInput({
index,
artifact,
setSubstat,
}: {
index: number
artifact: ICachedArtifact | undefined
setSubstat: (index: number, substat: ISubstat) => void
setSubstat: (
index: number,
substat: ISubstat,
isUnactivatedSubstat: boolean
) => void
}) {
const { t } = useTranslation('artifact')
const { mainStatKey = '', rarity = 5 } = artifact ?? {}
const [isUnactivatedSubstat, setIsUnactivatedSubstat] = useState(false)
const { mainStatKey = '', rarity = 5, level = 0 } = artifact ?? {}
const {
key = '',
value = 0,
rolls = [],
efficiency = 0,
} = artifact?.substats[index] ?? {}
} = getCorrectSubstats(artifact, isUnactivatedSubstat, index) ?? {}

const accurateValue = rolls.reduce((a, b) => a + b, 0)
const unit = getUnitStr(key),
Expand Down Expand Up @@ -89,6 +113,24 @@ export function SubstatInput({
[key, rarity]
)

const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => {
setIsUnactivatedSubstat(event.target.checked)
setSubstat(index, { key: key, value: 0 }, event.target.checked)
}

useEffect(() => {
if (level >= 4) {
setIsUnactivatedSubstat(false)
}

if (
artifact?.unactivatedSubstats?.length &&
artifact.unactivatedSubstats[0].key
) {
setIsUnactivatedSubstat(true)
}
}, [artifact?.unactivatedSubstats, level])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistent level check - useEffect vs checkbox disabled condition.

Line 122 resets isUnactivatedSubstat when level >= 4, but line 299 disables the checkbox when level > 0. These should match - if the checkbox is disabled at level 1+, the state should also reset at level 1+. squints at the discrepancy through tired eyes

Apply this diff to align the conditions:

   useEffect(() => {
-    if (level >= 4) {
+    if (level > 0) {
       setIsUnactivatedSubstat(false)
     }
📝 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.

Suggested change
useEffect(() => {
if (level >= 4) {
setIsUnactivatedSubstat(false)
}
if (
artifact?.unactivatedSubstats?.length &&
artifact.unactivatedSubstats[0].key
) {
setIsUnactivatedSubstat(true)
}
}, [artifact?.unactivatedSubstats, level])
useEffect(() => {
if (level > 0) {
setIsUnactivatedSubstat(false)
}
if (
artifact?.unactivatedSubstats?.length &&
artifact.unactivatedSubstats[0].key
) {
setIsUnactivatedSubstat(true)
}
}, [artifact?.unactivatedSubstats, level])
🤖 Prompt for AI Agents
In libs/gi/ui/src/components/artifact/editor/SubstatInput.tsx around lines 121
to 132, the useEffect resets isUnactivatedSubstat when level >= 4 but the
checkbox is disabled for level > 0; update the useEffect condition to match the
checkbox by resetting isUnactivatedSubstat when level > 0 (i.e., replace the
level >= 4 check with level > 0) and keep the subsequent
artifact.unactivatedSubstats check as-is so state reflects the actual checkbox
enabled/disabled logic.


return (
<CardThemed bgt="light">
<Box sx={{ display: 'flex', height: '2.5em' }}>
Expand All @@ -108,7 +150,9 @@ export function SubstatInput({
>
{key && (
<MenuItem
onClick={() => setSubstat(index, { key: '', value: 0 })}
onClick={() =>
setSubstat(index, { key: '', value: 0 }, isUnactivatedSubstat)
}
>
{t('editor.substat.noSubstat')}
</MenuItem>
Expand All @@ -120,7 +164,13 @@ export function SubstatInput({
key={k}
selected={key === k}
disabled={key === k}
onClick={() => setSubstat(index, { key: k, value: 0 })}
onClick={() =>
setSubstat(
index,
{ key: k, value: 0 },
isUnactivatedSubstat
)
}
>
<ListItemIcon>
<StatIcon statKey={k} />
Expand All @@ -144,8 +194,14 @@ export function SubstatInput({
float={unit === '%'}
placeholder={t('editor.substat.selectSub')}
value={key ? value : 0}
onChange={(value) => setSubstat(index, { key, value: value ?? 0 })}
disabled={!key}
onChange={(value) =>
setSubstat(
index,
{ key, value: value ?? 0 },
isUnactivatedSubstat
)
}
disabled={!key || (isUnactivatedSubstat && index === 3)}
error={!!error}
inputProps={{
sx: { textAlign: 'right' },
Expand All @@ -163,9 +219,17 @@ export function SubstatInput({
<Button
key={i}
color={`roll${clamp(rollOffset + i, 1, 6)}` as any}
disabled={(value && !rollNum) || allowedRolls <= 0}
disabled={
(value && !rollNum) ||
allowedRolls <= 0 ||
(isUnactivatedSubstat && rollNum > 0 && index === 3)
}
Comment on lines +207 to +211
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unactivated 4th substat still allows “next roll” buttons.

These buttons should be disabled when the substat is marked Unactivated; otherwise users can apply rolls to an unactivated slot.

-                disabled={
-                  (value && !rollNum) ||
-                  allowedRolls <= 0 ||
-                  (isUnactivatedSubstat && rollNum > 0 && index === 3)
-                }
+                disabled={
+                  (value && !rollNum) ||
+                  allowedRolls <= 0 ||
+                  (isUnactivatedSubstat && index === 3)
+                }
📝 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.

Suggested change
disabled={
(value && !rollNum) ||
allowedRolls <= 0 ||
(isUnactivatedSubstat && rollNum > 0 && index === 3)
}
disabled={
(value && !rollNum) ||
allowedRolls <= 0 ||
(isUnactivatedSubstat && index === 3)
}
🤖 Prompt for AI Agents
In libs/gi/ui/src/components/artifact/editor/SubstatInput.tsx around lines 222
to 226, the "next roll" buttons still become enabled for an unactivated 4th
substat because the current condition only blocks some cases
(isUnactivatedSubstat && rollNum > 0 && index === 3) instead of blocking all
interactions when the substat is unactivated; change the disabled expression to
include isUnactivatedSubstat as a standalone OR term so that if
isUnactivatedSubstat is true the buttons are always disabled (i.e., add "||
isUnactivatedSubstat" to the existing boolean chain).

onClick={() =>
setSubstat(index, { key, value: parseFloat(newValue) })
setSubstat(
index,
{ key, value: parseFloat(newValue) },
isUnactivatedSubstat
)
}
>
{newValue}
Expand All @@ -179,9 +243,13 @@ export function SubstatInput({
value={value}
marks={marks}
setValue={(v) =>
setSubstat(index, { key, value: (v as number) ?? 0 })
setSubstat(
index,
{ key, value: (v as number) ?? 0 },
isUnactivatedSubstat
)
}
disabled={!key}
disabled={!key || (isUnactivatedSubstat && index === 3)}
/>
</Box>
<Box sx={{ px: 1, pb: 1 }}>
Expand All @@ -202,7 +270,7 @@ export function SubstatInput({
: t('editor.substat.noRoll')}
</SqBadge>
</Grid>
<Grid item flexGrow={1}>
<Grid item>
{!!rolls.length &&
[...rolls].sort().map((val, i) => (
<Typography
Expand All @@ -219,6 +287,24 @@ export function SubstatInput({
</Typography>
))}
</Grid>
<Grid item flexGrow={1}>
{index === 3 ? (
<FormControlLabel
label={t('editor.unactivated')}
control={
<Checkbox
checked={isUnactivatedSubstat}
onChange={handleChange}
sx={{ padding: 0 }}
disabled={!artifact || level > 0}
></Checkbox>
}
sx={{ ml: 1 }}
/>
) : (
''
)}
</Grid>
<Grid item xs="auto" flexShrink={1}>
<Typography>
<Trans
Expand Down
58 changes: 55 additions & 3 deletions libs/gi/ui/src/components/artifact/editor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,19 @@ import { UploadExplainationModal } from './UploadExplainationModal'
const allSubstatFilter = new Set(allSubstatKeys)
type ResetMessage = { type: 'reset' }
type SubstatMessage = { type: 'substat'; index: number; substat: ISubstat }
type UnactivatedSubstatMessage = {
type: 'unactivatedSubstat'
index: number
substat: ISubstat
}
type OverwriteMessage = { type: 'overwrite'; artifact: IArtifact }
type UpdateMessage = { type: 'update'; artifact: Partial<IArtifact> }
type Message = ResetMessage | SubstatMessage | OverwriteMessage | UpdateMessage
type Message =
| ResetMessage
| SubstatMessage
| OverwriteMessage
| UpdateMessage
| UnactivatedSubstatMessage
function artifactReducer(
state: IArtifact | undefined,
action: Message
Expand All @@ -114,6 +124,14 @@ function artifactReducer(
const oldIndex = substat.key
? state!.substats.findIndex((current) => current.key === substat.key)
: -1

if (
state!.unactivatedSubstats?.length &&
state!.unactivatedSubstats[0].key
) {
state!.unactivatedSubstats = []
}

if (oldIndex === -1 || oldIndex === index)
state!.substats[index] = substat
// Already in used, swap the items instead
Expand All @@ -124,6 +142,39 @@ function artifactReducer(
]
return { ...state! }
}
case 'unactivatedSubstat': {
if (state?.unactivatedSubstats) {
const { substat } = action

let activeSubstat = null

if (state!.substats[3].key) {
activeSubstat = state!.substats[3]
state!.substats[3] = { key: '', value: 0 }
} else {
activeSubstat = substat
}

const oldIndex = substat.key
? state!.unactivatedSubstats.findIndex(
(current) => current.key === substat.key
)
: -1
if (oldIndex === -1 || oldIndex === 0)
state!.unactivatedSubstats[0] = activeSubstat
// Already in used, swap the items instead
else
[
state!.unactivatedSubstats[0],
state!.unactivatedSubstats[oldIndex],
] = [
state!.unactivatedSubstats[oldIndex],
state!.unactivatedSubstats[0],
]
}
console.log(state!.substats, 'subers')
return { ...state! }
}
case 'overwrite':
return action.artifact
case 'update':
Expand Down Expand Up @@ -304,8 +355,9 @@ export function ArtifactEditor({
[artifact, artStat, artifactDispatch, fixedSlotKey]
)
const setSubstat = useCallback(
(index: number, substat: ISubstat) => {
artifactDispatch({ type: 'substat', index, substat })
(index: number, substat: ISubstat, isUnactivatedSubstat: boolean) => {
const type = isUnactivatedSubstat ? 'unactivatedSubstat' : 'substat'
artifactDispatch({ type: type, index, substat })
},
Comment on lines 392 to 396
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Gate unactivated handling to the 4th substat only (prevents cross-row mutations).

Right now, any row edit while the toggle is on dispatches as “unactivated”, which explains the “editing 3rd changes 4th” reports. Only the 4th slot can be unactivated—gate by index.

-  const setSubstat = useCallback(
-    (index: number, substat: ISubstat, isUnactivatedSubstat: boolean) => {
-      const type = isUnactivatedSubstat ? 'unactivatedSubstat' : 'substat'
-      artifactDispatch({ type: type, index, substat })
-    },
+  const setSubstat = useCallback(
+    (index: number, substat: ISubstat, isUnactivatedSubstat: boolean) => {
+      const type =
+        isUnactivatedSubstat && index === 3 ? 'unactivatedSubstat' : 'substat'
+      artifactDispatch({ type, index, substat })
+    },
📝 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.

Suggested change
const setSubstat = useCallback(
(index: number, substat: ISubstat) => {
artifactDispatch({ type: 'substat', index, substat })
(index: number, substat: ISubstat, isUnactivatedSubstat: boolean) => {
const type = isUnactivatedSubstat ? 'unactivatedSubstat' : 'substat'
artifactDispatch({ type: type, index, substat })
},
const setSubstat = useCallback(
(index: number, substat: ISubstat, isUnactivatedSubstat: boolean) => {
const type =
isUnactivatedSubstat && index === 3 ? 'unactivatedSubstat' : 'substat'
artifactDispatch({ type, index, substat })
},
🤖 Prompt for AI Agents
In libs/gi/ui/src/components/artifact/editor/index.tsx around lines 391-395,
update setSubstat so that the "unactivatedSubstat" branch is only used for the
4th slot (index === 3); if isUnactivatedSubstat is true but index !== 3, treat
it as a normal 'substat' to avoid cross-row mutations. Change the
ternary/branching that picks the dispatch type to check index === 3 in addition
to isUnactivatedSubstat, and dispatch the appropriate action accordingly.

[artifactDispatch]
)
Expand Down
Loading