Skip to content
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.

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

Refactor state mgmt #1326

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
5 changes: 0 additions & 5 deletions frontend/src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ let App = {
packs: 3,
cubePoolSize: 90,

addBots: true,
shufflePlayers: true,
useTimer: true,
timerLength: "Moderate", // Fast Moderate or Slow

beep: true,
notify: false,
notificationGranted: false,
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/components/Checkbox.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import PropTypes from "prop-types";

import App from "../app";

const Checkbox = ({link, text, side, onChange, ...rest}) => (
const Checkbox = ({link, text, side, onChange, value, ...rest}) => (
HerveH44 marked this conversation as resolved.
Show resolved Hide resolved
<div>
{side === "right" ? text : ""}
<input
Expand All @@ -12,7 +12,7 @@ const Checkbox = ({link, text, side, onChange, ...rest}) => (
onChange={onChange || function (e) {
App.save(link, e.currentTarget.checked);
}}
checked={App.state[link]}/>
checked={value || App.state[link]}/>
{side === "left" ? text : ""}
</div>
);
Expand Down
7 changes: 4 additions & 3 deletions frontend/src/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {vanillaToast} from "vanilla-toast";
import DOMPurify from "dompurify";
import {range, times, constant, countBy, findIndex} from "lodash";
import {ZONE_JUNK, ZONE_MAIN, ZONE_PACK, ZONE_SIDEBOARD} from "./zones";
import store from "./state/store";

/**
* @desc this is the list of all the events that can be triggered by the App
Expand Down Expand Up @@ -47,9 +48,9 @@ const events = {
hash();
},
start() {
const {addBots, useTimer, timerLength, shufflePlayers} = App.state;
const options = {addBots, useTimer, timerLength, shufflePlayers};
App.send("start", options);
App.send("start", {
...store.getState().startControls
});
},
pickNumber(pick) {
App.save("pickNumber", pick);
Expand Down
12 changes: 6 additions & 6 deletions frontend/src/game/GameSettings.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from "react";

import App from "../app";
import Checkbox from "../components/Checkbox";
import './GameSettings.scss';
import "./GameSettings.scss";

const GameSettings = () => (
<div className='Game-Settings'>
Expand Down Expand Up @@ -41,10 +41,10 @@ const SortCards = () => (
Sort cards by:
<div className='connected-container' >
{["CMC", "Color", "Type", "Rarity"].map((sort, index) => {
const isActive = sort.toLowerCase() === App.state.sort
const isActive = sort.toLowerCase() === App.state.sort;

return (
<label key={index}
<label key={index}
className={isActive
? "active connected-component"
: "connected-component"
Expand All @@ -59,7 +59,7 @@ const SortCards = () => (
/>
<div>{sort}</div>
</label>
)
);
})}
</div>
</div>
Expand All @@ -77,7 +77,7 @@ const CardsImageQuality = () => (
Card image quality:
<div className='connected-container'>
{Object.keys(sizeDisplay).map((size, index) => {
const isActive = size.toLowerCase() === App.state.cardSize
const isActive = size.toLowerCase() === App.state.cardSize;

return (
<label key={index}
Expand All @@ -94,7 +94,7 @@ const CardsImageQuality = () => (
value={size.toLowerCase()} />
<div>{sizeDisplay[size]}</div>
</label>
)
);
})}
</div>
</div>
Expand Down
38 changes: 30 additions & 8 deletions frontend/src/game/StartPanel.jsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import React from "react";
import { useDispatch, useSelector } from "react-redux";

import App from "../app";
import Checkbox from "../components/Checkbox";
import Select from "../components/Select";
import { selectAddBots, selectShufflePlayers, selectUseTimer, toggleBots, toggleUseTimer, toggleShufflePlayers, selectTimerLength, setTimerLength, timers } from "../state/start-controls";
import { toTitleCase } from "../utils";

const StartPanel = () => {
Expand Down Expand Up @@ -40,21 +42,41 @@ const StartControls = () => {
};

const Options = () => {
const {useTimer} = App.state;
const timers = ["Fast", "Moderate", "Slow", "Leisurely"];
HerveH44 marked this conversation as resolved.
Show resolved Hide resolved
const dispatch = useDispatch();
const addBots = useSelector(selectAddBots);
const shufflePlayers = useSelector(selectShufflePlayers);
const useTimer = useSelector(selectUseTimer);
const timerLength = useSelector(selectTimerLength);

return (
<span>
{showAddBotsCheckbox()
? <Checkbox side="left" link="addBots" text="Fill empty seats with Bots"/>
? <Checkbox
side="left"
text="Fill empty seats with Bots"
value={addBots}
onChange={()=> dispatch(toggleBots())} />
: null
}
{showShufflePlayersCheckbox()
? <Checkbox side="left" link="shufflePlayers" text="Random seating"/>
? <Checkbox
side="left"
text="Random seating"
value={shufflePlayers}
onChange={()=> dispatch(toggleShufflePlayers())} />
: null
}
<div>
<Checkbox side="left" link="useTimer" text="Timer: "/>
<Select link="timerLength" opts={timers} disabled={!useTimer}/>
<Checkbox
side="left"
text="Timer: "
value={useTimer}
onChange={()=> dispatch(toggleUseTimer())} />
<Select
value={timerLength}
onChange={(e) => dispatch(setTimerLength(e.target.value))}
opts={timers}
disabled={!useTimer} />
</div>
</span>
);
Expand All @@ -63,11 +85,11 @@ const Options = () => {
const showAddBotsCheckbox = () => {
// No need for bots in decadent draft since there's no passing.
return !App.state.isDecadentDraft;
}
};

const showShufflePlayersCheckbox = () => {
// No need to shuffle players in decadent draft because there's no passing.
return !App.state.isDecadentDraft;
}
};

export default StartPanel;
10 changes: 9 additions & 1 deletion frontend/src/router.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
/* eslint-disable */
import React, { Suspense } from "react";
import { render } from "react-dom";
import { Provider } from "react-redux";
import store from "./state/store";
import { PersistGate } from 'redux-persist/integration/react';
import { persistStore } from 'redux-persist';

const Lobby = React.lazy(() => import("./lobby/Lobby"));
const Game = React.lazy(() => import("./game/Game"));
Expand All @@ -26,7 +30,11 @@ function route() {
App.once("gameInfos", App.updateGameInfos);
component = (
<Suspense fallback={<div>Loading...</div>}>
<Game id={ id } />
<Provider store={store}>
<PersistGate loading={null} persistor={persistStore(store)}>
<Game id={ id } />
</PersistGate>
</Provider>
</Suspense>
);
break;
Expand Down
31 changes: 31 additions & 0 deletions frontend/src/state/migration/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
const readAndDeleteFromLocalStorage = (key, defaultValue) => {
const val = localStorage[key];
Copy link
Member

Choose a reason for hiding this comment

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

Should probably use localStorage.setItem

getItem and removeItem?

I guess that protects against accidentally mutating wrong things on the raw object?

if (!val) {
return defaultValue;
}

try {
return JSON.parse(val);
} catch (e) {
return defaultValue;
} finally {
delete localStorage[key];
}
};

export default function migrateState(initialState, migratedState) {
const ret = {};
if (!migratedState) {
migratedState = {};
}
for (const property in initialState) {
ret[property] = readAndDeleteFromLocalStorage(
property,
migratedState[property] == undefined
? initialState[property]
: migratedState[property]
);
}

return ret;
}
41 changes: 41 additions & 0 deletions frontend/src/state/start-controls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { createSlice } from "@reduxjs/toolkit";

export const timers = ["Fast", "Moderate", "Slow", "Leisurely"];

export const initialState = {
addBots: true,
shufflePlayers: true,
useTimer: true,
timerLength: "Moderate", // Fast Moderate or Slow
};

const startControls = createSlice({
name: "startControls",
initialState,
reducers: {
toggleBots: state => {
state.addBots = !state.addBots;
},
toggleShufflePlayers: state => {
state.shufflePlayers = !state.shufflePlayers;
},
toggleUseTimer: state => {
state.useTimer = !state.useTimer;
},
setTimerLength: (state, action) => {
if (timers.includes(action.payload)) {
state.timerLength = action.payload;
}
}
}
}
);
Copy link
Member

Choose a reason for hiding this comment

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

These reducers are what we could easily write tests over. Haven't seen this createSlice thing before but we just need a way to reach in and check those reduce functions. These ones are simple, but would be good to get into habit of putting tests over ones which have multiple paths over them.

I would say we should have a testing pattern in place before merging this. Would set us up well. Happy to help


export const selectAddBots = state => state.startControls.addBots;
export const selectShufflePlayers = state => state.startControls.shufflePlayers;
export const selectUseTimer = state => state.startControls.useTimer;
export const selectTimerLength = state => state.startControls.timerLength;

export const { toggleBots, toggleShufflePlayers, toggleUseTimer, setTimerLength } = startControls.actions;

export default startControls;
44 changes: 44 additions & 0 deletions frontend/src/state/store.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { combineReducers, configureStore, getDefaultMiddleware } from "@reduxjs/toolkit";
import storage from "redux-persist/lib/storage";
import {
persistReducer, FLUSH,
REHYDRATE,
PAUSE,
PERSIST,
PURGE,
REGISTER
} from "redux-persist";
import { initialState as startControlInitState } from "./start-controls";
import startControls from "./start-controls";
import migrateState from "./migration";

const reducers = combineReducers({
startControls: startControls.reducer
});

const persistConfig = {
key: "root",
storage,
migrate: (state) => {
if (!state) {
state = {};
}
const newState = {
...state,
startControls: migrateState(startControlInitState, state.startControls)
};
return Promise.resolve(newState);
}
};

const persistedReducer = persistReducer(persistConfig, reducers);

export default configureStore({
reducer: persistedReducer,
devTools: true,
middleware: getDefaultMiddleware({
serializableCheck: {
ignoredActions: [FLUSH, REHYDRATE, PAUSE, PERSIST, PURGE, REGISTER]
}
}),
});
Loading