Skip to content

fix: paths #311

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 3 commits into from
Nov 1, 2024
Merged

fix: paths #311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02e4ee9
fix: dojo starter
ponderingdemocritus Nov 1, 2024
b2af4b4
fix: readme
ponderingdemocritus Nov 1, 2024
111d148
fix: new historial types
ponderingdemocritus Nov 1, 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
2 changes: 1 addition & 1 deletion examples/example-nodejs-bot/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import manifest from "../../worlds/dojo-starter/manifests/dev/deployment/manifest.json" assert { type: "json" };
import manifest from "../../worlds/dojo-starter/manifest_dev.json" assert { type: "json" };

Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Path change needs revision - manifest file not found

The verification reveals critical issues with the manifest path change:

  • The new manifest file manifest_dev.json does not exist at the specified location
  • Multiple example projects are attempting to use this non-existent path
  • The tsconfig.json suggests the manifest should be at dojo-starter/target/dev/manifest.json

This path change appears to be incorrect and needs to be revised to point to the actual manifest location.

🔗 Analysis chain

Verify manifest file existence and compatibility

The import path has been simplified to use manifest_dev.json. Let's verify the file exists and maintains the expected structure.

Let me gather more information about the manifest file location and structure.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify manifest file existence and check other examples for consistency

# Check if the new manifest file exists
echo "Checking manifest file existence..."
if [ -f "worlds/dojo-starter/manifest_dev.json" ]; then
    echo "✓ manifest_dev.json exists"
else
    echo "✗ manifest_dev.json not found"
fi

# Check if all example configs use the same manifest path
echo -e "\nChecking consistency across example configs..."
rg -l "manifest.*\.json.*assert" examples/

Length of output: 396

Script:

#!/bin/bash
# Check for any manifest files in the repository
echo "Searching for manifest files..."
fd manifest.*\.json

# Check the content of the PR diff to understand the path changes
echo -e "\nChecking PR changes related to manifest paths..."
git diff HEAD~1 | grep -A 2 -B 2 "manifest.*\.json"

# Look for any references to manifest files in the codebase
echo -e "\nSearching for manifest references in the code..."
rg "manifest.*\.json" -B 2 -A 2

Length of output: 5378

import { createDojoConfig } from "@dojoengine/core";

Expand Down
2 changes: 1 addition & 1 deletion examples/example-vanillajs-phaser-recs/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createDojoConfig } from "@dojoengine/core";

import manifest from "../../worlds/dojo-starter/manifests/dev/deployment/manifest.json";
import manifest from "../../worlds/dojo-starter/manifest_dev.json";

export const dojoConfig = createDojoConfig({
manifest,
Expand Down
2 changes: 1 addition & 1 deletion examples/example-vite-kitchen-sink/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createDojoConfig } from "@dojoengine/core";

import manifest from "../../worlds/dojo-starter/manifests/dev/deployment/manifest.json";
import manifest from "../../worlds/dojo-starter/manifest_dev.json";

export const dojoConfig = createDojoConfig({
manifest,
Expand Down
2 changes: 1 addition & 1 deletion examples/example-vite-react-app-recs/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createDojoConfig } from "@dojoengine/core";

import manifest from "../../worlds/dojo-starter/manifests/dev/deployment/manifest.json";
import manifest from "../../worlds/dojo-starter/manifest_dev.json";

export const dojoConfig = createDojoConfig({
manifest,
Expand Down
2 changes: 1 addition & 1 deletion examples/example-vite-react-phaser-recs/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import manifest from "../../worlds/dojo-starter/manifests/dev/deployment/manifest.json";
import manifest from "../../worlds/dojo-starter/manifest_dev.json";
import { createDojoConfig } from "@dojoengine/core";

export const dojoConfig = createDojoConfig({
Expand Down
2 changes: 1 addition & 1 deletion examples/example-vite-react-pwa-recs/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createDojoConfig } from "@dojoengine/core";

import manifest from "../../worlds/dojo-starter/manifests/dev/deployment/manifest.json";
import manifest from "../../worlds/dojo-starter/manifest_dev.json";
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Invalid manifest import path across multiple examples

The verification reveals critical issues:

  • The new import path ../../worlds/dojo-starter/manifest_dev.json is invalid as the file doesn't exist
  • Found inconsistency in example-vite-kitchen-sink using a different path: ../../../worlds/onchain-dash/manifests/release/deployment/manifest.json
  • References in example-nodejs-bot suggest manifest should be in dojo-starter/target/dev/manifest.json

This indicates the path changes are incorrect and need to be fixed to point to the actual manifest location.

🔗 Analysis chain

Verify the manifest file location and consistency.

The import path has been simplified, which aligns with the PR objective to fix paths. Let's verify the new structure is consistent and valid.

Let me verify if the manifest file exists in a different location or if there are any related manifest files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify manifest file existence and path consistency across examples

# Check if the new manifest file exists
echo "Checking manifest_dev.json existence:"
ls -l worlds/dojo-starter/manifest_dev.json

# Check for consistency across all dojoConfig.ts files
echo -e "\nChecking manifest import paths in all dojoConfig.ts files:"
fd dojoConfig.ts --exec grep -l "manifest.*json" {} \;

# Show the actual import lines to verify consistency
echo -e "\nShowing manifest import lines from all dojoConfig.ts files:"
fd dojoConfig.ts --exec grep "manifest.*json" {} \;

Length of output: 1817

Script:

#!/bin/bash
# Search for manifest files in the repository
echo "Searching for manifest files:"
fd manifest.*json

# Search for any references to manifest files in the codebase
echo -e "\nSearching for manifest file references in code:"
rg "manifest.*json" -g "!package*.json" -g "!dojoConfig.ts"

Length of output: 708


export const dojoConfig = createDojoConfig({
manifest,
Expand Down
2 changes: 1 addition & 1 deletion examples/example-vite-react-sdk/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createDojoConfig } from "@dojoengine/core";

import manifest from "../../worlds/dojo-starter/manifests/dev/deployment/manifest.json";
import manifest from "../../worlds/dojo-starter/manifest_dev.json";

export const dojoConfig = createDojoConfig({
manifest,
Expand Down
2 changes: 1 addition & 1 deletion examples/example-vite-react-threejs-recs/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createDojoConfig } from "@dojoengine/core";

import manifest from "../../worlds/dojo-starter/manifests/dev/deployment/manifest.json";
import manifest from "../../worlds/dojo-starter/manifest_dev.json";

export const dojoConfig = createDojoConfig({
manifest,
Expand Down
2 changes: 1 addition & 1 deletion examples/example-vite-svelte-recs/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import manifest from "../../worlds/dojo-starter/manifests/dev/deployment/manifest.json";
import manifest from "../../worlds/dojo-starter/manifest_dev.json";

import { createDojoConfig } from "@dojoengine/core";

Expand Down
2 changes: 1 addition & 1 deletion examples/example-vue-app-recs/dojoConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createDojoConfig } from "@dojoengine/core";

import manifest from "../../worlds/dojo-starter/manifests/dev/deployment/manifest.json";
import manifest from "../../worlds/dojo-starter/manifest_dev.json";
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Path inconsistency detected across examples

The verification reveals inconsistencies in manifest file imports:

  • Most examples use ../../worlds/dojo-starter/manifest_dev.json

  • One example (kitchen-sink/src/dojo.config.ts) still uses the old path structure: ../../../worlds/onchain-dash/manifests/release/deployment/manifest.json

  • The manifest files referenced in the imports are not present in the worlds directory structure

  • examples/example-vite-kitchen-sink/src/dojo.config.ts needs to be updated to use the new path structure

  • The manifest files (manifest_dev.json) are missing from the worlds/dojo-starter directory, which will cause runtime errors

  • Consider adding a note in the PR to ensure the manifest files are properly generated or copied to the new locations

🔗 Analysis chain

Verify the new manifest path structure.

The simplified path structure looks good and aligns with the PR's objective to fix paths. However, let's ensure the changes are complete and consistent.

Let's verify the changes:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify manifest file existence and check for any remaining old path references

# Test 1: Check if the new manifest file exists
if [ -f "worlds/dojo-starter/manifest_dev.json" ]; then
    echo "✓ New manifest file exists"
else
    echo "⨯ New manifest file not found"
fi

# Test 2: Search for any remaining references to the old path structure
echo "Checking for remaining references to old path..."
rg -l "manifests/dev/deployment/manifest.json"

# Test 3: Verify no other files are still using the old manifest path
echo "Checking for other dojoConfig files..."
fd "dojoConfig.ts" | xargs rg "manifests/dev/deployment"

Length of output: 415

Script:

#!/bin/bash
# Let's verify with a broader search to understand the manifest file structure

# Check for any manifest files in the repository
echo "=== Looking for manifest files ==="
fd "manifest.*\.json"

# Search for any manifest-related imports
echo -e "\n=== Checking manifest imports ==="
rg "import.*manifest.*from" -A 1

# Look for manifest-related paths in configuration files
echo -e "\n=== Checking configuration files for manifest paths ==="
fd "config|dojoConfig" | xargs rg "manifest"

# Check the specific directory structure
echo -e "\n=== Checking worlds directory structure ==="
ls -R worlds/ 2>/dev/null || echo "worlds directory not found"

Length of output: 5268


export const dojoConfig = createDojoConfig({
manifest,
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk/src/getEventMessages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export async function getEventMessages<T extends SchemaType>(
};

try {
const entities = await client.getEventMessages(toriiQuery);
const entities = await client.getEventMessages(toriiQuery, true);

if (options?.logging) {
console.log(`Fetched entities at offset ${cursor}:`, entities);
Expand Down
1 change: 1 addition & 0 deletions packages/sdk/src/subscribeEventQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export async function subscribeEventQuery<T extends SchemaType>(
): Promise<torii.Subscription> {
return client.onEventMessageUpdated(
convertQueryToEntityKeyClauses(query, schema),
true,
(entityId: string, entityData: any) => {
try {
if (callback) {
Expand Down
16 changes: 10 additions & 6 deletions packages/state/src/recs/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,15 @@ export const getEvents = async <S extends Schema>(
let continueFetching = true;

while (continueFetching) {
const entities = await client.getEventMessages({
limit,
offset,
clause,
dont_include_hashed_keys: false,
});
const entities = await client.getEventMessages(
{
limit,
offset,
clause,
dont_include_hashed_keys: false,
},
true
);

if (logging) console.log("entities", entities);

Expand Down Expand Up @@ -303,6 +306,7 @@ export const syncEvents = async <S extends Schema>(
if (logging) console.log("Starting syncEvents");
return await client.onEventMessageUpdated(
entityKeyClause,
true,
(fetchedEntities: any, data: any) => {
if (logging) console.log("Event message updated", fetchedEntities);

Expand Down
2 changes: 1 addition & 1 deletion readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ To run the examples, you'll need to set up three terminal windows:

```bash
sozo build
sozo migrate apply
sozo migrate
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Inconsistent migration commands in the documentation.

The migration command has been updated to sozo migrate in the Contributing section, but it still shows as sozo build && sozo migrate apply in the "Quick start in 5 minutes" section. This inconsistency could confuse users.

Update both instances to use the same command syntax:

- sozo build && sozo migrate apply
+ sozo build && sozo migrate

Committable suggestion skipped: line range outside the PR's diff.

```

3. Run Torii (indexer) with the world address and allowed origins:
Expand Down
Loading