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.

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

Raschel sync #2555

Merged
merged 13 commits into from
Dec 20, 2024
Merged

Raschel sync #2555

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
e6a0195
add army sync by position
aymericdelab Dec 17, 2024
3ff0406
add stamina
aymericdelab Dec 17, 2024
4fddc1a
limit sync because of useeffects
aymericdelab Dec 17, 2024
080f8ce
resource arrivals sync
aymericdelab Dec 17, 2024
40781d9
Merge branch 'next' into raschel-sync
aymericdelab Dec 19, 2024
c12696d
improve resource arrival component
aymericdelab Dec 19, 2024
f6b5866
add resource arrivals sync queries
aymericdelab Dec 19, 2024
27aeadd
add multiple loading states
aymericdelab Dec 19, 2024
dc94fac
fix
aymericdelab Dec 19, 2024
642dee4
add loading states and optimize queries
aymericdelab Dec 20, 2024
4548342
Merge branch 'next' into raschel-sync
aymericdelab Dec 20, 2024
de96dbf
revert event change
aymericdelab Dec 20, 2024
f44e971
merge
ponderingdemocritus Dec 20, 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
6 changes: 4 additions & 2 deletions client/src/dojo/setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,12 @@ export async function setup({ ...config }: DojoConfig) {
"s0_eternum-BankConfig",
"s0_eternum-Bank",
"s0_eternum-Trade",
"s0_eternum-Army",
// improve
// "s0_eternum-Army",
"s0_eternum-Structure",
"s0_eternum-Battle",
"s0_eternum-EntityOwner",
// improve
// "s0_eternum-EntityOwner",
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Both Army and EntityOwner components are actively used in the codebase

The components marked for improvement are actually critical parts of the system:

  • Army component is used in battle systems and army-related operations (create, delete, buy troops, merge troops)
  • EntityOwner component is used in conjunction with Army components for ownership tracking

Commenting them out in the entity fetch configuration (setup.ts) while they're still actively used elsewhere could lead to synchronization issues.

  • Remove the "improve" comments and uncomment these components in client/src/dojo/setup.ts
  • If improvements are needed, create separate tickets to track the improvements while keeping the components functional
🔗 Analysis chain

Clarify the improvement plan for commented-out entities.

The commented-out entities "s0_eternum-Army" and "s0_eternum-EntityOwner" are marked with "improve" comments, but there's no clear indication of the improvement plan or why they were commented out.

Let's verify if these entities are still being used:

Would you like me to help create a GitHub issue to track the improvement plan for these entities?

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Army and EntityOwner usage
rg -A 3 "s0_eternum-Army|s0_eternum-EntityOwner" --type ts --type tsx

Length of output: 99

Script:

#!/bin/bash
# Search for Army and EntityOwner usage with correct file type
rg -A 3 "s0_eternum-Army|s0_eternum-EntityOwner" --type typescript

# Also search for any references to these components in a case-insensitive manner
rg -i "army|entityowner" --type typescript

# Check if these components exist in any system definitions
fd "system" -e ts -x cat {} | rg -A 3 "Army|EntityOwner"

Length of output: 1761

Script:

#!/bin/bash
# Let's check the component definitions
fd "component" -e ts -x cat {} | rg -A 3 "Army|EntityOwner"

# Let's also check the setup file content to understand the context better
cat client/src/dojo/setup.ts

Length of output: 7823

],
},
},
Expand Down
112 changes: 56 additions & 56 deletions client/src/three/scenes/Worldmap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,62 +734,62 @@ export default class WorldmapScene extends HexagonScene {
1000,
false,
);
// const promisePositions = getEntities(
// this.dojo.network.toriiClient,
// {
// Composite: {
// operator: "And",
// clauses: [
// {
// Composite: {
// operator: "And",
// clauses: [
// {
// Member: {
// model: "s0_eternum-Position",
// member: "x",
// operator: "Gte",
// value: { Primitive: { U32: startCol - range } },
// },
// },
// {
// Member: {
// model: "s0_eternum-Position",
// member: "x",
// operator: "Lte",
// value: { Primitive: { U32: startCol + range } },
// },
// },
// {
// Member: {
// model: "s0_eternum-Position",
// member: "y",
// operator: "Gte",
// value: { Primitive: { U32: startRow - range } },
// },
// },
// {
// Member: {
// model: "s0_eternum-Position",
// member: "y",
// operator: "Lte",
// value: { Primitive: { U32: startRow + range } },
// },
// },
// ],
// },
// },
// ],
// },
// },
// this.dojo.network.contractComponents as any,
// [],
// ["s0_eternum-Tile"],
// 1000,
// false,
// );
Promise.all([promiseTiles]).then(([tiles]) => {
// console.log(tiles, positions);
const promisePositions = getEntities(
this.dojo.network.toriiClient,
{
Composite: {
operator: "And",
clauses: [
{
Member: {
model: "s0_eternum-Position",
member: "x",
operator: "Gte",
value: { Primitive: { U32: startCol - range } },
},
},
{
Member: {
model: "s0_eternum-Position",
member: "x",
operator: "Lte",
value: { Primitive: { U32: startCol + range } },
},
},
{
Member: {
model: "s0_eternum-Position",
member: "y",
operator: "Gte",
value: { Primitive: { U32: startRow - range } },
},
},
{
Member: {
model: "s0_eternum-Position",
member: "y",
operator: "Lte",
value: { Primitive: { U32: startRow + range } },
},
},
],
},
},
this.dojo.network.contractComponents as any,
[],
[
"s0_eternum-Army",
"s0_eternum-Position",
"s0_eternum-Health",
"s0_eternum-EntityOwner",
"s0_eternum-Protectee",
"s0_eternum-Stamina",
],
1000,
false,
);
Promise.all([promiseTiles, promisePositions]).then(([tiles, positions]) => {
// Promise.all([promiseTiles]).then(([tiles]) => {
});
} catch (error) {
// If there's an error, remove the chunk from cached set so it can be retried
Expand Down
3 changes: 3 additions & 0 deletions client/src/three/systems/SystemManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ export class SystemManager {
isComponentUpdate(update, this.setup.components.Health)
) {
const army = getComponentValue(this.setup.components.Army, update.entity);

console.log({ armyUpdates: army });

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove or conditionally enable debug logging.

The console.log statement for army updates could impact performance and clutter the console in production.

Consider using a debug flag:

+const DEBUG_ARMY_UPDATES = process.env.NODE_ENV === 'development';

 const army = getComponentValue(this.setup.components.Army, update.entity);
-console.log({ armyUpdates: army });
+if (DEBUG_ARMY_UPDATES) {
+  console.log({ armyUpdates: army });
+}

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

if (!army) return;

const position = getComponentValue(this.setup.components.Position, update.entity);
Expand Down
88 changes: 50 additions & 38 deletions client/src/ui/layouts/World.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Leva } from "leva";
import { lazy, Suspense, useEffect, useMemo, useState } from "react";
import { lazy, Suspense, useEffect } from "react";
import { Redirect } from "wouter";
import useUIStore from "../../hooks/store/useUIStore";

Expand All @@ -10,7 +10,7 @@ import {
addToSubscriptionTwoKeyModelbyRealmEntityId,
} from "@/dojo/queries";
import { useDojo } from "@/hooks/context/DojoContext";
import { PlayerStructure, useEntities } from "@/hooks/helpers/useEntities";
import { useEntities } from "@/hooks/helpers/useEntities";
import { useStructureEntityId } from "@/hooks/helpers/useStructureEntityId";
import { useFetchBlockchainData } from "@/hooks/store/useBlockchainStore";
import { useWorldStore } from "@/hooks/store/useWorldLoading";
Expand Down Expand Up @@ -91,7 +91,6 @@ const MiniMapNavigation = lazy(() =>
);

export const World = ({ backgroundImage }: { backgroundImage: string }) => {
const [subscriptions, setSubscriptions] = useState<{ [entity: string]: boolean }>({});
const showBlankOverlay = useUIStore((state) => state.showBlankOverlay);
const isLoadingScreenEnabled = useUIStore((state) => state.isLoadingScreenEnabled);

Expand All @@ -106,7 +105,6 @@ export const World = ({ backgroundImage }: { backgroundImage: string }) => {

// We could optimise this deeper....

const worldLoading = useWorldStore((state) => state.isWorldLoading);
const setWorldLoading = useWorldStore((state) => state.setWorldLoading);
const setMarketLoading = useWorldStore((state) => state.setMarketLoading);

Expand All @@ -116,62 +114,76 @@ export const World = ({ backgroundImage }: { backgroundImage: string }) => {
const { playerStructures } = useEntities();
const structures = playerStructures();

const filteredStructures = useMemo(
() => structures.filter((structure: PlayerStructure) => !subscriptions[structure.entity_id.toString()]),
[structures, subscriptions],
);

useEffect(() => {
if (
!structureEntityId ||
subscriptions[structureEntityId.toString()] ||
subscriptions[ADMIN_BANK_ENTITY_ID.toString()] ||
structureEntityId === 999999999
) {
return;
}

const position = getComponentValue(
dojo.setup.components.Position,
getEntityIdFromKeys([BigInt(structureEntityId)]),
);

setWorldLoading(true);
setSubscriptions((prev) => ({
...prev,
[structureEntityId.toString()]: true,
[ADMIN_BANK_ENTITY_ID.toString()]: true,
...Object.fromEntries(filteredStructures.map((structure) => [structure.entity_id.toString(), true])),
}));

const fetch = async () => {
try {
await Promise.all([
addToSubscriptionOneKeyModelbyRealmEntityId(
addToSubscription(
dojo.network.toriiClient,
dojo.network.contractComponents as any,
[...filteredStructures.map((structure) => structure.entity_id.toString())],
[structureEntityId.toString()],
[{ x: position?.x || 0, y: position?.y || 0 }],
),
addToSubscriptionTwoKeyModelbyRealmEntityId(
]);
} catch (error) {
console.error("Fetch failed", error);
} finally {
setWorldLoading(false);
setMarketLoading(false);
}
};

fetch();
}, [structureEntityId]);

useEffect(() => {
const structuresEntityIds = structures.map((structure) => structure.entity_id.toString());

const fetch = async () => {
try {
await Promise.all([
addToSubscriptionOneKeyModelbyRealmEntityId(
dojo.network.toriiClient,
dojo.network.contractComponents as any,
[...filteredStructures.map((structure) => structure.entity_id.toString())],
structuresEntityIds,
),
addToSubscription(
addToSubscriptionTwoKeyModelbyRealmEntityId(
dojo.network.toriiClient,
dojo.network.contractComponents as any,
[structureEntityId.toString()],
[{ x: position?.x || 0, y: position?.y || 0 }],
structuresEntityIds,
),
addToSubscription(dojo.network.toriiClient, dojo.network.contractComponents as any, structuresEntityIds, [
...structures.map((structure) => ({ x: structure.position.x, y: structure.position.y })),
]),
]);
} catch (error) {
console.error("Fetch failed", error);
} finally {
setWorldLoading(false);
setMarketLoading(false);
}
};

fetch();
}, [structures]);
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Optimize batch fetching for structures.

The current implementation makes multiple API calls in parallel for each structure. Consider batching these calls to reduce network overhead.

Consider consolidating the API calls:

 const fetch = async () => {
   try {
+    const batchSize = 50;
+    const batches = [];
+    for (let i = 0; i < structuresEntityIds.length; i += batchSize) {
+      const batch = structuresEntityIds.slice(i, i + batchSize);
       await Promise.all([
         addToSubscriptionOneKeyModelbyRealmEntityId(
           dojo.network.toriiClient,
           dojo.network.contractComponents as any,
-          structuresEntityIds,
+          batch,
         ),
         addToSubscriptionTwoKeyModelbyRealmEntityId(
           dojo.network.toriiClient,
           dojo.network.contractComponents as any,
-          structuresEntityIds,
+          batch,
         ),
       ]);
+    }
   } catch (error) {
     console.error("Fetch failed", error);
   }
 };

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


useEffect(() => {
setWorldLoading(true);

const fetch = async () => {
try {
await Promise.all([
addToSubscription(dojo.network.toriiClient, dojo.network.contractComponents as any, [
ADMIN_BANK_ENTITY_ID.toString(),
]),
addToSubscription(
dojo.network.toriiClient,
dojo.network.contractComponents as any,
[...filteredStructures.map((structure) => structure.entity_id.toString())],
[...filteredStructures.map((structure) => ({ x: structure.position.x, y: structure.position.y }))],
),

addMarketSubscription(dojo.network.toriiClient, dojo.network.contractComponents as any),
]);
} catch (error) {
Expand All @@ -183,7 +195,7 @@ export const World = ({ backgroundImage }: { backgroundImage: string }) => {
};

fetch();
}, [structureEntityId, subscriptions, setWorldLoading, setSubscriptions, filteredStructures]);
}, []);

return (
<div
Expand Down
Loading