Skip to content

Improve state management [SYNTH-263] #1433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
0xda157 wants to merge 8 commits into
base: dev
Choose a base branch
from
+83 −101
Open

Improve state management [SYNTH-263] #1433

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9263add
ConfigureSchemeInterface: take setSelectedScheme
0xda157 Jul 18, 2026
1563ded
InputSchemeSelection: remove unneeded update reducer
0xda157 Jul 18, 2026
89671ab
BehaviorCard: remove duplicated update call
0xda157 Jul 22, 2026
5134e57
SettingsModal: GeneralTab: use components to handle state
0xda157 Jul 22, 2026
bea3dff
fmt
0xda157 Jul 22, 2026
e9202b2
Revert "InputSchemeSelection: remove unneeded update reducer"
0xda157 Jul 24, 2026
68c28c7
Revert "ConfigureSchemeInterface: take setSelectedScheme"
0xda157 Jul 24, 2026
abf3016
Merge branch 'dev' into dionkud/do-not-mutate-react
0xda157 Jul 24, 2026
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
2 changes: 1 addition & 1 deletion fission/src/ui/components/Checkbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { SoundPlayer } from "@/systems/sound/SoundPlayer"
import Label from "./Label"
import { LabelWithTooltip } from "./StyledComponents"

type CheckboxProps = {
export type CheckboxProps = {
/**
* The label text that will be on the right of the checkbox.
*/
Expand Down
18 changes: 9 additions & 9 deletions fission/src/ui/components/StatefulSlider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import { Slider, Stack, Tooltip, Typography } from "@mui/material"
import Label from "./Label"
import { useEffect, useState } from "react"

const StatefulSlider: React.FC<
Omit<Parameters<typeof Slider>[0], "value" | "onChange"> & {
label: string
defaultValue: number
onChange: (val: number) => void
tooltip?: string
showValue?: boolean
}
> = props => {
export type StatefulSliderProps = Omit<Parameters<typeof Slider>[0], "value" | "onChange"> & {
label: string
defaultValue: number
onChange: (val: number) => void
tooltip?: string
showValue?: boolean
}

const StatefulSlider: React.FC<StatefulSliderProps> = props => {
const [value, setValue] = useState(props.defaultValue)

useEffect(() => {
Expand Down
151 changes: 71 additions & 80 deletions fission/src/ui/modals/configuring/SettingsModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { Box, Stack, Tab, Tabs, TextField } from "@mui/material"
import type React from "react"
import { useCallback, useEffect, useReducer, useState } from "react"
import { useCallback, useEffect, useState } from "react"
import { globalAddToast, globalOpenModal } from "@/components/GlobalUIControls.ts"
import PreferencesSystem from "@/systems/preferences/PreferencesSystem"
import type { UserPreference, UserPreferences } from "@/systems/preferences/PreferenceTypes"
import type { UserPreferences } from "@/systems/preferences/PreferenceTypes"
import { SoundPlayer } from "@/systems/sound/SoundPlayer"
import World from "@/systems/World"
import Checkbox from "@/ui/components/Checkbox"
import Checkbox, { type CheckboxProps } from "@/ui/components/Checkbox"
import Label from "@/ui/components/Label"
import type { ModalImplProps } from "@/ui/components/Modal"
import StatefulSlider from "@/ui/components/StatefulSlider"
import StatefulSlider, { type StatefulSliderProps } from "@/ui/components/StatefulSlider"
import { Button, Spacer, SynthesisIcons } from "@/ui/components/StyledComponents"
import { useThemeContext } from "@/ui/helpers/ThemeProviderHelpers"
import { useUIContext } from "@/ui/helpers/UIProviderHelpers"
Expand Down Expand Up @@ -52,10 +52,6 @@ type ThemeEditorTabActions = {
reset: () => void
}

type GeneralTabProps = {
writePreference: <K extends UserPreference>(pref: K, value: UserPreferences[K]) => void
}

type GraphicsTabProps = {
onActionsChange?: (actions: GraphicsTabActions) => void
}
Expand All @@ -71,7 +67,7 @@ interface TabConfigBase {

interface GeneralTabConfig extends TabConfigBase {
key: "general"
component: React.ComponentType<GeneralTabProps>
component: React.ComponentType
}

interface GraphicsTabConfig extends TabConfigBase {
Expand Down Expand Up @@ -113,112 +109,113 @@ const ColorEditor: React.FC<{
)
}

const GeneralTab: React.FC<GeneralTabProps> = ({ writePreference }) => (
type TypedPreferenceKey<T> = {
[K in keyof UserPreferences]: UserPreferences[K] extends T ? K : never
}[keyof UserPreferences]

const GeneralTabSlider = ({
preference,
label,
...props
}: Omit<StatefulSliderProps, "defaultValue" | "onChange"> & { preference: TypedPreferenceKey<number> }) => {
const [pref, setPref] = useState(PreferencesSystem.getUserPreference(preference))
useEffect(() => {
PreferencesSystem.setUserPreference(preference, pref)
}, [pref])

return <StatefulSlider {...props} label={label} defaultValue={pref} onChange={setPref} />
}

const GeneralTabCheckbox = ({
preference,
label,
...props
}: Omit<CheckboxProps, "checked" | "onClick"> & { preference: TypedPreferenceKey<boolean> }) => {
const [pref, setPref] = useState(PreferencesSystem.getUserPreference(preference))
useEffect(() => {
PreferencesSystem.setUserPreference(preference, pref)
}, [pref])
Comment on lines +134 to +137

@Dhruv-0-Arora Dhruv-0-Arora Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This will fire a preferenceChanged event for every preference on mount and twice in dev strictmode

instead, I would suggest

<Checkbox {...props} label={label} checked={pref} onClick={
setPref(val)
PreferencesSystem.setUserPreferences(preference, val)
} />

@0xda157 0xda157 Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This will fire a preferenceChanged event for every preference on mount and twice in dev strictmode

why is this an issue?

instead, I would suggest

<Checkbox {...props} label={label} checked={pref} onClick={
setPref(val)
PreferencesSystem.setUserPreferences(preference, val)
} />

this is a very strong react anti-pattern. If setPref gets called somewhere else, after this change, it will lead to the UI being in an invalid state.

@PepperLola PepperLola Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with @0xda157; there's not much of a cost to the event being fired repeatedly since we have relatively few listeners and preferences, and it avoids ending up with an invalid UI state

@Dhruv-0-Arora Dhruv-0-Arora Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So we are reading from the preferences and setting the preferences...

There are 13 preference events being triggered and it seems that 8 do not have listeners:
ReportAnalytics, SubsystemGravity, RenderSceneTag, RenderScoreboard, ShowViewCube

ReportAnalytics & SubsystemGravity might be problematic:
-> ReportAnalytics calls sendMetaData() [eventually] -> sending to google analytics
-> calls GetMotorSettings() and rewrites 2 torque limits on a constraint

@PepperLola @0xda157


return <Checkbox {...props} label={label} checked={pref} onClick={setPref} />
}

const GeneralTab: React.FC = () => (
<Stack direction="column" gap={2}>
<Spacer height={5} />
<Label size="sm">Camera Settings</Label>
<StatefulSlider
<GeneralTabSlider
preference="SceneRotationSensitivity"
label="Scene Rotation Sensitivity"
tooltip="Controls how fast the scene rotates when dragging with the mouse."
showValue={false}
min={0.1}
max={2.0}
defaultValue={PreferencesSystem.getUserPreference("SceneRotationSensitivity")}
label={"Scene Rotation Sensitivity"}
onChange={value => writePreference("SceneRotationSensitivity", value)}
step={0.1}
tooltip="Controls how fast the scene rotates when dragging with the mouse."
showValue={false}
/>
<Spacer height={5} />
<StatefulSlider
<GeneralTabSlider
preference="ScenePanSensitivity"
label="Scene Pan Sensitivity"
tooltip="Controls how fast the scene pans when dragging with the right mouse button."
showValue={false}
min={0.1}
max={3.0}
defaultValue={PreferencesSystem.getUserPreference("ScenePanSensitivity")}
label={"Scene Pan Sensitivity"}
onChange={value => writePreference("ScenePanSensitivity", value)}
step={0.1}
tooltip="Controls how fast the scene pans when dragging with the right mouse button."
showValue={false}
/>
<Spacer height={5} />
<StatefulSlider
<GeneralTabSlider
preference="ViewCubeRotationSensitivity"
label="ViewCube Rotation Sensitivity"
tooltip="Controls how fast the view changes when dragging on the view cube."
showValue={false}
min={0.06}
max={6.0}
defaultValue={PreferencesSystem.getUserPreference("ViewCubeRotationSensitivity")}
label={"ViewCube Rotation Sensitivity"}
onChange={value => writePreference("ViewCubeRotationSensitivity", value)}
step={0.06}
tooltip="Controls how fast the view changes when dragging on the view cube."
showValue={false}
/>
<Checkbox
<GeneralTabCheckbox
preference="ShowViewCube"
label="Show View Cube"
checked={PreferencesSystem.getUserPreference("ShowViewCube")}
onClick={checked => writePreference("ShowViewCube", checked)}
tooltip="Show the view cube in the top-right corner for quick camera orientation changes."
/>
<Spacer height={10} />
<Label size="md" sx={{ fontWeight: 600 }}>
Preferences
</Label>
<Stack direction="column">
<Checkbox
<GeneralTabCheckbox
preference="ReportAnalytics"
label="Report Analytics"
checked={PreferencesSystem.getUserPreference("ReportAnalytics")}
onClick={checked => writePreference("ReportAnalytics", checked)}
tooltip="Record user data such as what robots are spawned and how they are configured. No personal data will be collected."
/>
<Checkbox
<GeneralTabCheckbox
preference="SubsystemGravity"
label="Realistic Subsystem Gravity"
checked={PreferencesSystem.getUserPreference("SubsystemGravity")}
onClick={checked => writePreference("SubsystemGravity", checked)}
tooltip="Allows you to set a target torque or force for subsystems and joints. If not properly configured, joints may not be able to resist gravity or may not behave as intended."
/>
<Checkbox
<GeneralTabCheckbox
preference="RenderScoringZones"
label="Show Score Zones"
checked={PreferencesSystem.getUserPreference("RenderScoringZones")}
onClick={checked => writePreference("RenderScoringZones", checked)}
tooltip="If disabled, scoring zones will not be visible but will continue to function the same."
/>
<Checkbox
<GeneralTabCheckbox
preference="RenderProtectedZones"
label="Show Protected Zones"
checked={PreferencesSystem.getUserPreference("RenderProtectedZones")}
onClick={checked => writePreference("RenderProtectedZones", checked)}
tooltip="If disabled, protected zones will not be visible but will continue to function the same."
/>
<Checkbox
label="Show Scene Tags"
checked={PreferencesSystem.getUserPreference("RenderSceneTags")}
onClick={checked => writePreference("RenderSceneTags", checked)}
tooltip="Name tags above robot."
/>
<Checkbox
label="Show Scoreboard"
checked={PreferencesSystem.getUserPreference("RenderScoreboard")}
onClick={checked => {
writePreference("RenderScoreboard", checked)
if (checked) {
// TODO: figure out scoreboard - I think it should be its own component and not a panel
// openPanel("scoreboard");
}
}}
/>
<Checkbox
<GeneralTabCheckbox preference="RenderSceneTags" label="Show Scene Tags" tooltip="Name tags above robot." />
<GeneralTabCheckbox preference="RenderScoreboard" label="Show Scoreboard" />
<GeneralTabCheckbox
preference="ShowCenterOfMassIndicators"
label="Show Centers of Mass"
checked={PreferencesSystem.getUserPreference("ShowCenterOfMassIndicators")}
onClick={checked => writePreference("ShowCenterOfMassIndicators", checked)}
tooltip="Show a purple dot to indicate the center of mass of each robot in frame"
/>
<Checkbox
label="Mute All Sound"
checked={PreferencesSystem.getUserPreference("MuteAllSound")}
onClick={checked => writePreference("MuteAllSound", checked)}
/>
<StatefulSlider
<GeneralTabCheckbox preference="MuteAllSound" label="Mute All Sound" />
<GeneralTabSlider
preference="SFXVolume"
label="SFX Volume"
tooltip="Volume of sound effects (%)."
min={0}
max={100}
defaultValue={PreferencesSystem.getUserPreference("SFXVolume")}
label={"SFX Volume"}
onChange={value => writePreference("SFXVolume", value)}
tooltip="Volume of sound effects (%)."
/>
</Stack>
</Stack>
Expand Down Expand Up @@ -539,7 +536,6 @@ interface SettingsModalCustomProps {

const SettingsModal: React.FC<ModalImplProps<void, SettingsModalCustomProps | undefined>> = ({ modal }) => {
const { configureScreen } = useUIContext()
const [_, refresh] = useReducer(x => !x, false)
const [activeTab, setActiveTab] = useState<string>(modal?.props.custom?.initialTab || "general")

const [graphicsActions, setGraphicsActions] = useState<GraphicsTabActions | null>(null)
Expand All @@ -552,11 +548,6 @@ const SettingsModal: React.FC<ModalImplProps<void, SettingsModalCustomProps | un
{ key: "theme", label: "Theme Editor", component: ThemeEditorTab },
]

const writePreference = <K extends UserPreference>(pref: K, value: UserPreferences[K]) => {
PreferencesSystem.setUserPreference(pref, value)
refresh()
}

const save = useCallback(() => {
if (graphicsActions) {
graphicsActions.save()
Expand Down Expand Up @@ -595,7 +586,7 @@ const SettingsModal: React.FC<ModalImplProps<void, SettingsModalCustomProps | un
switch (currentTab.key) {
case "general": {
const GeneralComponent = currentTab.component
return <GeneralComponent writePreference={writePreference} />
return <GeneralComponent />
}
case "graphics": {
const GraphicsComponent = currentTab.component
Expand Down
54 changes: 37 additions & 17 deletions fission/src/ui/panels/configuring/assembly-config/ConfigurePanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,47 @@ interface ConfigInterfaceProps<T, P> {
assembly: MirabufSceneObject
}

const ConfigureInterfaceSetControls = ({
brainIndex,
panel,
assembly,
}: {
brainIndex: number
panel: UIScreen<void, ConfigurePanelCustomProps>
assembly: MirabufSceneObject
}) => {
const { openPanel, closePanel } = useUIContext()

const [scheme, setScheme] = useState(InputSystem.brainIndexSchemeMap.get(brainIndex))
useEffect(() => {
if (!scheme) return
InputSystem.brainIndexSchemeMap.set(brainIndex, scheme)
}, [scheme])

return (
<>
<Button
onClick={() => {
setSpotlightAssembly(assembly)
openPanel(ChooseInputSchemePanel, undefined, panel)
closePanel(panel.id, CloseType.Overwrite)
}}
>
Set Scheme
</Button>
{scheme && (
<ConfigureSchemeInterface selectedScheme={scheme} setSelectedScheme={setScheme} panelId={panel?.id} />
)}
</>
)
}

const ConfigInterface: React.FC<ConfigInterfaceProps<void, ConfigurePanelCustomProps>> = ({
panel,
configMode,
assembly,
}) => {
const { openPanel, closePanel } = useUIContext()
const { closePanel } = useUIContext()

useEffect(() => {
if (configMode !== ConfigMode.MOVE) return
Expand All @@ -169,22 +204,7 @@ const ConfigInterface: React.FC<ConfigInterfaceProps<void, ConfigurePanelCustomP
return <ConfigureSubsystemsInterface selectedRobot={assembly} />
case ConfigMode.CONTROLS: {
const brainIndex = (assembly.brain as SynthesisBrain).brainIndex
const scheme = InputSystem.brainIndexSchemeMap.get(brainIndex)

return (
<>
<Button
onClick={() => {
setSpotlightAssembly(assembly)
openPanel(ChooseInputSchemePanel, undefined, panel)
closePanel(panel.id, CloseType.Overwrite)
}}
>
Set Scheme
</Button>
{scheme && <ConfigureSchemeInterface selectedScheme={scheme} panelId={panel?.id} />}
</>
)
return <ConfigureInterfaceSetControls panel={panel} assembly={assembly} brainIndex={brainIndex} />
}
case ConfigMode.SEQUENTIAL:
return <SequentialBehaviorsInterface selectedRobot={assembly} />
Expand Down
Loading
Loading