Skip to content

fix: readme + phaser recs example #287

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

Merged
merged 2 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all 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: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@
[submodule "worlds/onchain-dash"]
path = worlds/onchain-dash
url = https://github.com/MartianGreed/onchain-dash
[submodule "worlds/dojo-starter"]
path = worlds/dojo-starter
url = https://github.com/dojoengine/dojo-starter
Comment on lines +10 to +12
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate submodule entries

The newly added submodule entry for "worlds/dojo-starter" (lines 10-12) is a duplicate of the existing entry (lines 1-3). Additionally, there's a duplicate entry for "onchain-dash" (lines 7-9). Duplicate entries in the .gitmodules file can lead to confusion and potential issues with Git submodule management.

To resolve this, please remove the duplicate entries. Here's the suggested change:

[submodule "dojo-starter"]
	path = worlds/dojo-starter
	url = https://github.com/dojoengine/dojo-starter
[submodule "onchain-dash"]
	path = worlds/onchain-dash
	url = https://github.com/MartianGreed/onchain-dash
-[submodule "worlds/onchain-dash"]
-	path = worlds/onchain-dash
-	url = https://github.com/MartianGreed/onchain-dash
-[submodule "worlds/dojo-starter"]
-	path = worlds/dojo-starter
-	url = https://github.com/dojoengine/dojo-starter

This change will maintain the correct submodule configurations without redundancy.

📝 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
[submodule "worlds/dojo-starter"]
path = worlds/dojo-starter
url = https://github.com/dojoengine/dojo-starter
[submodule "dojo-starter"]
path = worlds/dojo-starter
url = https://github.com/dojoengine/dojo-starter
[submodule "onchain-dash"]
path = worlds/onchain-dash
url = https://github.com/MartianGreed/onchain-dash

8 changes: 4 additions & 4 deletions examples/example-nodejs-bot/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,19 @@ Features:
### Developing

```bash
bun install
pnpm install
```

### Terminal 1 - Serve the Bot

```bash
bun run serve
pnpm serve
```

### Terminal 2 - Build and Watch

```bash
bun run build --watch
pnpm build --watch
```

Now, try running it on your server. Remember to restart your bot after making changes.
Expand All @@ -46,7 +46,7 @@ You can access the GraphQL dashboard by navigating to [http://0.0.0.0:8080/graph
Add your GraphQL schema to `src/graphql/schema.graphql`, then run

```bash
bun run codegen
pnpm codegen
```

Now you can access the sdk in your app like:
Expand Down
5 changes: 1 addition & 4 deletions examples/example-vanillajs-phaser-recs/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,4 @@ export default setup(dojoConfig)
game.scene.add("MainScene", new SceneMain(dojo));
return game;
})
.catch((e) => {
console.error(e);
return;
});
.catch(console.error);
13 changes: 8 additions & 5 deletions examples/example-vanillajs-phaser-recs/src/scenes/scene-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export default class SceneMain extends Scene {
return chunk;
}

update() {
async update() {
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid making the update() method asynchronous in Phaser scenes

Making the update() method asynchronous in Phaser can lead to unexpected behavior and performance issues. The Phaser game loop expects update() to be a synchronous function called on each frame. Introducing await inside this method may cause frame drops or delays. It's recommended to keep update() synchronous and handle asynchronous operations differently.

Apply this diff to remove the async keyword and adjust the await calls:

-        async update() {
+        update() {

In each movement case, remove the await keyword:

if (null !== this.keyW && this.keyW.isDown) {
    this.followPoint.y -= this.cameraSpeed;
    this.cameras.main.centerOn(this.followPoint.x, this.followPoint.y);
-   await this.dojo.systemCalls.move(this.dojo.account, Direction.Up);
+   this.dojo.systemCalls.move(this.dojo.account, Direction.Up);
    return;
}

Consider handling the promises returned by this.dojo.systemCalls.move without awaiting them inside the update() method, or by using event-driven approaches to manage asynchronous behavior.

Committable suggestion was skipped due to low confidence.

if (this.followPoint === null || this.followPoint === undefined) {
throw new Error("failed to initialize followPoint");
}
Expand Down Expand Up @@ -135,25 +135,28 @@ export default class SceneMain extends Scene {
if (null !== this.keyW && this.keyW.isDown) {
this.followPoint.y -= this.cameraSpeed;
this.cameras.main.centerOn(this.followPoint.x, this.followPoint.y);
this.dojo.systemCalls.move(this.dojo.account, Direction.Up);
await this.dojo.systemCalls.move(this.dojo.account, Direction.Up);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for asynchronous movement commands

When calling await this.dojo.systemCalls.move(...), any errors thrown by the promise will not be caught, potentially causing unhandled exceptions. To ensure application stability, wrap the await calls in try-catch blocks to handle possible errors gracefully.

Apply this diff to add error handling:

// Up direction
if (null !== this.keyW && this.keyW.isDown) {
    this.followPoint.y -= this.cameraSpeed;
    this.cameras.main.centerOn(this.followPoint.x, this.followPoint.y);
-   await this.dojo.systemCalls.move(this.dojo.account, Direction.Up);
+   try {
+       await this.dojo.systemCalls.move(this.dojo.account, Direction.Up);
+   } catch (error) {
+       console.error('Failed to move Up:', error);
+   }
    return;
}

// Down direction
if (null !== this.keyS && this.keyS.isDown) {
    this.followPoint.y += this.cameraSpeed;
    this.cameras.main.centerOn(this.followPoint.x, this.followPoint.y);
-   await this.dojo.systemCalls.move(this.dojo.account, Direction.Down);
+   try {
+       await this.dojo.systemCalls.move(this.dojo.account, Direction.Down);
+   } catch (error) {
+       console.error('Failed to move Down:', error);
+   }
    return;
}

// Left direction
if (null !== this.keyA && this.keyA.isDown) {
    this.followPoint.x -= this.cameraSpeed;
    this.cameras.main.centerOn(this.followPoint.x, this.followPoint.y);
-   await this.dojo.systemCalls.move(this.dojo.account, Direction.Left);
+   try {
+       await this.dojo.systemCalls.move(this.dojo.account, Direction.Left);
+   } catch (error) {
+       console.error('Failed to move Left:', error);
+   }
    return;
}

// Right direction
if (null !== this.keyD && this.keyD.isDown) {
    this.followPoint.x += this.cameraSpeed;
    this.cameras.main.centerOn(this.followPoint.x, this.followPoint.y);
-   await this.dojo.systemCalls.move(
-       this.dojo.account,
-       Direction.Right
-   );
+   try {
+       await this.dojo.systemCalls.move(
+           this.dojo.account,
+           Direction.Right
+       );
+   } catch (error) {
+       console.error('Failed to move Right:', error);
+   }
    return;
}

This ensures that any errors during the movement calls are handled appropriately, preventing potential crashes.

Also applies to: 143-143, 149-149, 155-158

return;
}
if (null !== this.keyS && this.keyS.isDown) {
this.followPoint.y += this.cameraSpeed;
await this.dojo.systemCalls.move(this.dojo.account, Direction.Down);
this.cameras.main.centerOn(this.followPoint.x, this.followPoint.y);
this.dojo.systemCalls.move(this.dojo.account, Direction.Down);
return;
}
if (null !== this.keyA && this.keyA.isDown) {
this.followPoint.x -= this.cameraSpeed;
await this.dojo.systemCalls.move(this.dojo.account, Direction.Left);
this.cameras.main.centerOn(this.followPoint.x, this.followPoint.y);
this.dojo.systemCalls.move(this.dojo.account, Direction.Left);
return;
}
if (null !== this.keyD && this.keyD.isDown) {
this.followPoint.x += this.cameraSpeed;
await this.dojo.systemCalls.move(
this.dojo.account,
Direction.Right
);
this.cameras.main.centerOn(this.followPoint.x, this.followPoint.y);
this.dojo.systemCalls.move(this.dojo.account, Direction.Right);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ To run the examples, you'll need to set up three terminal windows:
3. Run Torii (indexer) with the world address and allowed origins:

```bash
torii --world 0xc82dfe2cb4f8a90dba1e88dfa24578aeb1c19152d51e3c7cf413be6d65d9e --allowed-origins "*"
torii --world <WORLD_ADDRESS> --allowed-origins "*"
```

Note: The world address may change. Ensure you're using the correct address from your migration output.
Expand Down
Loading