Skip to content

Commit

Permalink
refactor: Optomise how page data is updated
Browse files Browse the repository at this point in the history
Each inventory page is now a seperate array in the store that can be updated on its own. As opposed to a large ggCharacters object having to be updated regardless of which page changed.

Then instead of always creating a costly mutative 'create' object the checks for new data are done first. If the data changes then then mutative is used.
  • Loading branch information
NigelBreslaw authored and gitbutler-client committed Apr 23, 2024
1 parent f3d18d9 commit 19d4f75
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 52 deletions.
4 changes: 0 additions & 4 deletions native/app/bungie/Types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type {
GuardianRaceType,
ItemType,
DamageType,
UISections,
} from "@/app/bungie/Common.ts";
import { array, boolean, isoTimestamp, merge, number, object, optional, record, string, unknown } from "valibot";
import type { Output } from "valibot";
Expand Down Expand Up @@ -435,7 +434,4 @@ export type GGCharacterUiData = {
secondarySpecial: string;
lastActiveCharacter: boolean;
ggCharacterType: GGCharacterType;
armorPageData: UISections[];
generalPageData: UISections[];
weaponsPageData: UISections[];
};
29 changes: 14 additions & 15 deletions native/app/screens/InventoryPage.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { GGCharacterUiData } from "@/app/bungie/Types.ts";
import { InventoryPageEnums, type UISections } from "@/app/bungie/Common";
import { UiCellRenderItem } from "@/app/inventory/UiRowRenderItem.tsx";
import { useGGStore } from "@/app/store/GGStore.ts";
Expand All @@ -23,17 +22,6 @@ function calcCurrentListIndex(posX: number, PAGE_WIDTH: number) {
useGGStore.getState().setCurrentListIndex(index);
}

function getData(ggCharacter: GGCharacterUiData, inventoryPage: InventoryPageEnums): UISections[] | undefined {
switch (inventoryPage) {
case InventoryPageEnums.Armor:
return ggCharacter.armorPageData;
case InventoryPageEnums.General:
return ggCharacter.generalPageData;
case InventoryPageEnums.Weapons:
return ggCharacter.weaponsPageData;
}
}

type InventoryPageProps = {
inventoryPages: InventoryPageEnums;
};
Expand Down Expand Up @@ -101,7 +89,18 @@ export default function InventoryPage(props: InventoryPageProps) {
const debouncedMove = debounce(listMoved, 40);
const debounceListIndex = debounce(calcCurrentListIndex, 40);

const mainData = useGGStore((state) => state.ggCharacters);
function getData(inventoryPage: InventoryPageEnums): UISections[][] | undefined {
switch (inventoryPage) {
case InventoryPageEnums.Armor:
return useGGStore((state) => state.ggArmor);
case InventoryPageEnums.General:
return useGGStore((state) => state.ggGeneral);
case InventoryPageEnums.Weapons:
return useGGStore((state) => state.ggWeapons);
}
}

const mainData = getData(props.inventoryPages) ?? [];

return (
<View style={rootStyles.root}>
Expand All @@ -112,15 +111,15 @@ export default function InventoryPage(props: InventoryPageProps) {
onScroll={(e) => debounceListIndex(e.nativeEvent.contentOffset.x, HOME_WIDTH)}
ref={pagedScrollRef}
>
{mainData.map((character, index) => {
{mainData.map((_c, index) => {
return (
// biome-ignore lint/suspicious/noArrayIndexKey: <Index is unique for each page in this case>
<View key={index} style={styles.page}>
<FlashList
ref={(ref) => {
listRefs.current[index] = ref;
}}
data={getData(character, props.inventoryPages)}
data={mainData[index]}
renderItem={UiCellRenderItem}
keyExtractor={keyExtractor}
estimatedItemSize={pageEstimatedFlashListItemSize[index]}
Expand Down
77 changes: 50 additions & 27 deletions native/app/store/AccountInventoryLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,39 +39,62 @@ export function updateAllPages(get: AccountSliceGetter, set: AccountSliceSetter)
createUIData(get);
const p1 = performance.now();

const weaponsPageData = buildUIData(get, weaponsPageBuckets);
const ggCharacters = get().ggCharacters;
// For each page use a deepEqual compare to see if the data has changed.
// If it has changed then update just that page.
const ggWeapons = get().ggWeapons;
const newWeaponsPageData = buildUIData(get, weaponsPageBuckets);
const updatedWeapons = getUpdatedItems(ggWeapons, newWeaponsPageData);
if (updatedWeapons) {
set({ ggWeapons: updatedWeapons });
}

const ggArmor = get().ggArmor;
const newArmorPageData = buildUIData(get, armorPageBuckets);
const updatedArmor = getUpdatedItems(ggArmor, newArmorPageData);
if (updatedArmor) {
set({ ggArmor: updatedArmor });
}

const armorPageData = buildUIData(get, armorPageBuckets);
const generalPageData = buildUIData(get, generalPageBuckets);
const ggGeneral = get().ggGeneral;
const newGeneralPageData = buildUIData(get, generalPageBuckets);
const updatedGeneral = getUpdatedItems(ggGeneral, newGeneralPageData);
if (updatedGeneral) {
set({ ggGeneral: updatedGeneral });
}
const p2 = performance.now();
console.log("buildUIData took:", `${(p2 - p1).toFixed(4)} ms`);
console.log("updateAllPages", `${(p2 - p1).toFixed(4)} ms`);
}

const updatedGGCharacters = create(ggCharacters, (draft) => {
let index = 0;
for (const ggCharacter of draft) {
const newWeaponsPageData = weaponsPageData[index];
function getUpdatedItems(previousPages: UISections[][], newPageData: UISections[][]): UISections[][] | null {
const newPages: UISections[][] = [];
let foundNewItems = false;
let index = 0;
for (const page of newPageData) {
if (!deepEqual(previousPages[index], page)) {
newPages.push(page);
foundNewItems = true;
} else {
const emptySection: UISections[] = [];
newPages.push(emptySection);
}
index++;
}

if (newWeaponsPageData && !deepEqual(ggCharacter.weaponsPageData, newWeaponsPageData)) {
ggCharacter.weaponsPageData = newWeaponsPageData;
}
const newArmorPageData = armorPageData[index];
if (newArmorPageData && !deepEqual(ggCharacter.armorPageData, newArmorPageData)) {
ggCharacter.armorPageData = newArmorPageData;
if (foundNewItems) {
const updatedPages = create(previousPages, (draft) => {
let indexPages = 0;
for (const page of newPages) {
if (page.length > 0) {
draft[indexPages] = page;
}
indexPages++;
}
});

const newGeneralPageData = generalPageData[index];
if (newGeneralPageData && !deepEqual(ggCharacter.generalPageData, newGeneralPageData)) {
ggCharacter.generalPageData = newGeneralPageData;
}
index++;
}
});
const p3 = performance.now();
console.log("compare ggCharacters took:", `${(p3 - p2).toFixed(4)} ms`);
set({ ggCharacters: updatedGGCharacters });
const p4 = performance.now();
console.log("rebuild UI took:", `${(p4 - p3).toFixed(4)} ms`);
return updatedPages;
}

return null;
}

function createUIData(get: AccountSliceGetter) {
Expand Down
6 changes: 0 additions & 6 deletions native/app/store/AccountLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ export function getCharactersAndVault(guardians: Record<string, Guardian>): GGCh
secondarySpecial,
lastActiveCharacter: false,
ggCharacterType: GGCharacterType.Vault,
armorPageData: [],
generalPageData: [],
weaponsPageData: [],
};
ggCharacters.push(vaultData);

Expand All @@ -70,9 +67,6 @@ function addCharacterDefinition(guardianData: GuardianData): GGCharacterUiData {
secondarySpecial: "",
lastActiveCharacter: false,
ggCharacterType: GGCharacterType.Guardian,
armorPageData: [],
generalPageData: [],
weaponsPageData: [],
};

return data;
Expand Down
7 changes: 7 additions & 0 deletions native/app/store/AccountSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
DestinyClass,
ItemType,
type DestinyItemIdentifier,
type UISections,
} from "@/app/bungie/Common";
import { findDestinyItem, getCharactersAndVault } from "@/app/store/AccountLogic.ts";
import {
Expand Down Expand Up @@ -58,6 +59,9 @@ export interface AccountSlice {
// The characters live in an object. This array does duplicate some of this data, but it's order
// dictates
ggCharacters: GGCharacterUiData[];
ggWeapons: UISections[][];
ggArmor: UISections[][];
ggGeneral: UISections[][];

selectedItem: DestinyItem | null;

Expand Down Expand Up @@ -91,6 +95,9 @@ export const createAccountSlice: StateCreator<IStore, [], [], AccountSlice> = (s
currentListIndex: 0,

ggCharacters: [],
ggWeapons: [],
ggArmor: [],
ggGeneral: [],

selectedItem: null,

Expand Down

0 comments on commit 19d4f75

Please sign in to comment.