improve transport management

[ponderingdemocritus]

User description

• improvements all around

---

PR Type

Enhancement, Bug fix

---

Description

• Added PromiseQueue class to manage promise queueing and introduced withQueueing and withErrorHandling wrappers for system calls.
• Refactored getArrivalsWithResources and getAllArrivalsWithResources to use useMemo for optimization.
• Simplified entity status rendering logic and updated background color logic based on entity state.
• Fixed notification count for resource arrivals and updated AllResourceArrivals component usage.

---

Changes walkthrough 📝

	Relevant files
Enhancement	createSystemCalls.ts Add promise queueing and error handling to system calls    client/src/dojo/createSystemCalls.ts Added PromiseQueue class to manage promise queueing. Introduced withQueueing and withErrorHandling wrappers for system calls. Applied queueing and error handling to specific system calls. +54/-7    useResources.tsx Optimize resource arrival queries with useMemo                      client/src/hooks/helpers/useResources.tsx Refactored getArrivalsWithResources to use useMemo. Refactored getAllArrivalsWithResources to use useMemo. Improved filtering and sorting logic for arrivals with resources. +44/-27  Entity.tsx Simplify entity status rendering and update styles              client/src/ui/components/entities/Entity.tsx Simplified entity status rendering logic. Updated background color logic based on entity state. +6/-10
Bug fix	RightNavigationModule.tsx Fix notification count and update resource arrivals component client/src/ui/modules/navigation/RightNavigationModule.tsx Fixed notification count for resource arrivals. Updated AllResourceArrivals component usage. +2/-2

---

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
eternum	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 25, 2024 0:05am

[github-actions[bot]]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	4
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Error Handling Consistency: The PromiseQueue class catches errors and logs them, but does not propagate them back to the caller. This could lead to situations where errors are silently swallowed, making debugging difficult. Consider rethrowing the error or handling it in a way that the caller can react to.
	Dependency on External State: The getAllArrivalsWithResources function in useResources.tsx now depends on Date.now() to filter arrivals, which can lead to different results on each call if the component re-renders around the boundary of the current time. This might not be intended and could lead to unpredictable UI behavior.
	useMemo Dependency Array: The useMemo hook used in getArrivalsWithResources and getAllArrivalsWithResources does not include all dependencies used inside the memoized function. For instance, account.address is used but not included in the dependency array. This can lead to stale closures capturing outdated values.

[github-actions[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	Implement a maximum queue size in PromiseQueue to prevent potential memory issues ___ **To avoid potential memory leaks or excessive memory usage, consider limiting the size of the queue in the PromiseQueue class. This can be achieved by setting a maximum queue size and implementing logic to handle the scenario when the queue is full.** [client/src/dojo/createSystemCalls.ts [5-7]](https://github.com/BibliothecaDAO/eternum/pull/1003/files#diff-322af6c204272be73084828a2b1bab251422314ba594744850c8659eed13a8faR5-R7) ```diff class PromiseQueue { private queue: (() => Promise)[] = []; private processing = false; + private maxQueueSize = 100; // Maximum number of tasks in the queue + + enqueue(task: () => Promise) { + if (this.queue.length >= this.maxQueueSize) { + throw new Error("Queue is full"); + } + return new Promise((resolve, reject) => { + this.queue.push(() => task().then(resolve).catch(reject)); + this.processQueue(); + }); + } } ``` Suggestion importance[1-10]: 8 Why: Limiting the queue size can prevent memory leaks and excessive memory usage, which is a significant enhancement for the stability of the application.	8
Possible issue	Add error handling in the PromiseQueue class to manage exceptions more effectively ___ **Consider adding error handling for the PromiseQueue class methods to ensure that errors are not only logged but also handled appropriately. This can prevent unhandled promise rejections and improve the robustness of the queue processing.** [client/src/dojo/createSystemCalls.ts [5-32]](https://github.com/BibliothecaDAO/eternum/pull/1003/files#diff-322af6c204272be73084828a2b1bab251422314ba594744850c8659eed13a8faR5-R32) ```diff class PromiseQueue { private queue: (() => Promise)[] = []; private processing = false; enqueue(task: () => Promise) { return new Promise((resolve, reject) => { - this.queue.push(() => task().then(resolve).catch(reject)); + this.queue.push(() => task().then(resolve).catch((error) => { + console.error("Failed to execute task:", error); + reject(error); + })); this.processQueue(); }); } private async processQueue() { if (this.processing) return; this.processing = true; while (this.queue.length > 0) { const task = this.queue.shift(); if (task) { try { await task(); } catch (error) { console.error("Error processing task:", error); + // Handle error appropriately here } } } this.processing = false; } } ``` Suggestion importance[1-10]: 7 Why: The suggestion improves error handling by logging and rejecting errors, which enhances robustness. However, it doesn't address all potential error handling scenarios comprehensively.	7
Maintainability	Refactor getAllArrivalsWithResources to improve code clarity and maintainability ___ **Refactor the getAllArrivalsWithResources function to separate concerns more clearly. This function is currently doing too much by fetching data, transforming it, and also filtering and sorting. Breaking it down into smaller functions can improve readability and maintainability.** [client/src/hooks/helpers/useResources.tsx [100-121]](https://github.com/BibliothecaDAO/eternum/pull/1003/files#diff-da868a5a06b096d28f8b0e970a0260a8d26d34f5acfafa3dcbd0156da8f15749R100-R121) ```diff -const getAllArrivalsWithResources = useMemo(() => { +const fetchArrivals = () => atPositionWithInventory.map((id) => { + const entityOwner = getComponentValue(EntityOwner, id); + const owner = getComponentValue(Owner, getEntityIdFromKeys([entityOwner?.entity_owner_id || BigInt(0)])); + const arrivalTime = getComponentValue(ArrivalTime, id); + const position = getComponentValue(Position, id); + + return { + id, + entityId: position?.entity_id || BigInt(""), + arrivesAt: Number(arrivalTime?.arrives_at || 0), + isOwner: BigInt(owner?.address || "") === BigInt(account.address), + }; +}); + +const filterAndSortArrivals = (arrivals) => { const currentTime = Date.now(); - - return atPositionWithInventory - .map((id) => { - const entityOwner = getComponentValue(EntityOwner, id); - const owner = getComponentValue(Owner, getEntityIdFromKeys([entityOwner?.entity_owner_id || BigInt(0)])); - const arrivalTime = getComponentValue(ArrivalTime, id); - const position = getComponentValue(Position, id); - - return { - id, - entityId: position?.entity_id || BigInt(""), - arrivesAt: Number(arrivalTime?.arrives_at || 0), - isOwner: BigInt(owner?.address || "") === BigInt(account.address), - }; - }) + return arrivals .filter(({ isOwner, arrivesAt }) => isOwner && arrivesAt <= currentTime) .sort((a, b) => a.arrivesAt - b.arrivesAt) .map(({ entityId }) => entityId) .filter((entityId) => entityId !== BigInt("")); -}, [atPositionWithInventory, account.address]); +}; +const getAllArrivalsWithResources = useMemo(() => filterAndSortArrivals(fetchArrivals()), [atPositionWithInventory, account.address]); + ``` Suggestion importance[1-10]: 6 Why: The refactoring suggestion improves readability and maintainability by separating concerns, but it doesn't introduce new functionality or fix critical issues.	6
	Simplify the renderEntityStatus function in the Entity component for better maintainability ___ **The conditional rendering logic in Entity component can be simplified by extracting it into a separate function or using a mapping object. This will make the component cleaner and easier to maintain.** [client/src/ui/components/entities/Entity.tsx [48-64]](https://github.com/BibliothecaDAO/eternum/pull/1003/files#diff-e41f7ca60992e606521c1c7a6cda0e416abefc1bc9ec7d0716bc4037a6cb1c43R48-R64) ```diff -const renderEntityStatus = () => { - switch (entityState) { - case EntityState.WaitingForDeparture: - return Waiting... ; - case EntityState.Idle: - case EntityState.WaitingToOffload: - return depositEntityId !== undefined && hasResources ? ( - Waiting to offload - ) : ( - Idle - ); - case EntityState.Traveling: - return entity.arrivalTime && nextBlockTimestamp ? ( - - {formatSecondsLeftInDaysHours(entity.arrivalTime - nextBlockTimestamp)} - - ) : null; - } +const statusMessages = { + [EntityState.WaitingForDeparture]: "Waiting...", + [EntityState.Idle]: "Idle", + [EntityState.WaitingToOffload]: "Waiting to offload", + [EntityState.Traveling]: () => entity.arrivalTime && nextBlockTimestamp ? formatSecondsLeftInDaysHours(entity.arrivalTime - nextBlockTimestamp) : null, }; +const renderEntityStatus = () => { + const message = statusMessages[entityState]; + return typeof message === 'function' ? message() : {message} ; +}; + ``` Suggestion importance[1-10]: 5 Why: The suggestion simplifies the conditional rendering logic, making the code cleaner and easier to maintain. However, the improvement is relatively minor.	5