Skip to content

feat: draft zustand state management system #280

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

Merged
merged 7 commits into from
Sep 27, 2024
Merged

feat: draft zustand state management system #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2d8cb57
feat: draft zustand state management system
ponderingdemocritus Sep 26, 2024
46b79fc
feat: move to sdk
ponderingdemocritus Sep 26, 2024
03a5689
feat: iteration on state
ponderingdemocritus Sep 26, 2024
89804b5
feat: tests
ponderingdemocritus Sep 26, 2024
2398139
fix: lockfile
ponderingdemocritus Sep 26, 2024
aa6d5df
feat: cleanup
ponderingdemocritus Sep 26, 2024
cad1567
fix: remove boked CI
ponderingdemocritus Sep 27, 2024
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
26 changes: 0 additions & 26 deletions .github/workflows/npx-test.yaml
View file
Open in desktop

This file was deleted.

12 changes: 9 additions & 3 deletions examples/example-vite-react-sdk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,19 @@
"preview": "vite preview"
},
"dependencies": {
"@dojoengine/core": "1.0.0-alpha.12",
"@dojoengine/core": "workspace:*",
"@dojoengine/create-burner": "workspace:*",
"@dojoengine/sdk": "workspace:*",
"@dojoengine/torii-wasm": "1.0.0-alpha.12",
"@dojoengine/torii-wasm": "workspace:*",
"@types/uuid": "^10.0.0",
"immer": "^10.1.1",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"starknet": "6.11.0",
"uuid": "^10.0.0",
"vite-plugin-top-level-await": "^1.4.4",
"vite-plugin-wasm": "^3.3.0"
"vite-plugin-wasm": "^3.3.0",
"zustand": "^4.5.5"
},
"devDependencies": {
"@eslint/js": "^9.11.1",
Expand Down
86 changes: 50 additions & 36 deletions examples/example-vite-react-sdk/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import { useEffect, useState } from "react";
import { useEffect } from "react";
import "./App.css";
import { ParsedEntity, SDK } from "@dojoengine/sdk";
import { SDK, createDojoStore } from "@dojoengine/sdk";
import { Schema } from "./bindings.ts";

import { v4 as uuidv4 } from "uuid";

export const useDojoStore = createDojoStore<Schema>();

function App({ db }: { db: SDK<Schema> }) {
const [entities, setEntities] = useState<ParsedEntity<Schema>[]>([]);
const state = useDojoStore((state) => state);
const entities = useDojoStore((state) => state.entities);

Comment on lines +11 to +12
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider optimizing state selection

While using useDojoStore for state management is a good approach, selecting the entire state on line 11 might lead to unnecessary re-renders. Consider selecting only the specific parts of the state that this component needs.

Example:

const { entities, someOtherNeededState } = useDojoStore(state => ({
  entities: state.entities,
  someOtherNeededState: state.someOtherNeededState
}));

This way, the component will only re-render when these specific parts of the state change.

useEffect(() => {
let unsubscribe: (() => void) | undefined;
Expand All @@ -28,15 +33,7 @@ function App({ db }: { db: SDK<Schema> }) {
response.data &&
response.data[0].entityId !== "0x0"
) {
console.log(response.data);
setEntities((prevEntities) => {
return prevEntities.map((entity) => {
const newEntity = response.data?.find(
(e) => e.entityId === entity.entityId
);
return newEntity ? newEntity : entity;
});
});
state.setEntities(response.data);
}
},
{ logging: true }
Expand All @@ -54,8 +51,6 @@ function App({ db }: { db: SDK<Schema> }) {
};
}, [db]);

console.log("entities:");

useEffect(() => {
const fetchEntities = async () => {
try {
Expand All @@ -76,23 +71,7 @@ function App({ db }: { db: SDK<Schema> }) {
return;
}
if (resp.data) {
console.log(resp.data);
setEntities((prevEntities) => {
const updatedEntities = [...prevEntities];
resp.data?.forEach((newEntity) => {
const index = updatedEntities.findIndex(
(entity) =>
entity.entityId ===
newEntity.entityId
);
if (index !== -1) {
updatedEntities[index] = newEntity;
} else {
updatedEntities.push(newEntity);
}
});
return updatedEntities;
});
state.setEntities(resp.data);
}
}
);
Expand All @@ -104,20 +83,55 @@ function App({ db }: { db: SDK<Schema> }) {
fetchEntities();
}, [db]);

const optimisticUpdate = async () => {
const entityId =
"0x571368d35c8fe136adf81eecf96a72859c43de7efd8fdd3d6f0d17e308df984";

Comment on lines +87 to +88
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid hardcoding the entityId

Hardcoding the entityId reduces flexibility and may cause issues if the ID changes. Consider passing the entityId as a parameter or obtaining it dynamically based on the application's context.

const transactionId = uuidv4();

state.applyOptimisticUpdate(transactionId, (draft) => {
draft.entities[entityId].models.dojo_starter.Moves!.remaining = 10;
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure entity exists before applying optimistic update

When applying the optimistic update, ensure that draft.entities[entityId] exists to prevent potential runtime errors if the entity is undefined.

Suggested fix:

 state.applyOptimisticUpdate(transactionId, (draft) => {
+    if (draft.entities[entityId]) {
         draft.entities[entityId].models.dojo_starter.Moves!.remaining = 10;
+    }
 });
📝 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
draft.entities[entityId].models.dojo_starter.Moves!.remaining = 10;
state.applyOptimisticUpdate(transactionId, (draft) => {
if (draft.entities[entityId]) {
draft.entities[entityId].models.dojo_starter.Moves!.remaining = 10;
}
});


try {
// Wait for the entity to be updated before full resolving the transaction. Reverts if the condition is not met.
const updatedEntity = await state.waitForEntityChange(
entityId,
(entity) => {
// Define your specific condition here
return entity?.models.dojo_starter.Moves?.can_move === true;
}
Comment on lines +101 to +102
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Define the specific condition for waitForEntityChange

The condition in waitForEntityChange is currently a placeholder. Please replace it with the actual condition that determines when the entity has been updated.

Example:

 return entity?.models.dojo_starter.Moves?.can_move === true;
- // Define your specific condition here
+ // Condition: Checks if the 'can_move' flag is true

Committable suggestion was skipped due to low confidence.

);

console.log("Entity has been updated to active:", updatedEntity);

console.log("Updating entities...");
} catch (error) {
console.error("Error updating entities:", error);
state.revertOptimisticUpdate(transactionId);
} finally {
console.log("Updating entities...");
state.confirmTransaction(transactionId);
}
};
Comment on lines +112 to +115
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Review transaction confirmation placement

Calling state.confirmTransaction(transactionId); in the finally block means it executes regardless of success or failure. If the transaction fails and is reverted, confirming it may not be appropriate. Consider moving the confirmation inside the try block after the update succeeds.

Suggested change:

     } catch (error) {
         console.error("Error updating entities:", error);
         state.revertOptimisticUpdate(transactionId);
-    } finally {
+    } finally {
         console.log("Updating entities...");
-        state.confirmTransaction(transactionId);
     }
+    state.confirmTransaction(transactionId);
📝 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
} finally {
console.log("Updating entities...");
state.confirmTransaction(transactionId);
}
} catch (error) {
console.error("Error updating entities:", error);
state.revertOptimisticUpdate(transactionId);
} finally {
console.log("Updating entities...");
}
state.confirmTransaction(transactionId);


return (
<div>
<h1>Game State</h1>
{entities.map((entity) => (
<div key={entity.entityId}>
<h2>Entity {entity.entityId}</h2>
<button onClick={optimisticUpdate}>update</button>
{Object.entries(entities).map(([entityId, entity]) => (
<div key={entityId}>
<h2>Entity {entityId}</h2>
<h3>Position</h3>
<p>
Player:{" "}
{entity.models.dojo_starter.Position?.player ?? "N/A"}
<br />
X: {entity.models.dojo_starter.Position?.vec.x ?? "N/A"}
X:{" "}
{entity.models.dojo_starter.Position?.vec?.x ?? "N/A"}
<br />
Y: {entity.models.dojo_starter.Position?.vec.y ?? "N/A"}
Y:{" "}
{entity.models.dojo_starter.Position?.vec?.y ?? "N/A"}
</p>
<h3>Moves</h3>
<p>
Expand Down
8 changes: 6 additions & 2 deletions packages/sdk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,25 @@
"./package.json": "./package.json"
},
"devDependencies": {
"@rollup/plugin-commonjs": "^28.0.0",
"@vitest/coverage-v8": "^1.6.0",
"eslint": "^8.57.1",
"prettier": "^2.8.8",
"tsup": "^8.3.0",
"typescript": "^5.6.2",
"vite": "^3.2.11",
"vitest": "^1.6.0"
"vitest": "^1.6.0",
"benchmark": "^2.1.4",
"lodash": "^4.17.21"
},
"peerDependencies": {
"starknet": "6.11.0"
},
"dependencies": {
"@dojoengine/create-burner": "workspace:*",
"@dojoengine/torii-client": "workspace:*",
"axios": "^0.27.2",
"lodash": "^4.17.21",
"immer": "^10.1.1",
"vite-plugin-wasm": "^3.3.0",
"zustand": "^4.5.5"
},
Expand Down
12 changes: 7 additions & 5 deletions packages/sdk/src/__example__/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,20 @@ export interface ItemModel {
durability: number;
}

export interface GalaxyModel {
fieldOrder: string[];
id: string;
name: string;
}

export interface MockSchemaType extends SchemaType {
world: {
player: PlayerModel;
game: GameModel;
item: ItemModel;
};
universe: {
galaxy: {
fieldOrder: string[];
id: string;
name: string;
};
galaxy: GalaxyModel;
};
}

Expand Down
Loading
Loading