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.

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

feat: add Torii local launcher #2663

Open
wants to merge 4 commits into
base: next
Choose a base branch
from
Open

feat: add Torii local launcher #2663

wants to merge 4 commits into from

Conversation

edisontim
Copy link
Collaborator

@edisontim edisontim commented Jan 23, 2025

Allows a user to simply download a .dmg file built from this app, install the dmg and then run a local torii pointing to mainnet.

Configuration is dynamic, user can choose to use a custom RPC, mainnet, sepolia or localhost.

Further improvement, handle torii version if torii is already installed on user's machine. Currently it checks if the user has torii and in that case just uses the version of the user. Otherwise it downloads the compatible version

Summary by CodeRabbit

Release Notes for Torii Launcher

New Features

  • Introduced a comprehensive Electron application for the Torii Launcher
  • Added RPC configuration management with support for Mainnet, Sepolia, and Localhost networks
  • Implemented synchronization state tracking for blockchain indexing
  • Developed a notification system with toast alerts
  • Created a settings panel for RPC configuration

Improvements

  • Enhanced configuration management with TypeScript and Webpack
  • Added support for SVG imports and Tailwind CSS
  • Implemented robust error handling and logging mechanisms
  • Created reusable UI components like Button and Warning

Technical Updates

  • Configured Electron Forge for application packaging
  • Set up Vite and Webpack build configurations
  • Integrated Inter-Process Communication (IPC) for message handling
  • Added TypeScript type definitions and ESLint configuration

Development Tools

  • Added ESLint configuration
  • Configured PostCSS and Tailwind CSS
  • Set up Webpack and Vite build configurations
  • Implemented TypeScript compilation settings

@edisontim edisontim marked this pull request as ready for review January 23, 2025 22:02
Copy link
Contributor

mentatbot bot commented Jan 23, 2025

Hi @edisontim! You need to be added as a user to interact with me. Please ask @ponderingdemocritus to add you on the settings page.

You are receiving this comment because I am set to review all PRs. That is configurable here.

Copy link

vercel bot commented Jan 23, 2025

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

Name Status Preview Comments Updated (UTC)
eternum ❌ Failed (Inspect) Jan 29, 2025 9:57pm
eternum-docs ❌ Failed (Inspect) Jan 29, 2025 9:57pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
eternum-landing ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2025 9:57pm

Copy link
Contributor

coderabbitai bot commented Jan 23, 2025

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

client/apps/game/src/ui/modules/loading-screen.tsx

Oops! Something went wrong! :(

ESLint: 9.18.0

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.

From ESLint v9.0.0, the default configuration file is now eslint.config.js.
If you are using a .eslintrc.* file, please follow the migration guide
to update your configuration file to the new format:

https://eslint.org/docs/latest/use/configure/migration-guide

If you still have problems after following the migration guide, please stop by
https://eslint.org/chat/help to chat with the team.

client/apps/torii-launcher/src/constants.ts

Oops! Something went wrong! :(

ESLint: 9.18.0

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.

From ESLint v9.0.0, the default configuration file is now eslint.config.js.
If you are using a .eslintrc.* file, please follow the migration guide
to update your configuration file to the new format:

https://eslint.org/docs/latest/use/configure/migration-guide

If you still have problems after following the migration guide, please stop by
https://eslint.org/chat/help to chat with the team.

client/apps/torii-launcher/src/frontend/app.tsx

Oops! Something went wrong! :(

ESLint: 9.18.0

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.

From ESLint v9.0.0, the default configuration file is now eslint.config.js.
If you are using a .eslintrc.* file, please follow the migration guide
to update your configuration file to the new format:

https://eslint.org/docs/latest/use/configure/migration-guide

If you still have problems after following the migration guide, please stop by
https://eslint.org/chat/help to chat with the team.

  • 15 others

Walkthrough

This pull request introduces a comprehensive configuration and setup for the Torii Launcher, an Electron-based application for managing blockchain interactions. The changes include multiple configuration files for build tools (Webpack, Vite), TypeScript, ESLint, and Tailwind CSS. The project establishes a robust development environment with support for React, TypeScript, and inter-process communication in an Electron application. Key additions include launcher components, notification handling, settings management, and synchronization state tracking.

Changes

File Change Summary
forge.config.ts Added Electron Forge configuration with makers, publishers, and plugins
package.json Defined project metadata, scripts, and dependencies
src/main.ts Implemented comprehensive Electron main process logic with IPC methods
src/types.ts Added type definitions for IPC methods and RPC configurations
src/frontend/* Added React components for launcher, settings, notifications, and UI utilities
Configuration Files Added Webpack, Vite, ESLint, PostCSS, Tailwind configs

Sequence Diagram

sequenceDiagram
    participant User
    participant Launcher
    participant ElectronAPI
    participant ToriiService
    participant Blockchain

    User->>Launcher: Open Application
    Launcher->>ElectronAPI: Request RPC Settings
    ElectronAPI-->>Launcher: Return Current RPC
    Launcher->>ToriiService: Initialize Synchronization
    ToriiService->>Blockchain: Fetch Current Block
    Blockchain-->>ToriiService: Return Block Number
    ToriiService->>Launcher: Update Sync Progress
    User->>Launcher: Change RPC Settings
    Launcher->>ElectronAPI: Send New RPC Configuration
    ElectronAPI->>ToriiService: Restart with New RPC
Loading

Possibly related PRs

Poem

🐰 Hop, hop, config deploy!
Electron's launcher, a coding joy
Webpack and Vite, dancing light
TypeScript rules, everything's right
A rabbit's code, precise and neat 🚀

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Failed to generate code suggestions for PR

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 22

🧹 Nitpick comments (28)
client/apps/torii-launcher/webpack.rules.js (2)

4-4: Improve the test pattern regex for better file matching.

The current regex pattern /.jsx?$/ could potentially match unwanted files. Consider using a more specific pattern.

-    test: /.jsx?$/,
+    test: /^[^.]+\.jsx?$/,

1-14: Consider adding TypeScript support for better type safety.

Since this is an Electron application, consider adding TypeScript support to improve maintainability and catch potential issues early.

You would need to:

  1. Install required dependencies (@babel/preset-typescript, @types/webpack)
  2. Add TypeScript configuration
  3. Update the test pattern to include .tsx files

Would you like me to provide the necessary configuration changes?

client/apps/torii-launcher/webpack.plugins.ts (2)

3-4: Consider future migration to ESM imports.

While the current CommonJS require() approach with type assertion works correctly, consider migrating to ESM imports when possible. This would eliminate the need for the ESLint disable comment and provide better static analysis.

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


7-9: Consider enhancing type checking configuration.

The current ForkTsCheckerWebpackPlugin configuration is minimal. Consider adding these options to improve type checking:

  • async: true - for better build performance
  • typescript: { configFile: './tsconfig.json' } - explicit TypeScript config
  • issue: { include: [{ file: '**/src/**/*.{ts,tsx}' }] } - scope type checking to source files
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/webpack.rules.ts (2)

11-20: Consider adding optimization options.

While the current configuration is correct, consider adding the following optimizations:

  • Cache the loader output using cacheDirectory: true
  • Add include path to limit the scope to your app's node_modules
 {
   test: /[/\\]node_modules[/\\].+\.(m?js|node)$/,
   parser: { amd: false },
   use: {
     loader: '@vercel/webpack-asset-relocator-loader',
     options: {
       outputAssetBase: 'native_modules',
+      cacheDirectory: true
     },
   },
+  include: /node_modules/,
 },
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


1-31: Fix code formatting.

The GitHub Actions pipeline indicates that the code formatting doesn't match Prettier standards. Please run Prettier to format the code:

npx prettier --write client/apps/torii-launcher/webpack.rules.ts
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/webpack.renderer.config.ts (2)

11-34: Consider using array spread for cleaner rule addition.

The configuration is correct, but the syntax could be more elegant.

-rules.push(
-  {
-    test: /\.tsx?$/,
+rules.push(...[
+  {
+    test: /\.tsx?$/,
     include: [path.resolve(__dirname, "src")],
     loader: "babel-loader",
     resolve: {
       extensions: [".js", ".jsx", ".json", ".ts", ".tsx"],
     },
   },
   {
     test: /\.(html)$/,
     include: [path.resolve(__dirname, "src")],
     use: {
       loader: "html-loader",
     },
   },
   {
     test: /\.css$/,
     include: [path.resolve(__dirname, "src")],
     use: ["style-loader", "css-loader", "postcss-loader"],
   }
-);
+]);
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


42-50: Ensure consistent extension lists.

The resolve.extensions list is shorter than the one defined in the TypeScript rule (missing .jsx and .json).

   resolve: {
-    extensions: [".js", ".ts", ".jsx", ".tsx"],
+    extensions: [".js", ".jsx", ".json", ".ts", ".tsx"],
   },
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/public/torii.toml (1)

3-3: Consider environment-specific RPC configuration.

While hardcoding the mainnet RPC endpoint aligns with the current PR objectives, consider making this configurable through environment variables for future flexibility.

Example approach:

rpc = "${TORII_RPC_URL:-https://api.cartridge.gg/x/starknet/mainnet}"
client/apps/torii-launcher/webpack.main.config.ts (4)

11-11: Consider using path.resolve for the entry point.

The relative path './src/index.ts' might cause issues depending on the working directory. Using path.resolve would make this more robust.

+import path from 'path';
+
 export const mainConfig: Configuration = {
-  entry: './src/index.ts',
+  entry: path.resolve(__dirname, './src/index.ts'),
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


12-12: Remove or improve the TODO-style comment.

The comment "Put your normal webpack config below here" doesn't provide value and should be removed or replaced with meaningful documentation.

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


1-20: Fix code formatting.

The pipeline shows Prettier formatting issues. Run Prettier to fix the formatting:

#!/bin/bash
npx prettier --write "client/apps/torii-launcher/webpack.main.config.ts"
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


6-20: Consider environment-specific configurations.

Since this is an Electron app, consider:

  1. Adding environment-specific settings (development vs production)
  2. Enabling source maps for development
  3. Adding optimization settings for production builds

Example structure:

export const mainConfig: Configuration = {
  ...baseConfig,
  mode: process.env.NODE_ENV === 'production' ? 'production' : 'development',
  devtool: process.env.NODE_ENV === 'production' ? false : 'source-map',
  optimization: process.env.NODE_ENV === 'production' ? {
    minimize: true,
    minimizer: [new TerserPlugin()],
  } : undefined,
}
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/src/main.ts (1)

16-19: Consider disabling nodeIntegration for improved security.

Enabling nodeIntegration in webPreferences can expose your application to security risks if untrusted content is loaded or if XSS vulnerabilities exist. If Node.js integration is not required in the renderer process, it's recommended to set nodeIntegration to false and use a preload script to expose necessary APIs.

Apply this diff to enhance security:

 webPreferences: {
   preload: path.join(__dirname, "preload.js"),
-  nodeIntegration: true,
+  nodeIntegration: false,
 }
client/apps/torii-launcher/src/frontend/Logo-animated.tsx (2)

1-5: Code formatting does not match Prettier standards.

The code does not adhere to the project's formatting guidelines enforced by Prettier, as indicated by the pipeline failure. Please format your code using Prettier to pass the linting checks.

You can run the following command to format your code:

npm run format
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


3-5: Use PascalCase for component filenames for consistency.

React components are typically named using PascalCase. Consider renaming Logo-animated.tsx to LogoAnimated.tsx for consistency with other components and to follow naming conventions.

Apply this change:

  • Rename file from Logo-animated.tsx to LogoAnimated.tsx.
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/src/index.tsx (1)

6-7: Add React.StrictMode and error boundaries

Consider wrapping the app with StrictMode for additional runtime checks and error boundaries for graceful error handling.

Apply this diff:

+import { StrictMode } from 'react';
+import { ErrorBoundary } from './components/ErrorBoundary';

const rootElement = document.createElement('div');
rootElement.id = 'root';
document.body.appendChild(rootElement);
const root = createRoot(rootElement);
-root.render(<App />);
+root.render(
+  <StrictMode>
+    <ErrorBoundary>
+      <App />
+    </ErrorBoundary>
+  </StrictMode>
+);
client/apps/torii-launcher/src/preload.ts (2)

6-6: Use ES modules consistently

Replace require with import for consistency with the rest of the codebase.

Apply this diff:

-const { contextBridge, ipcRenderer } = require("electron/renderer");
+import { contextBridge, ipcRenderer } from "electron/renderer";

8-10: Improve type safety for IPC communication

Consider adding type definitions for the payload object and declaring the API interface.

Apply this diff:

+interface IpcPayload {
+  // Add your payload properties here
+}
+
+interface ElectronAPI {
+  sendMessage: (message: IpcMethod, payload?: IpcPayload) => void;
+}
+
-contextBridge.exposeInMainWorld("electronAPI", {
-  sendMessage: (message: IpcMethod) => ipcRenderer.send(message, {}),
+contextBridge.exposeInMainWorld("electronAPI", {
+  sendMessage: (message: IpcMethod, payload: IpcPayload = {}) => 
+    ipcRenderer.send(message, payload),
} as ElectronAPI);
client/apps/torii-launcher/tailwind.config.js (2)

3-3: Expand content glob pattern

The current pattern might miss some file extensions. Consider including more file types that might contain Tailwind classes.

Apply this diff:

-  content: ["./src/**/*.{html,tsx}"],
+  content: ["./src/**/*.{html,tsx,ts,jsx,js}"],

6-10: Consider extracting theme values

Move color definitions to a separate theme file for better maintainability and reusability across the application.

Create a new file src/theme/colors.ts:

export const colors = {
  brown: "#0C0A08",
  gold: "#F6C297",
  red: "#FC4C4C",
} as const;

Then import it in the Tailwind config:

+const { colors } = require('./src/theme/colors');
 module.exports = {
   theme: {
     extend: {
-      colors: {
-        brown: "#0C0A08",
-        gold: "#F6C297",
-        red: "#FC4C4C",
-      },
+      colors,
client/apps/torii-launcher/forge.config.ts (1)

24-27: Use environment variables for configurable paths

Consider making asset paths configurable through environment variables for better flexibility across different environments.

Apply this diff:

+const iconPath = process.env.ICON_PATH || "./assets/icon.png";
+
 config: {
-  background: "./assets/icon.png",
+  background: iconPath,
   format: "ULFO",
-  icon: "./assets/icon.png",
+  icon: iconPath,
   name: "Eternum Launcher",
 },
🧰 Tools
🪛 GitHub Actions: knip

[warning] Unused file detected

client/apps/torii-launcher/src/frontend/Button.tsx (1)

23-42: Consider extracting styles to a separate constants file.

The STYLES object contains a large number of style definitions that could be better organized in a separate file.

Consider creating a new file ButtonStyles.ts to improve maintainability and reusability.

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/src/frontend/app.tsx (1)

35-44: Consider moving styled components to a separate file.

The DraggableArea styled component should be defined outside the component to prevent unnecessary re-creation.

Move the styled component to a separate file (e.g., StyledComponents.ts).

client/apps/torii-launcher/package.json (1)

6-6: Update application description.

The default Electron application description should be replaced with a meaningful description.

-  "description": "My Electron application description",
+  "description": "Eternum Launcher - A desktop application for managing Torii instances",
client/apps/torii-launcher/png_to_isns.sh (3)

18-30: Improve command-line argument handling.

  1. The initial argument check might interfere with valid flag-only usage (e.g., -h).
  2. The case statement combines h with the wildcard, making it less explicit.

Apply this diff:

-# If no option were passed display usage and exit
-if [[ $1 == "" ]]
-then
-    usageFunction
-fi
-
 # Iterate over script options
 while getopts "hi:" opt; do
    case "$opt" in
       i ) image_path="$OPTARG" ;;
-      h | *) usageFunction ;;
+      h ) usageFunction ;;
+      * ) usageFunction ;;
    esac
 done

89-100: Improve result reporting.

  1. Use absolute path in success message.
  2. Provide more specific error information.
  3. Use appropriate exit codes.

Apply this diff:

 # Echo result status
 icon_path="${icon_name}.${icon_extension}"
 if [ -f "$icon_path" ]
 then
-    echo "INFO: The icon has been successfully created: ./${icon_path}"
+    echo "INFO: Icon successfully created: $(pwd)/${icon_path}"
+    exit 0
 else
-    # First print an empty line, to help lisibility
-    echo ""
-    echo "ERROR: An error occurred please check the logs above."
+    echo "ERROR: Failed to create icon '${icon_path}'."
+    exit 1
 fi
-
-exit 0

3-5: Add comprehensive documentation.

Add a documentation section that clearly states:

  1. Platform requirements (Mac OS only)
  2. Required commands (sips, iconutil)
  3. Environment variables (ICON_BASE_NAME)
  4. Example usage

Add this documentation after the copyright notice:

 # PNG to ICNS v1.0 - https://github.com/BenSouchet/png-to-icns
 # Copyright(C) 2022 Ben Souchet | MIT License
+
+# Requirements:
+# - Mac OS (uses Mac OS specific commands: sips, iconutil)
+#
+# Environment variables:
+# - ICON_BASE_NAME: Optional. Base name for the output icon (default: "icon")
+#
+# Example usage:
+# ./png_to_icns.sh -i input.png            # Creates icon.icns
+# ICON_BASE_NAME=app ./png_to_icns.sh -i input.png  # Creates app.icns
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 44f0db3 and cb918e0.

⛔ Files ignored due to path filters (10)
  • .DS_Store is excluded by !**/.DS_Store
  • client/apps/torii-launcher/assets/icon.ico is excluded by !**/*.ico
  • client/apps/torii-launcher/assets/icon.png is excluded by !**/*.png
  • client/apps/torii-launcher/package-lock.json is excluded by !**/package-lock.json
  • client/apps/torii-launcher/public/eternum-new.svg is excluded by !**/*.svg
  • client/apps/torii-launcher/public/favicon.ico is excluded by !**/*.ico
  • client/apps/torii-launcher/public/icon.ico is excluded by !**/*.ico
  • client/apps/torii-launcher/public/icon.png is excluded by !**/*.png
  • client/apps/torii-launcher/src/assets/eternum-new.svg is excluded by !**/*.svg
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (30)
  • client/apps/torii-launcher/.eslintrc.json (1 hunks)
  • client/apps/torii-launcher/.gitignore (1 hunks)
  • client/apps/torii-launcher/forge.config.ts (1 hunks)
  • client/apps/torii-launcher/forge.env.d.ts (1 hunks)
  • client/apps/torii-launcher/index.html (1 hunks)
  • client/apps/torii-launcher/package.json (1 hunks)
  • client/apps/torii-launcher/png_to_isns.sh (1 hunks)
  • client/apps/torii-launcher/postcss.config.js (1 hunks)
  • client/apps/torii-launcher/public/torii.toml (1 hunks)
  • client/apps/torii-launcher/src/frontend/Button.tsx (1 hunks)
  • client/apps/torii-launcher/src/frontend/Logo-animated.tsx (1 hunks)
  • client/apps/torii-launcher/src/frontend/SyncingState.tsx (1 hunks)
  • client/apps/torii-launcher/src/frontend/app.tsx (1 hunks)
  • client/apps/torii-launcher/src/index.css (1 hunks)
  • client/apps/torii-launcher/src/index.tsx (1 hunks)
  • client/apps/torii-launcher/src/main.ts (1 hunks)
  • client/apps/torii-launcher/src/preload.ts (1 hunks)
  • client/apps/torii-launcher/src/renderer.ts (1 hunks)
  • client/apps/torii-launcher/src/types.ts (1 hunks)
  • client/apps/torii-launcher/tailwind.config.js (1 hunks)
  • client/apps/torii-launcher/tsconfig.json (1 hunks)
  • client/apps/torii-launcher/vite.main.config.ts (1 hunks)
  • client/apps/torii-launcher/vite.preload.config.ts (1 hunks)
  • client/apps/torii-launcher/vite.renderer.config.ts (1 hunks)
  • client/apps/torii-launcher/webpack.main.config.ts (1 hunks)
  • client/apps/torii-launcher/webpack.plugins.ts (1 hunks)
  • client/apps/torii-launcher/webpack.renderer.config.ts (1 hunks)
  • client/apps/torii-launcher/webpack.rules.js (1 hunks)
  • client/apps/torii-launcher/webpack.rules.ts (1 hunks)
  • notes (0 hunks)
💤 Files with no reviewable changes (1)
  • notes
✅ Files skipped from review due to trivial changes (11)
  • client/apps/torii-launcher/vite.preload.config.ts
  • client/apps/torii-launcher/postcss.config.js
  • client/apps/torii-launcher/vite.main.config.ts
  • client/apps/torii-launcher/src/index.css
  • client/apps/torii-launcher/.gitignore
  • client/apps/torii-launcher/forge.env.d.ts
  • client/apps/torii-launcher/.eslintrc.json
  • client/apps/torii-launcher/src/renderer.ts
  • client/apps/torii-launcher/vite.renderer.config.ts
  • client/apps/torii-launcher/tsconfig.json
  • client/apps/torii-launcher/index.html
🧰 Additional context used
🪛 GitHub Actions: lint
client/apps/torii-launcher/webpack.plugins.ts

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/webpack.main.config.ts

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/src/frontend/Logo-animated.tsx

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/webpack.renderer.config.ts

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/webpack.rules.ts

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/src/frontend/Button.tsx

[warning] Code formatting does not match Prettier standards

🪛 GitHub Actions: knip
client/apps/torii-launcher/forge.config.ts

[warning] Unused file detected

🪛 Biome (1.9.4)
client/apps/torii-launcher/src/frontend/SyncingState.tsx

[error] 63-63: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

🔇 Additional comments (14)
client/apps/torii-launcher/webpack.plugins.ts (2)

1-2: LGTM! Clean type import.

The type-only import for ForkTsCheckerWebpackPlugin follows TypeScript best practices.

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


6-10: Fix formatting to match Prettier standards.

The configuration looks good, but there are formatting issues reported by the pipeline.

Run this command to fix formatting:

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/webpack.rules.ts (3)

1-3: LGTM! Good type safety practices.

The use of explicit type imports and the Required utility type ensures proper type checking for webpack configuration.

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


4-10: LGTM! Proper native module handling.

The configuration correctly handles native Node modules with appropriate path matching for cross-platform compatibility.

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


21-30: Ensure type checking is handled separately.

While using transpileOnly: true improves build performance, it skips type checking. Verify that you have ForkTsCheckerWebpackPlugin configured in your webpack plugins to run type checking in parallel.

✅ Verification successful

Type checking is properly configured

The ForkTsCheckerWebpackPlugin is correctly set up in webpack.plugins.ts to handle type checking in parallel, which complements the transpileOnly: true setting.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if ForkTsCheckerWebpackPlugin is properly configured
rg -l "ForkTsCheckerWebpackPlugin" client/apps/torii-launcher/

Length of output: 108


Script:

#!/bin/bash
# Check the content of webpack.plugins.ts
cat client/apps/torii-launcher/webpack.plugins.ts

Length of output: 416

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/webpack.renderer.config.ts (2)

1-5: LGTM! Clean and well-structured imports.

The imports are properly organized, with clear separation between standard libraries, webpack types, and local configuration files.

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


1-50: Fix code formatting.

The pipeline indicates that the code formatting doesn't match Prettier standards. Please run Prettier to format the file:

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/public/torii.toml (3)

12-14: Verify world block value and empty contracts configuration.

Two concerns:

  1. The world_block = 940000 seems arbitrary. Consider documenting why this specific block was chosen.
  2. The empty contracts array might indicate missing configuration.

Let's verify the world block value and expected contracts:

✅ Verification successful

Configuration is valid for the launcher's event consumption role

The world_block value matches the mainnet configuration, and the empty contracts array is appropriate for this launcher configuration file which focuses on event consumption rather than contract interaction.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for world block references
rg -A 5 "world_block.*940000"

# Search for contract configurations
rg -A 5 "contracts.*\[" --type toml

Length of output: 3555


1-2: Uncomment the database directory configuration.

The database directory configuration is commented out. This might cause issues if there's no default fallback in the application.

Let's verify if there's a default fallback in the codebase:

✅ Verification successful

The commented database directory configuration is intentional

The database directory appears to be dynamically set based on the network configuration, with a default pattern of "torii.$NETWORK.db". This is consistent across all environment configurations and is handled by the indexer script.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for default database directory configuration
rg -A 5 "db_dir" --type rust --type typescript

Length of output: 81


Script:

#!/bin/bash
# Search for db_dir and related configuration terms
rg "db_dir|database.*dir|DB_DIR" -i

# Search for configuration handling files
fd -e rs -e ts -e js -e json "config|torii"

Length of output: 11860


16-18: Verify events configuration completeness.

The events configuration:

  1. Enables raw event processing which might have performance implications
  2. Only tracks two trophy-related events (TrophyCreation and TrophyProgression)

Verify if this covers all necessary events for the application.

Let's search for other relevant events in the codebase:

✅ Verification successful

Events configuration is complete and appropriate.

The configuration correctly tracks all trophy-related events:

  • TrophyCreation: Captures trophy unlocks
  • TrophyProgression: Tracks advancement through trophy levels

Raw event processing is necessary for the trophy achievement system to function properly.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for event definitions and usages
rg "s0_eternum-Trophy" --type typescript --type rust

Length of output: 87


Script:

#!/bin/bash
# Search for eternum events and related patterns
rg "s0_eternum-" -A 2 -B 2

# Search for event-related configurations
fd "toml$" --exec grep -A 5 -B 5 "\[events\]" {} \;

# Search for trophy-related code
rg -i "trophy" -A 2 -B 2

Length of output: 96379

client/apps/torii-launcher/webpack.main.config.ts (2)

1-4: LGTM! Well-structured imports.

Good separation of concerns by keeping rules and plugins in separate files.

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards


17-19: Review resolve.extensions for main process.

Since this is the main process configuration (not renderer), including '.css' in the extensions array is unusual as the main process typically doesn't handle CSS files. Consider removing it unless there's a specific need.

✅ Verification successful

Remove '.css' from main process webpack resolve.extensions

The main process code doesn't import any CSS files and handles only Node.js/Electron operations. The '.css' extension should be removed from webpack.main.config.ts as it's unnecessary for the main process.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if any CSS imports exist in the main process code
rg -t ts -t js --glob '!*.test.*' --glob '!*.spec.*' -l '\.css' ./src/

Length of output: 119


Script:

#!/bin/bash
# First, let's understand the project structure
fd -t f "main" client/apps/torii-launcher/

# Then check for CSS imports in the main process files
rg -t ts -t js --glob '!*.test.*' --glob '!*.spec.*' -l '\.css' client/apps/torii-launcher/

Length of output: 417


Script:

#!/bin/bash
# Let's examine the main process entry point
cat client/apps/torii-launcher/src/main.ts

Length of output: 3530

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

client/apps/torii-launcher/src/types.ts (1)

1-4: LGTM!

The IpcMethod enum is well-defined and improves the clarity of IPC communication.

client/apps/torii-launcher/forge.config.ts (1)

1-67: Verify file usage

The pipeline reports this file as unused. Please verify that this configuration file is properly referenced in your build scripts.

🧰 Tools
🪛 GitHub Actions: knip

[warning] Unused file detected

Comment on lines +5 to +12
use: {
loader: "babel-loader",
options: {
exclude: /node_modules/,
presets: ["@babel/preset-react"],
},
},
},
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

Restructure the babel-loader configuration for better maintainability and performance.

The current configuration has several areas for improvement:

  1. The exclude option should be at the root level
  2. Missing include option to restrict transpilation scope
  3. Missing environment-specific optimizations
  4. Could benefit from additional presets
     use: {
       loader: "babel-loader",
       options: {
-        exclude: /node_modules/,
         presets: ["@babel/preset-react"],
+        presets: [
+          "@babel/preset-react",
+          ["@babel/preset-env", {
+            targets: {
+              electron: process.versions.electron
+            }
+          }]
+        ],
+        cacheDirectory: true
       },
     },
+    exclude: /node_modules/,
+    include: path.resolve(__dirname, "src"),

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

Comment on lines +6 to +9
rules.push({
test: /\.css$/,
use: [{ loader: "style-loader" }, { loader: "css-loader" }],
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate CSS rule.

This CSS rule conflicts with the more specific rule defined later (lines 29-33). The later rule is more comprehensive as it includes path restrictions and PostCSS support.

-rules.push({
-  test: /\.css$/,
-  use: [{ loader: "style-loader" }, { loader: "css-loader" }],
-});
🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

Comment on lines +36 to +41
plugins.push(
new HtmlWebpackPlugin({
template: path.resolve(__dirname, "app/src/index.html"),
filename: "index.html",
})
);
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

⚠️ Potential issue

Fix incorrect template path in HtmlWebpackPlugin configuration

The template path is incorrect. The index.html file exists at the root of the torii-launcher directory, not in app/src.

plugins.push(
  new HtmlWebpackPlugin({
-   template: path.resolve(__dirname, "app/src/index.html"),
+   template: path.resolve(__dirname, "index.html"),
    filename: "index.html",
  })
);
🔗 Analysis chain

Add missing HtmlWebpackPlugin import.

The HtmlWebpackPlugin is used but not imported.

+import HtmlWebpackPlugin from "html-webpack-plugin";

Verify template path consistency.

The template path uses app/src while other rules use src. This might indicate an inconsistent directory structure.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the template file exists and verify directory structure
fd --type f "index.html" --full-path ./client/apps/torii-launcher

Length of output: 105

🧰 Tools
🪛 GitHub Actions: lint

[warning] Code formatting does not match Prettier standards

Comment on lines 7 to 11
events_chunk_size = 1024 # Size of events page to fetch
blocks_chunk_size = 1024 # Blocks to process before DB commit
pending = true # Enable indexing pending blocks
polling_interval = 500 # Polling interval in milliseconds
max_concurrent_tasks = 100 # Maximum concurrent indexing tasks
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

Consider increasing blocks_chunk_size to match other configurations

The current value of blocks_chunk_size = 1024 is significantly lower than the value of 10240 used in other environment configurations (mainnet, sepolia, local). Other parameters like events_chunk_size, polling_interval, and max_concurrent_tasks align with the established patterns across the codebase.

🔗 Analysis chain

Review performance parameters for resource constraints.

The current configuration sets aggressive values that might impact system resources:

  • Large chunk sizes (1024 for both events and blocks)
  • High number of concurrent tasks (100)
  • Frequent polling (500ms)

Consider documenting the rationale for these values and their impact on system resources.

Let's check for any performance-related discussions or issues:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for performance-related discussions
gh issue list --search "performance Torii" --json number,title,body

Length of output: 72


Script:

#!/bin/bash
# Search for documentation and similar config files
echo "=== Searching for documentation ==="
fd -e md -e txt | xargs rg -i "events_chunk|blocks_chunk|concurrent_tasks|polling_interval" -C 2

echo -e "\n=== Searching for similar TOML configs ==="
fd -e toml | xargs rg "events_chunk|blocks_chunk|concurrent_tasks|polling_interval" -C 2

echo -e "\n=== Checking file history ==="
git log -p client/apps/torii-launcher/public/torii.toml

Length of output: 3717

Comment on lines 83 to 112
while (true) {
fs.mkdirSync(dbPath, { recursive: true });
console.log("Launching torii");
child = spawn(`${dojoPath}/bin/torii`, [
"--world",
"0x6a9e4c6f0799160ea8ddc43ff982a5f83d7f633e9732ce42701de1288ff705f",
"--http.cors_origins",
"*",
"--config",
toriiTomlPath,
"--db-dir",
dbPath,
]);

child.stdout.on("data", (data: any) => {
console.log(`stdout: ${data}`);
});

child.stderr.on("data", (data: any) => {
console.error(`stderr: ${data}`);
});

while (child.exitCode === null) {
console.log("Torii is still running");
await timeout(10000);
}
child.removeAllListeners();

console.log(`child process exited with code ${child.exitCode}`);
}
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

Implement a retry mechanism with backoff strategy instead of infinite loop.

The infinite while (true) loop may cause rapid restarts if the Torii process fails immediately, potentially leading to resource exhaustion. Consider implementing a retry mechanism with a maximum number of retries or an exponential backoff strategy to handle failures gracefully.

Comment on lines +83 to +88
# Create the iconset
iconutil -c icns $output_iconset_name

# Delete the temp folder
rm -R $output_iconset_name

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

Add error handling for ICNS creation.

  1. Add error handling for the iconutil command.
  2. Quote the cleanup command.
  3. Check for iconutil availability.

Apply this diff:

+# Check for iconutil
+if ! command -v iconutil >/dev/null 2>&1; then
+    echo "ERROR: This script requires the 'iconutil' command (Mac OS only)."
+    rm -rf "${output_iconset_name}"
+    exit 1
+fi
+
 # Create the iconset
-iconutil -c icns $output_iconset_name
+if ! iconutil -c icns "${output_iconset_name}"; then
+    echo "ERROR: Failed to create ICNS file."
+    rm -rf "${output_iconset_name}"
+    exit 1
+fi
 
 # Delete the temp folder
-rm -R $output_iconset_name
+rm -rf "${output_iconset_name}"
📝 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
# Create the iconset
iconutil -c icns $output_iconset_name
# Delete the temp folder
rm -R $output_iconset_name
# Check for iconutil
if ! command -v iconutil >/dev/null 2>&1; then
echo "ERROR: This script requires the 'iconutil' command (Mac OS only)."
rm -rf "${output_iconset_name}"
exit 1
fi
# Create the iconset
if ! iconutil -c icns "${output_iconset_name}"; then
echo "ERROR: Failed to create ICNS file."
rm -rf "${output_iconset_name}"
exit 1
fi
# Delete the temp folder
rm -rf "${output_iconset_name}"

Comment on lines +53 to +64
icon_orginal_name="icon"
icon_name=$icon_orginal_name
icon_extension="icns"

# If dest path exists, add suffix number
# to avoid the new icon file to replace an existing file
icon_index=2
while [ -f "${icon_name}.${icon_extension}" ]
do
icon_name="${icon_orginal_name}_${icon_index}"
icon_index=$((icon_index + 1))
done
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

Improve output file handling.

  1. Variables should be quoted to handle spaces correctly.
  2. Consider making the base icon name configurable.
  3. Add write permission check for the output directory.

Apply this diff:

-icon_orginal_name="icon"
+# Allow custom base name via environment variable
+icon_original_name="${ICON_BASE_NAME:-icon}"
+icon_name="${icon_original_name}"
+icon_extension="icns"
 
-icon_name=$icon_orginal_name
-icon_extension="icns"
+# Check if current directory is writable
+if [ ! -w "." ]; then
+    echo "ERROR: Current directory is not writable."
+    exit 1
+fi
 
 # If dest path exists, add suffix number
 # to avoid the new icon file to replace an existing file
 icon_index=2
-while [ -f "${icon_name}.${icon_extension}" ]
+while [ -f "${icon_name}.${icon_extension}" ]
 do
-   icon_name="${icon_orginal_name}_${icon_index}"
+   icon_name="${icon_original_name}_${icon_index}"
    icon_index=$((icon_index + 1))
 done
📝 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
icon_orginal_name="icon"
icon_name=$icon_orginal_name
icon_extension="icns"
# If dest path exists, add suffix number
# to avoid the new icon file to replace an existing file
icon_index=2
while [ -f "${icon_name}.${icon_extension}" ]
do
icon_name="${icon_orginal_name}_${icon_index}"
icon_index=$((icon_index + 1))
done
# Allow custom base name via environment variable
icon_original_name="${ICON_BASE_NAME:-icon}"
icon_name="${icon_original_name}"
icon_extension="icns"
# Check if current directory is writable
if [ ! -w "." ]; then
echo "ERROR: Current directory is not writable."
exit 1
fi
# If dest path exists, add suffix number
# to avoid the new icon file to replace an existing file
icon_index=2
while [ -f "${icon_name}.${icon_extension}" ]
do
icon_name="${icon_original_name}_${icon_index}"
icon_index=$((icon_index + 1))
done

Comment on lines +32 to +51
# if empty image_path, display error message and usage
if [ -z "$image_path" ]
then
echo "ERROR: '-i' parameter cannot be empty."
usageFunction
fi

# if extension not .png, display error message
if [[ ! $image_path =~ .png$ ]]
then
echo "ERROR: image path need to end with .png"
exit 1
fi

# if image path file doesn't exists, display error message
if [ ! -f "$image_path" ]
then
echo "ERROR: image path doesn't seems to exists on your disk."
exit 1
fi
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

Enhance input validation robustness.

  1. The PNG extension check is case-sensitive and might reject valid files.
  2. File permissions should be checked.
  3. Error messages should include the actual path.

Apply this diff:

 # if empty image_path, display error message and usage
 if [ -z "$image_path" ]
 then
-   echo "ERROR: '-i' parameter cannot be empty."
+   echo "ERROR: Image path parameter (-i) cannot be empty."
    usageFunction
 fi
 
 # if extension not .png, display error message
-if [[ ! $image_path =~ .png$ ]]
+if [[ ! $image_path =~ .[pP][nN][gG]$ ]]
 then
-    echo "ERROR: image path need to end with .png"
+    echo "ERROR: Image path '$image_path' must end with .png"
     exit 1
 fi
 
-# if image path file doesn't exists, display error message
-if [ ! -f "$image_path" ]
+# Check if file exists and is readable
+if [ ! -f "$image_path" ] || [ ! -r "$image_path" ]
 then
-    echo "ERROR: image path doesn't seems to exists on your disk."
+    echo "ERROR: Image '$image_path' doesn't exist or is not readable."
     exit 1
 fi
📝 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
# if empty image_path, display error message and usage
if [ -z "$image_path" ]
then
echo "ERROR: '-i' parameter cannot be empty."
usageFunction
fi
# if extension not .png, display error message
if [[ ! $image_path =~ .png$ ]]
then
echo "ERROR: image path need to end with .png"
exit 1
fi
# if image path file doesn't exists, display error message
if [ ! -f "$image_path" ]
then
echo "ERROR: image path doesn't seems to exists on your disk."
exit 1
fi
# if empty image_path, display error message and usage
if [ -z "$image_path" ]
then
echo "ERROR: Image path parameter (-i) cannot be empty."
usageFunction
fi
# if extension not .png, display error message
if [[ ! $image_path =~ .[pP][nN][gG]$ ]]
then
echo "ERROR: Image path '$image_path' must end with .png"
exit 1
fi
# Check if file exists and is readable
if [ ! -f "$image_path" ] || [ ! -r "$image_path" ]
then
echo "ERROR: Image '$image_path' doesn't exist or is not readable."
exit 1
fi

Comment on lines +1 to +16
#!/usr/bin/env bash

# PNG to ICNS v1.0 - https://github.com/BenSouchet/png-to-icns
# Copyright(C) 2022 Ben Souchet | MIT License

usageFunction()
{
echo "PNG to ICNS v1.0 - Easily convert your image to Mac OS icon!"
echo ""
echo "Usage: $0 -i <path of the .png image>"
echo -e "\t-i Relative path of the image (.png)"
echo -e "\t-h Display this usage message"

# Exit / Stop the script
exit 1
}
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

Fix script naming and usage function exit code.

  1. The script filename png_to_isns.sh contains a typo - it should be png_to_icns.sh to match the ICNS format.
  2. The usage function should exit with status 0 when displaying help, as this isn't an error condition.

Apply this diff:

-#!/usr/bin/env bash
+#!/usr/bin/env bash
+set -euo pipefail
 
 # PNG to ICNS v1.0 - https://github.com/BenSouchet/png-to-icns
 # Copyright(C) 2022 Ben Souchet | MIT License
 
 usageFunction()
 {
    echo "PNG to ICNS v1.0 - Easily convert your image to Mac OS icon!"
    echo ""
    echo "Usage: $0 -i <path of the .png image>"
    echo -e "\t-i Relative path of the image (.png)"
    echo -e "\t-h Display this usage message"
 
    # Exit / Stop the script
-   exit 1
+   exit 0
 }
📝 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
#!/usr/bin/env bash
# PNG to ICNS v1.0 - https://github.com/BenSouchet/png-to-icns
# Copyright(C) 2022 Ben Souchet | MIT License
usageFunction()
{
echo "PNG to ICNS v1.0 - Easily convert your image to Mac OS icon!"
echo ""
echo "Usage: $0 -i <path of the .png image>"
echo -e "\t-i Relative path of the image (.png)"
echo -e "\t-h Display this usage message"
# Exit / Stop the script
exit 1
}
#!/usr/bin/env bash
set -euo pipefail
# PNG to ICNS v1.0 - https://github.com/BenSouchet/png-to-icns
# Copyright(C) 2022 Ben Souchet | MIT License
usageFunction()
{
echo "PNG to ICNS v1.0 - Easily convert your image to Mac OS icon!"
echo ""
echo "Usage: $0 -i <path of the .png image>"
echo -e "\t-i Relative path of the image (.png)"
echo -e "\t-h Display this usage message"
# Exit / Stop the script
exit 0
}

Comment on lines +66 to +82
output_iconset_name="${icon_name}.iconset"

# Create the temp directory to save the icon sizes
mkdir $output_iconset_name

# Create the icon sizes
sips -z 16 16 -s format png "$image_path" --out "${output_iconset_name}/icon_16x16.png" > /dev/null
sips -z 32 32 -s format png "$image_path" --out "${output_iconset_name}/[email protected]" > /dev/null
sips -z 32 32 -s format png "$image_path" --out "${output_iconset_name}/icon_32x32.png" > /dev/null
sips -z 64 64 -s format png "$image_path" --out "${output_iconset_name}/[email protected]" > /dev/null
sips -z 128 128 -s format png "$image_path" --out "${output_iconset_name}/icon_128x128.png" > /dev/null
sips -z 256 256 -s format png "$image_path" --out "${output_iconset_name}/[email protected]" > /dev/null
sips -z 256 256 -s format png "$image_path" --out "${output_iconset_name}/icon_256x256.png" > /dev/null
sips -z 512 512 -s format png "$image_path" --out "${output_iconset_name}/[email protected]" > /dev/null
sips -z 512 512 -s format png "$image_path" --out "${output_iconset_name}/icon_512x512.png" > /dev/null
sips -z 1024 1024 -s format png "$image_path" --out "${output_iconset_name}/[email protected]" > /dev/null

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

Add error handling and platform checks.

  1. Add error handling for sips commands.
  2. Quote the mkdir command and handle errors.
  3. Check for required commands before proceeding.

Apply this diff:

+# Check for required commands
+if ! command -v sips >/dev/null 2>&1; then
+    echo "ERROR: This script requires the 'sips' command (Mac OS only)."
+    exit 1
+fi
+
 output_iconset_name="${icon_name}.iconset"
 
 # Create the temp directory to save the icon sizes
-mkdir $output_iconset_name
+if ! mkdir "${output_iconset_name}"; then
+    echo "ERROR: Failed to create temporary directory '${output_iconset_name}'."
+    exit 1
+fi
 
 # Create the icon sizes
-sips -z 16 16     -s format png "$image_path" --out "${output_iconset_name}/icon_16x16.png" > /dev/null
+generate_icon_size() {
+    local size="$1"
+    local output="$2"
+    if ! sips -z "$size" "$size" -s format png "$image_path" --out "$output" > /dev/null 2>&1; then
+        echo "ERROR: Failed to generate ${size}x${size} icon."
+        rm -rf "${output_iconset_name}"
+        exit 1
+    fi
+}
+
+generate_icon_size 16 "${output_iconset_name}/icon_16x16.png"
📝 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
output_iconset_name="${icon_name}.iconset"
# Create the temp directory to save the icon sizes
mkdir $output_iconset_name
# Create the icon sizes
sips -z 16 16 -s format png "$image_path" --out "${output_iconset_name}/icon_16x16.png" > /dev/null
sips -z 32 32 -s format png "$image_path" --out "${output_iconset_name}/[email protected]" > /dev/null
sips -z 32 32 -s format png "$image_path" --out "${output_iconset_name}/icon_32x32.png" > /dev/null
sips -z 64 64 -s format png "$image_path" --out "${output_iconset_name}/[email protected]" > /dev/null
sips -z 128 128 -s format png "$image_path" --out "${output_iconset_name}/icon_128x128.png" > /dev/null
sips -z 256 256 -s format png "$image_path" --out "${output_iconset_name}/[email protected]" > /dev/null
sips -z 256 256 -s format png "$image_path" --out "${output_iconset_name}/icon_256x256.png" > /dev/null
sips -z 512 512 -s format png "$image_path" --out "${output_iconset_name}/[email protected]" > /dev/null
sips -z 512 512 -s format png "$image_path" --out "${output_iconset_name}/icon_512x512.png" > /dev/null
sips -z 1024 1024 -s format png "$image_path" --out "${output_iconset_name}/[email protected]" > /dev/null
# Check for required commands
if ! command -v sips >/dev/null 2>&1; then
echo "ERROR: This script requires the 'sips' command (Mac OS only)."
exit 1
fi
output_iconset_name="${icon_name}.iconset"
# Create the temp directory to save the icon sizes
if ! mkdir "${output_iconset_name}"; then
echo "ERROR: Failed to create temporary directory '${output_iconset_name}'."
exit 1
fi
# Create the icon sizes
generate_icon_size() {
local size="$1"
local output="$2"
if ! sips -z "$size" "$size" -s format png "$image_path" --out "$output" > /dev/null 2>&1; then
echo "ERROR: Failed to generate ${size}x${size} icon."
rm -rf "${output_iconset_name}"
exit 1
fi
}
generate_icon_size 16 "${output_iconset_name}/icon_16x16.png"
generate_icon_size 32 "${output_iconset_name}/[email protected]"
generate_icon_size 32 "${output_iconset_name}/icon_32x32.png"
generate_icon_size 64 "${output_iconset_name}/[email protected]"
generate_icon_size 128 "${output_iconset_name}/icon_128x128.png"
generate_icon_size 256 "${output_iconset_name}/[email protected]"
generate_icon_size 256 "${output_iconset_name}/icon_256x256.png"
generate_icon_size 512 "${output_iconset_name}/[email protected]"
generate_icon_size 512 "${output_iconset_name}/icon_512x512.png"
generate_icon_size 1024 "${output_iconset_name}/[email protected]"

@aymericdelab
Copy link
Collaborator

aymericdelab commented Jan 24, 2025

can you centralize all in the public folder and remove the 2 asset folders ?

image

image

@aymericdelab
Copy link
Collaborator

can you use kebab case for all files ?

- Add notification
- Add settings to change RPC
- Adds different db directories for all configurations
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

♻️ Duplicate comments (5)
client/apps/torii-launcher/src/main.ts (3)

139-142: ⚠️ Potential issue

Use Node's fs.rmSync to safely delete directories instead of running rm -rf.

Invoking shell commands like rm -rf can be dangerous and is platform-dependent. A safer, cross-platform approach is to use Node's fs.rmSync.

-ipcMain.on(IpcMethod.ResetDatabase, async (event, arg) => {
-  try {
-    spawnSync("rm", ["-rf", getDbPath()]);
-    killTorii();
+ipcMain.on(IpcMethod.ResetDatabase, async (event, arg) => {
+  try {
+    fs.rmSync(getDbPath(), { recursive: true, force: true });
+    killTorii();

172-228: ⚠️ Potential issue

Implement a retry mechanism with backoff strategy instead of an infinite loop.

Continuously re-running Torii in a while (true) loop may cause resource exhaustion if it fails on startup. Implement a controlled retry mechanism with a maximum retry count or exponential backoff to avoid rapidly spawning processes in case of repeated failures.

   // existing code omitted for brevity
-  while (true) {
+  let retryCount = 0;
+  const maxRetries = 5;
+  while (retryCount < maxRetries) {
      // ...
      if (child.exitCode !== 0 && child.exitCode !== 137) {
        errorLog(`Torii exited with code ${child.exitCode}`);
        sendNotification({ type: "Error", message: `Torii exited with code ${child.exitCode}` });
      }
      // ...
+     retryCount++;
+     if (retryCount >= maxRetries) {
+       errorLog(`Exceeded max retries (${maxRetries}). Stopping attempts.`);
+       break;
+     }
   }

234-236: ⚠️ Potential issue

Avoid executing scripts directly from the internet.

Executing shell commands from an unverified source can introduce security risks. Consider downloading the script securely, verifying its integrity, or relying on a package manager to install dependencies.

- spawnSync("sh", ["-c", `curl -L https://install.dojoengine.org | bash && dojoup -v ${TORII_VERSION}`]);
+ // TODO: Implement secure installation with checksum verification for Torii
+ // e.g., download the script from a trusted URL, verify integrity, then execute.
client/apps/torii-launcher/src/frontend/components/settings.tsx (1)

45-50: ⚠️ Potential issue

Add error handling for RPC updates.

The setNewRpc callback lacks error handling. Since this involves IPC communication with the main process, it should handle potential failures.

-        onClick={() => {
-          setNewRpc(selectedRpc);
-        }}
+        onClick={async () => {
+          try {
+            await setNewRpc(selectedRpc);
+          } catch (error) {
+            console.error('Failed to update RPC:', error);
+            // Handle error appropriately, e.g., show error toast
+          }
+        }}
client/apps/torii-launcher/forge.config.ts (1)

10-13: ⚠️ Potential issue

Enable ASAR packaging for security

ASAR packaging is currently disabled. This could expose your source code and assets.

 packagerConfig: {
-  asar: false,
+  asar: true,
   icon: "public/icon",
 },
🧹 Nitpick comments (16)
client/apps/torii-launcher/tailwind.config.js (3)

1-3: Consider optimizing content paths for better build performance.

The current content path "./src/**/*.{html,tsx}" is quite broad. Consider being more specific by targeting only the directories that contain your components and templates.

-  content: ["./src/**/*.{html,tsx}"],
+  content: [
+    "./src/components/**/*.tsx",
+    "./src/pages/**/*.tsx",
+    "./src/templates/**/*.{html,tsx}"
+  ],

6-11: Enhance color system with semantic names and variations.

Consider:

  1. Using semantic color names that reflect their purpose
  2. Adding color variations for different states (hover, active, etc.)
       colors: {
-        gold: "#F6C297",
-        red: "#FC4C4C",
-        grey: "#121212",
-        brown: "#14100D",
+        primary: {
+          DEFAULT: "#F6C297",  // Current gold
+          hover: "#F7D1B1",
+          active: "#E5B186"
+        },
+        error: {
+          DEFAULT: "#FC4C4C",  // Current red
+          light: "#FD6666",
+          dark: "#E43939"
+        },
+        background: {
+          primary: "#14100D",  // Current brown
+          secondary: "#121212" // Current grey
+        }

12-26: Improve animation timing for better user experience.

Consider:

  1. Using CSS custom properties for animation durations to maintain consistency
  2. Using cubic-bezier for more natural motion
       keyframes: {
         "fade-in": {
           "0%": { opacity: "0" },
           "100%": { opacity: "1" },
         },
         "scale-in": {
           "0%": { opacity: "0", transform: "scale(0.95)" },
           "100%": { opacity: "1", transform: "scale(1)" },
         },
       },
       animation: {
-        "fade-in": "fade-in 300ms ease-in-out",
-        "scale-in": "scale-in 300ms ease-in-out",
+        "fade-in": "fade-in var(--animation-duration, 300ms) cubic-bezier(0.4, 0, 0.2, 1)",
+        "scale-in": "scale-in var(--animation-duration, 300ms) cubic-bezier(0.4, 0, 0.2, 1)",
       },
client/apps/torii-launcher/src/frontend/notifications/notifications-handler.tsx (1)

7-15: Consider removing or refining console logs in production builds.

Logging notifications is helpful for debugging, but printing sensitive notifications or large volumes of them could be undesirable in production. You might conditionally enable logs in development mode only.

     const sub = window.electronAPI.onMessage(IpcMethod.Notification, (notification: Notification) => {
-      console.log("Notification received: " + JSON.stringify(notification));
+      if (process.env.NODE_ENV !== "production") {
+        console.log("Notification received: " + JSON.stringify(notification));
+      }
       toast(
         <div className="text-xs text-gold m-auto text-center">
client/apps/torii-launcher/src/renderer.tsx (2)

29-30: Remove or enhance console logging

The console.log statement appears to be a development artifact. Consider removing it or replacing it with proper application logging.

-// Add this to the end of the existing file
-console.log("renderer");

37-38: Consider using a dedicated root element

Mounting directly to document.body is not a React best practice. Consider creating a dedicated root element.

-const root = createRoot(document.body);
+const rootElement = document.getElementById('root') || document.createElement('div');
+rootElement.id = 'root';
+document.body.appendChild(rootElement);
+const root = createRoot(rootElement);
client/apps/torii-launcher/src/frontend/components/warning.tsx (1)

43-48: Enhance accessibility

The warning dialog should implement proper ARIA attributes for better accessibility.

     <div
       className={`flex flex-col justify-start items-center max-h-full h-full
 		transition-all duration-300 ease-in-out p-4
-		${isVisible ? "scale-100 opacity-100" : "scale-95 opacity-0"}`}
+		${isVisible ? "scale-100 opacity-100" : "scale-95 opacity-0"}`}
+      role="alertdialog"
+      aria-modal="true"
+      aria-labelledby="warning-title"
+      aria-describedby="warning-message"
     >
client/apps/torii-launcher/src/frontend/utils/button.tsx (2)

3-13: Consider extracting variant and size types

The variant and size types should be extracted for reusability and type safety.

+export type ButtonVariant = "primary" | "secondary" | "success" | "red" | "danger" | "default" | "outline" | "opaque";
+export type ButtonSize = "xs" | "sm" | "md";
+
 type ButtonProps = {
   onClick?: (e: React.MouseEvent<HTMLButtonElement>) => void;
   children: React.ReactNode;
   className?: string;
   isPulsing?: boolean;
   disabled?: boolean;
-  variant?: "primary" | "secondary" | "success" | "red" | "danger" | "default" | "outline" | "opaque";
+  variant?: ButtonVariant;
   isLoading?: boolean;
   withoutSound?: boolean;
-  size?: "xs" | "sm" | "md";
+  size?: ButtonSize;
 } & React.ComponentPropsWithRef<"button">;

15-31: Consider using CSS modules or styled-components

The current style organization using a STYLES object could be improved using CSS modules or styled-components for better maintainability.

Consider migrating to CSS modules or styled-components to:

  • Improve style encapsulation
  • Enable better IDE support
  • Make styles more maintainable
client/apps/torii-launcher/src/frontend/components/syncing-state.tsx (2)

5-5: Consider making SYNC_INTERVAL configurable

The sync interval is hardcoded. Consider making it configurable through environment variables or settings.

-const SYNC_INTERVAL = 4000;
+const SYNC_INTERVAL = process.env.SYNC_INTERVAL ? parseInt(process.env.SYNC_INTERVAL, 10) : 4000;

58-64: Optimize interval management

The component creates two separate intervals that could be combined for better performance.

Consider combining both interval effects into a single effect that handles both chain and Torii block updates, reducing the number of active intervals.

Also applies to: 66-79

client/apps/torii-launcher/src/frontend/components/settings.tsx (1)

66-78: Consolidate duplicate event handling logic.

The same RPC selection logic is duplicated in both the onClick and onChange handlers. This violates the DRY principle and could lead to maintenance issues.

+  const handleRpcSelection = () => {
+    setSelectedRpc(Rpc[label as keyof typeof Rpc]);
+  };
+
   return (
     <div
-      onClick={() => {
-        setSelectedRpc(Rpc[label as keyof typeof Rpc]);
-      }}
+      onClick={handleRpcSelection}
       className="flex flex-row justify-center items-center gap-2 rounded-md"
     >
       <input
         type="radio"
         className="transition-all duration-300 ease-in-out w-3 h-3 bg-grey/40 appearance-none inline-block checked:bg-gold/60 checked:border-0 rounded-full checked:border-2 checked:border-gold/20"
         checked={checked}
-        onChange={() => {
-          setSelectedRpc(Rpc[label as keyof typeof Rpc]);
-        }}
+        onChange={handleRpcSelection}
       />
client/apps/torii-launcher/src/frontend/launcher.tsx (2)

21-30: Consolidate related state with useReducer.

Multiple pieces of state (reset, showWarning, showSettings, currentRpc, newRpc) are closely related and could be managed more effectively using useReducer.

+type LauncherState = {
+  reset: boolean;
+  showWarning: { method: IpcMethod; alertMessage: string; } | null;
+  showSettings: boolean;
+  currentRpc: CurrentRpc | null;
+  newRpc: CurrentRpc | null;
+};
+
+type LauncherAction =
+  | { type: 'TOGGLE_RESET' }
+  | { type: 'SET_WARNING'; payload: LauncherState['showWarning'] }
+  | { type: 'TOGGLE_SETTINGS' }
+  | { type: 'SET_RPC'; payload: CurrentRpc }
+  | { type: 'SET_NEW_RPC'; payload: CurrentRpc };
+
+const launcherReducer = (state: LauncherState, action: LauncherAction): LauncherState => {
+  switch (action.type) {
+    case 'TOGGLE_RESET':
+      return { ...state, reset: !state.reset };
+    case 'SET_WARNING':
+      return { ...state, showWarning: action.payload };
+    case 'TOGGLE_SETTINGS':
+      return { ...state, showSettings: !state.showSettings };
+    case 'SET_RPC':
+      return { ...state, currentRpc: action.payload };
+    case 'SET_NEW_RPC':
+      return { ...state, newRpc: action.payload };
+    default:
+      return state;
+  }
+};
+
 export const Launcher = () => {
-  const [reset, setReset] = useState(false);
-  const [showWarning, setShowWarning] = useState<{
-    method: IpcMethod;
-    alertMessage: string;
-  } | null>(null);
-  const [showSettings, setShowSettings] = useState(false);
-  const [currentRpc, setCurrentRpc] = useState<CurrentRpc | null>(null);
-  const [newRpc, setNewRpc] = useState<CurrentRpc | null>(null);
+  const [state, dispatch] = useReducer(launcherReducer, {
+    reset: false,
+    showWarning: null,
+    showSettings: false,
+    currentRpc: null,
+    newRpc: null,
+  });

1-8: Simplify background image handling and use consistent naming.

The background image handling can be simplified, and the import naming should follow a consistent pattern.

-import cover1 from "@public/covers/01.png";
-import cover02 from "@public/covers/02.png";
-import cover03 from "@public/covers/03.png";
-import cover04 from "@public/covers/04.png";
-import cover05 from "@public/covers/05.png";
-import cover06 from "@public/covers/06.png";
-import cover07 from "@public/covers/07.png";
+import cover01 from "@public/covers/01.png";
+import cover02 from "@public/covers/02.png";
+import cover03 from "@public/covers/03.png";
+import cover04 from "@public/covers/04.png";
+import cover05 from "@public/covers/05.png";
+import cover06 from "@public/covers/06.png";
+import cover07 from "@public/covers/07.png";

+const coverImages = [
+  cover01,
+  cover02,
+  cover03,
+  cover04,
+  cover05,
+  cover06,
+  cover07,
+];

 export const getRandomBackgroundImage = () => {
   const timestamp = Math.floor(Date.now() / 1000);
-  const imageNumber = (timestamp % 7) + 1;
+  const index = timestamp % coverImages.length;
+  return coverImages[index];
-
-  switch (imageNumber) {
-    case 1:
-      return cover1;
-    case 2:
-      return cover02;
-    case 3:
-      return cover03;
-    case 4:
-      return cover04;
-    case 5:
-      return cover05;
-    case 6:
-      return cover06;
-    case 7:
-      return cover07;
-    default:
-      return cover1;
-  }
 };

Also applies to: 131-153

client/apps/torii-launcher/src/frontend/app.tsx (1)

5-13: Add error boundaries and React.Suspense.

Consider wrapping components with error boundaries and React.Suspense for better error handling and code splitting.

+import { ErrorBoundary } from 'react-error-boundary';
+import { Suspense } from 'react';
+
 export const App = () => {
   return (
     <>
       <Toaster />
-      <NotificationsHandler />
-      <Launcher />
+      <ErrorBoundary FallbackComponent={ErrorFallback}>
+        <Suspense fallback={<div>Loading...</div>}>
+          <NotificationsHandler />
+          <Launcher />
+        </Suspense>
+      </ErrorBoundary>
     </>
   );
 };
client/apps/torii-launcher/src/types.ts (1)

38-42: Enhance RPC type safety

The CurrentRpc type could benefit from additional validation and safety measures.

 export type CurrentRpc = {
   name: string;
   url: string;
   worldBlock: number;
+  isSecure?: boolean;
+  timeout?: number;
+  validate?: () => Promise<boolean>;
 };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb918e0 and 1744682.

⛔ Files ignored due to path filters (13)
  • client/apps/torii-launcher/package-lock.json is excluded by !**/package-lock.json
  • client/apps/torii-launcher/public/covers/01.png is excluded by !**/*.png
  • client/apps/torii-launcher/public/covers/02.png is excluded by !**/*.png
  • client/apps/torii-launcher/public/covers/03.png is excluded by !**/*.png
  • client/apps/torii-launcher/public/covers/04.png is excluded by !**/*.png
  • client/apps/torii-launcher/public/covers/05.png is excluded by !**/*.png
  • client/apps/torii-launcher/public/covers/06.png is excluded by !**/*.png
  • client/apps/torii-launcher/public/covers/07.png is excluded by !**/*.png
  • client/apps/torii-launcher/public/icons/refresh.svg is excluded by !**/*.svg
  • client/apps/torii-launcher/public/icons/settings.svg is excluded by !**/*.svg
  • client/apps/torii-launcher/public/icons/trashcan.svg is excluded by !**/*.svg
  • client/apps/torii-launcher/public/icons/x-mark.svg is excluded by !**/*.svg
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (23)
  • client/apps/game/src/ui/modules/loading-screen.tsx (1 hunks)
  • client/apps/torii-launcher/.gitignore (1 hunks)
  • client/apps/torii-launcher/forge.config.ts (1 hunks)
  • client/apps/torii-launcher/index.html (1 hunks)
  • client/apps/torii-launcher/package.json (1 hunks)
  • client/apps/torii-launcher/public/torii/torii.toml (1 hunks)
  • client/apps/torii-launcher/src/constants.ts (1 hunks)
  • client/apps/torii-launcher/src/frontend/app.tsx (1 hunks)
  • client/apps/torii-launcher/src/frontend/components/settings.tsx (1 hunks)
  • client/apps/torii-launcher/src/frontend/components/syncing-state.tsx (1 hunks)
  • client/apps/torii-launcher/src/frontend/components/warning.tsx (1 hunks)
  • client/apps/torii-launcher/src/frontend/launcher.tsx (1 hunks)
  • client/apps/torii-launcher/src/frontend/notifications/notifications-handler.tsx (1 hunks)
  • client/apps/torii-launcher/src/frontend/notifications/toaster.tsx (1 hunks)
  • client/apps/torii-launcher/src/frontend/utils/button.tsx (1 hunks)
  • client/apps/torii-launcher/src/index.css (1 hunks)
  • client/apps/torii-launcher/src/main.ts (1 hunks)
  • client/apps/torii-launcher/src/preload.ts (1 hunks)
  • client/apps/torii-launcher/src/renderer.tsx (1 hunks)
  • client/apps/torii-launcher/src/types.ts (1 hunks)
  • client/apps/torii-launcher/tailwind.config.js (1 hunks)
  • client/apps/torii-launcher/tsconfig.json (1 hunks)
  • client/apps/torii-launcher/vite.renderer.config.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • client/apps/torii-launcher/src/constants.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • client/apps/torii-launcher/src/index.css
  • client/apps/torii-launcher/.gitignore
  • client/apps/torii-launcher/package.json
  • client/apps/torii-launcher/index.html
  • client/apps/torii-launcher/tsconfig.json
🔇 Additional comments (11)
client/apps/game/src/ui/modules/loading-screen.tsx (2)

63-63: LGTM! Proper BigInt comparison in the guard clause.

The condition correctly uses BigInt comparison with 0n and maintains a clean early return pattern.


56-56: Verify semantic change in timestamp variable.

The rename from currentBlockTimestamp to nextBlockTimestamp suggests a change in what the timestamp represents. Please verify that this timestamp indeed represents the next block's timestamp rather than the current block's timestamp.

client/apps/torii-launcher/public/torii/torii.toml (3)

1-7: Review performance implications of indexing configuration.

The current configuration parameters might need adjustment based on system resources and production load:

  1. Consider adding documentation about the impact of events_chunk_size and blocks_chunk_size on memory usage
  2. The polling interval of 500ms might be too aggressive for production use
  3. 100 concurrent tasks could potentially overwhelm system resources

Consider implementing the following improvements:

  • Add configuration validation
  • Document recommended ranges for these values
  • Consider making these values environment-specific (dev/prod)
  • Add documentation for the empty contracts array usage

9-11: Document event configuration requirements and implications.

The events configuration needs more context and documentation:

  1. The raw = true setting might have performance implications
  2. The historical events list seems tightly coupled to specific trophy events

Consider these improvements:

  • Document the performance impact of raw event processing
  • Make the historical events list configurable through environment variables
  • Add validation for event names to prevent typos
  • Consider adding a comment explaining the expected event format

1-11: Verify configuration consistency across environments.

Let's ensure these configuration values are consistent with other environments and properly documented.

✅ Verification successful

Environment-specific configurations detected - no action needed

The differences in configuration values across environments appear intentional:

  • Different block chunk sizes tuned for each environment's performance needs
  • Season prefixes (s0_ vs s1_) properly separated by environment
  • Additional indexing features in specific environments (e.g., sepolia)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other TOML configuration files and their values
echo "Searching for other TOML configuration files..."
fd -e toml | while read -r file; do
  echo "=== $file ==="
  if [ -f "$file" ]; then
    # Look for indexing and events configurations
    echo "Indexing configuration:"
    grep -A 6 "^\[indexing\]" "$file" || echo "No indexing configuration found"
    echo "Events configuration:"
    grep -A 3 "^\[events\]" "$file" || echo "No events configuration found"
  fi
done

# Search for documentation about these configurations
echo -e "\nSearching for configuration documentation..."
rg -l "events_chunk_size|blocks_chunk_size|polling_interval|max_concurrent_tasks" \
  -g "*.{md,txt,rst}"

Length of output: 90890

client/apps/torii-launcher/src/frontend/notifications/toaster.tsx (1)

5-22: Simplify prop spreading and tailor your toast styles if needed.

Overall design looks good. Ensure your unwrapped props are necessary and consider customizing the toast appearance further for brand consistency.

client/apps/torii-launcher/src/renderer.tsx (1)

8-26: Security Warning: Review Node.js integration settings

The code comments discuss enabling nodeIntegration, which has significant security implications. Enabling this feature could expose the application to security vulnerabilities by allowing malicious websites to execute native system commands.

Let's check the main process configuration:

client/apps/torii-launcher/vite.renderer.config.ts (1)

1-13: LGTM!

The Vite configuration is well-structured and includes necessary plugins and aliases.

client/apps/torii-launcher/src/preload.ts (1)

11-16: Add memory leak prevention for message listeners

The onMessage callback cleanup might not be called if the component unmounts unexpectedly. Consider using a WeakMap to track active listeners.

Also, improve type safety by using generics:

-  onMessage: (message: IpcMethod, callback: (data: any) => void) => {
-    ipcRenderer.on(message, (_event: any, data: any) => {
+  onMessage: <T = unknown>(message: IpcMethod, callback: (data: T) => void) => {
+    const wrappedCallback = (_event: Electron.IpcRendererEvent, data: T) => {
       callback(data);
-    });
-    return () => ipcRenderer.removeListener(message, callback);
+    };
+    ipcRenderer.on(message, wrappedCallback);
+    return () => ipcRenderer.removeListener(message, wrappedCallback);
   },
client/apps/torii-launcher/src/types.ts (1)

31-35: Review security implications of localhost RPC

The localhost RPC configuration could pose security risks if not properly validated. Consider:

  1. Adding validation for the localhost URL
  2. Making the world block configurable instead of hardcoded to 0
  3. Adding documentation about security considerations
client/apps/torii-launcher/forge.config.ts (1)

15-23: Review S3 bucket security configuration

The S3 bucket is configured with public access. This could pose security risks if not properly configured with appropriate CORS and bucket policies.

✅ Verification successful

S3 bucket security configuration is appropriate

The bucket is correctly configured with proper security controls. The public: true setting is specifically required for the electron auto-updater functionality and doesn't compromise security. The bucket successfully blocks public directory listing while still allowing the auto-updater to access specific update files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the S3 bucket has appropriate security headers
curl -I "http://s3.us-east-1.amazonaws.com/eternum-torii-updates/"

Length of output: 680

Comment on lines +28 to +44
plugins: [
function ({ addUtilities }) {
addUtilities({
".noselect": {
"-webkit-touch-callout": "none",
"-webkit-user-select": "none",
"-khtml-user-select": "none",
"-moz-user-select": "none",
"-ms-user-select": "none",
"user-select": "none",
},
".border-gradient": {
borderImage: "linear-gradient(to right, transparent, #F3C99F, transparent) 1",
},
});
},
],
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

Improve border-gradient utility for better cross-browser support and maintainability.

The current border-gradient implementation might have inconsistencies across browsers. Consider:

  1. Using CSS custom properties for gradient colors
  2. Adding proper vendor prefixes
  3. Including fallback styles
       ".border-gradient": {
-        borderImage: "linear-gradient(to right, transparent, #F3C99F, transparent) 1",
+        position: "relative",
+        "&::before": {
+          content: '""',
+          position: "absolute",
+          inset: 0,
+          padding: "1px",
+          background: "linear-gradient(to right, transparent, var(--border-color, #F3C99F), transparent)",
+          "-webkit-mask":
+            "linear-gradient(#fff 0 0) content-box, linear-gradient(#fff 0 0)",
+          "-webkit-mask-composite": "xor",
+          "mask-composite": "exclude",
+          pointerEvents: "none"
+        }
       },
📝 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
plugins: [
function ({ addUtilities }) {
addUtilities({
".noselect": {
"-webkit-touch-callout": "none",
"-webkit-user-select": "none",
"-khtml-user-select": "none",
"-moz-user-select": "none",
"-ms-user-select": "none",
"user-select": "none",
},
".border-gradient": {
borderImage: "linear-gradient(to right, transparent, #F3C99F, transparent) 1",
},
});
},
],
plugins: [
function ({ addUtilities }) {
addUtilities({
".noselect": {
"-webkit-touch-callout": "none",
"-webkit-user-select": "none",
"-khtml-user-select": "none",
"-moz-user-select": "none",
"-ms-user-select": "none",
"user-select": "none",
},
".border-gradient": {
position: "relative",
"&::before": {
content: '""',
position: "absolute",
inset: 0,
padding: "1px",
background: "linear-gradient(to right, transparent, var(--border-color, #F3C99F), transparent)",
"-webkit-mask":
"linear-gradient(#fff 0 0) content-box, linear-gradient(#fff 0 0)",
"-webkit-mask-composite": "xor",
"mask-composite": "exclude",
pointerEvents: "none"
}
},
});
},
],

Comment on lines +62 to +68
window.electronAPI.sendMessage(showWarning.method);
if (showWarning.method === IpcMethod.ResetDatabase) {
console.log("setting reset to true");
setReset(true);
}
handleClose();
}}
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

Add error handling for IPC communication

The IPC message sending lacks error handling, which could lead to silent failures.

-            window.electronAPI.sendMessage(showWarning.method);
+            try {
+              await window.electronAPI.sendMessage(showWarning.method);
+            } catch (error) {
+              console.error('Failed to send IPC message:', error);
+              // Consider showing an error notification to the user
+            }
📝 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
window.electronAPI.sendMessage(showWarning.method);
if (showWarning.method === IpcMethod.ResetDatabase) {
console.log("setting reset to true");
setReset(true);
}
handleClose();
}}
try {
await window.electronAPI.sendMessage(showWarning.method);
if (showWarning.method === IpcMethod.ResetDatabase) {
console.log("setting reset to true");
setReset(true);
}
handleClose();
} catch (error) {
console.error('Failed to send IPC message:', error);
// Consider showing an error notification to the user
}

Comment on lines +102 to +108
const getChainCurrentBlock = async () => {
const provider = new RpcProvider({
nodeUrl: "https://api.cartridge.gg/x/starknet/mainnet",
});
const block = await provider.getBlockNumber();
return block;
};
Copy link
Contributor

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 RPC provider

The chain block fetching lacks error handling and could fail silently.

 const getChainCurrentBlock = async () => {
+  try {
     const provider = new RpcProvider({
       nodeUrl: "https://api.cartridge.gg/x/starknet/mainnet",
     });
     const block = await provider.getBlockNumber();
     return block;
+  } catch (error) {
+    console.error("Failed to fetch chain block:", error);
+    throw error;
+  }
 };
📝 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
const getChainCurrentBlock = async () => {
const provider = new RpcProvider({
nodeUrl: "https://api.cartridge.gg/x/starknet/mainnet",
});
const block = await provider.getBlockNumber();
return block;
};
const getChainCurrentBlock = async () => {
try {
const provider = new RpcProvider({
nodeUrl: "https://api.cartridge.gg/x/starknet/mainnet",
});
const block = await provider.getBlockNumber();
return block;
} catch (error) {
console.error("Failed to fetch chain block:", error);
throw error;
}
};

Comment on lines +110 to +122
const getToriiCurrentBlock = async () => {
const sqlQuery = "SELECT head FROM contracts WHERE contract_type = 'WORLD' LIMIT 1;";
const params = new URLSearchParams([["query", sqlQuery]]).toString();

const toriiUrl = `http://localhost:8080/sql?${params}`;

const response = await fetch(toriiUrl, {
method: "GET",
});
const data = await response.json();

return data[0].head;
};
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

Add retry mechanism for Torii server requests

The Torii block fetching should implement a retry mechanism for better reliability.

 const getToriiCurrentBlock = async () => {
+  const maxRetries = 3;
+  let retryCount = 0;
+
   const sqlQuery = "SELECT head FROM contracts WHERE contract_type = 'WORLD' LIMIT 1;";
   const params = new URLSearchParams([["query", sqlQuery]]).toString();
   const toriiUrl = `http://localhost:8080/sql?${params}`;
 
+  while (retryCount < maxRetries) {
+    try {
       const response = await fetch(toriiUrl, {
         method: "GET",
       });
+      if (!response.ok) {
+        throw new Error(`HTTP error! status: ${response.status}`);
+      }
       const data = await response.json();
+      if (!data?.[0]?.head) {
+        throw new Error('Invalid response format');
+      }
       return data[0].head;
+    } catch (error) {
+      retryCount++;
+      if (retryCount === maxRetries) throw error;
+      await new Promise(resolve => setTimeout(resolve, 1000 * retryCount));
+    }
+  }
 };
📝 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
const getToriiCurrentBlock = async () => {
const sqlQuery = "SELECT head FROM contracts WHERE contract_type = 'WORLD' LIMIT 1;";
const params = new URLSearchParams([["query", sqlQuery]]).toString();
const toriiUrl = `http://localhost:8080/sql?${params}`;
const response = await fetch(toriiUrl, {
method: "GET",
});
const data = await response.json();
return data[0].head;
};
const getToriiCurrentBlock = async () => {
const maxRetries = 3;
let retryCount = 0;
const sqlQuery = "SELECT head FROM contracts WHERE contract_type = 'WORLD' LIMIT 1;";
const params = new URLSearchParams([["query", sqlQuery]]).toString();
const toriiUrl = `http://localhost:8080/sql?${params}`;
while (retryCount < maxRetries) {
try {
const response = await fetch(toriiUrl, {
method: "GET",
});
if (!response.ok) {
throw new Error(`HTTP error! status: ${response.status}`);
}
const data = await response.json();
if (!data?.[0]?.head) {
throw new Error('Invalid response format');
}
return data[0].head;
} catch (error) {
retryCount++;
if (retryCount === maxRetries) throw error;
await new Promise(resolve => setTimeout(resolve, 1000 * retryCount));
}
}
};

Comment on lines +117 to +129
<input
type="text"
value={customUrl}
onChange={(e) => {
setCustomUrl(e.target.value);
setSelectedRpc({ name: label, url: e.target.value, worldBlock: 0 });
}}
onFocus={() => {
setSelectedRpc({ name: label, url: customUrl, worldBlock: 0 });
}}
placeholder="Enter custom RPC URL"
className="mt-1 transition-all duration-300 ease-in-out max-w-24 w-24 px-2 py-1 rounded-md outline-none border-0 bg-transparent focus:border-0 focus:outline-none focus:ring-0"
/>
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

Add URL validation and debounce URL updates.

The custom RPC URL input lacks validation and could benefit from debouncing to prevent excessive state updates.

+import { useCallback } from 'react';
+import debounce from 'lodash/debounce';
+
+const isValidUrl = (url: string) => {
+  try {
+    new URL(url);
+    return true;
+  } catch {
+    return false;
+  }
+};
+
+const CustomCheckBox = ({...}) => {
+  const debouncedSetRpc = useCallback(
+    debounce((url: string) => {
+      if (isValidUrl(url)) {
+        setSelectedRpc({ name: label, url, worldBlock: 0 });
+      }
+    }, 300),
+    [label, setSelectedRpc]
+  );
+
   return (
     // ...
         <input
           type="text"
           value={customUrl}
           onChange={(e) => {
             setCustomUrl(e.target.value);
-            setSelectedRpc({ name: label, url: e.target.value, worldBlock: 0 });
+            debouncedSetRpc(e.target.value);
           }}
           onFocus={() => {
-            setSelectedRpc({ name: label, url: customUrl, worldBlock: 0 });
+            if (isValidUrl(customUrl)) {
+              setSelectedRpc({ name: label, url: customUrl, worldBlock: 0 });
+            }
           }}

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

Comment on lines +63 to +124
return (
<>
<img className="z-1 absolute h-screen w-screen object-cover" src={`${backgroundImage}`} alt="Cover" />
<div className="relative top-0 left-0 right-0 bottom-0 w-[100vw] h-[100vh] overflow-hidden flex flex-col justify-center items-center z-100">
<DraggableArea />
<div className="flex flex-row justify-center items-center gap-4 w-6 h-6 z-1000 fixed top-6 left-6 ">
{showSettings ? (
<XMark
onClick={() => {
setShowSettings(false);
}}
className="w-4 h-4 fill-gold transition-all duration-300 ease-in-out hover:scale-125"
/>
) : (
currentRpc && (
<SettingsIcon
onClick={() => {
setShowSettings(true);
}}
className="w-4 h-4 fill-gold z-index-1000 transition-all duration-300 ease-in-out hover:scale-125"
/>
)
)}
</div>

<div className="flex flex-col justify-center items-center max-w-[50vw] bg-black/20 self-center border-[0.5px] border-gradient rounded-lg p-4 text-gold w-full overflow-hidden relative z-0 backdrop-filter backdrop-blur-[24px] shadow-[0_4px_4px_0_rgba(0,0,0,0.25)] transition-all duration-300 ease-in-out">
{showSettings ? (
<Settings showSettings={showSettings} currentRpc={currentRpc} setNewRpc={setNewRpc} />
) : showWarning ? (
<Warning setReset={setReset} showWarning={showWarning} setShowWarning={setShowWarning} />
) : (
<>
<EternumLogo className="w-16 h-16 fill-gold mb-4" />
<SyncingState reset={reset} />

<div className="flex flex-row items-center gap-4 mt-4">
<Refresh
className="hover:bg-brown/10 w-3 h-3 fill-gold transition-all duration-300 ease-in-out hover:scale-125"
onClick={() =>
setShowWarning({
method: IpcMethod.KillTorii,
alertMessage: killToriiAlertMessage,
})
}
/>

<TrashCan
className="hover:bg-brown/10 fill-red w-3 h-3 transition-all duration-300 ease-in-out hover:scale-125"
onClick={() =>
setShowWarning({
method: IpcMethod.ResetDatabase,
alertMessage: resetDatabaseAlertMessage,
})
}
/>
</div>
</>
)}
</div>
</div>
</>
);
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

Add error boundary and loading states.

The component lacks error handling and loading states, which could lead to poor user experience during state transitions or errors.

+import { ErrorBoundary, FallbackProps } from 'react-error-boundary';
+
+const ErrorFallback = ({ error, resetErrorBoundary }: FallbackProps) => (
+  <div className="text-red">
+    <p>Something went wrong:</p>
+    <pre>{error.message}</pre>
+    <button onClick={resetErrorBoundary}>Try again</button>
+  </div>
+);
+
 return (
   <>
     <img className="z-1 absolute h-screen w-screen object-cover" src={`${backgroundImage}`} alt="Cover" />
     <div className="relative top-0 left-0 right-0 bottom-0 w-[100vw] h-[100vh] overflow-hidden flex flex-col justify-center items-center z-100">
       <DraggableArea />
+      <ErrorBoundary FallbackComponent={ErrorFallback}>
+        {isLoading ? (
+          <div className="text-gold">Loading...</div>
+        ) : (
           // ... rest of the UI
+        )}
+      </ErrorBoundary>
     </div>
   </>
 );

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

Comment on lines +9 to +10
sendMessage: (message: IpcMethod, data?: any) => ipcRenderer.send(message, data),
invoke: (message: IpcMethod, data?: any) => ipcRenderer.invoke(message, data),
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

Improve type safety for message data

The data parameter uses any type which bypasses TypeScript's type checking. Consider creating specific interfaces for each IPC method's payload.

-  sendMessage: (message: IpcMethod, data?: any) => ipcRenderer.send(message, data),
+  sendMessage: <T = void>(message: IpcMethod, data?: T) => ipcRenderer.send(message, data),
📝 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
sendMessage: (message: IpcMethod, data?: any) => ipcRenderer.send(message, data),
invoke: (message: IpcMethod, data?: any) => ipcRenderer.invoke(message, data),
sendMessage: <T = void>(message: IpcMethod, data?: T) => ipcRenderer.send(message, data),
invoke: (message: IpcMethod, data?: any) => ipcRenderer.invoke(message, data),

Comment on lines +26 to +43
{
name: '@electron-forge/maker-zip',
config: (arch:string) => ({
// Note that we must provide this S3 URL here
// in order to support smooth version transitions
// especially when using a CDN to front your updates
macUpdateManifestBaseUrl: `http://s3.us-east-1.amazonaws.com/eternum-torii-updates/darwin/${arch}`
})
},
{
name: '@electron-forge/maker-zip',
config: (arch: string) => ({
// Note that we must provide this S3 URL here
// in order to support smooth version transitions
// especially when using a CDN to front your updates
macUpdateManifestBaseUrl: `http://s3.us-east-1.amazonaws.com/eternum-torii-updates/darwin/${arch}`
})
},
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate maker-zip configuration

There are two identical maker-zip configurations with the same S3 URL pattern.

Remove one of the duplicate configurations and consider extracting the S3 URL to a constant:

+const S3_BASE_URL = 'http://s3.us-east-1.amazonaws.com/eternum-torii-updates';
+
 makers: [
   new MakerSquirrel({}),
   {
     name: '@electron-forge/maker-zip',
     config: (arch:string) => ({
-      macUpdateManifestBaseUrl: `http://s3.us-east-1.amazonaws.com/eternum-torii-updates/darwin/${arch}`
+      macUpdateManifestBaseUrl: `${S3_BASE_URL}/darwin/${arch}`
     })
   },
-  {
-    name: '@electron-forge/maker-zip',
-    config: (arch: string) => ({
-      macUpdateManifestBaseUrl: `http://s3.us-east-1.amazonaws.com/eternum-torii-updates/darwin/${arch}`
-    })
-  },
📝 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
{
name: '@electron-forge/maker-zip',
config: (arch:string) => ({
// Note that we must provide this S3 URL here
// in order to support smooth version transitions
// especially when using a CDN to front your updates
macUpdateManifestBaseUrl: `http://s3.us-east-1.amazonaws.com/eternum-torii-updates/darwin/${arch}`
})
},
{
name: '@electron-forge/maker-zip',
config: (arch: string) => ({
// Note that we must provide this S3 URL here
// in order to support smooth version transitions
// especially when using a CDN to front your updates
macUpdateManifestBaseUrl: `http://s3.us-east-1.amazonaws.com/eternum-torii-updates/darwin/${arch}`
})
},
const S3_BASE_URL = 'http://s3.us-east-1.amazonaws.com/eternum-torii-updates';
{
name: '@electron-forge/maker-zip',
config: (arch:string) => ({
// Note that we must provide this S3 URL here
// in order to support smooth version transitions
// especially when using a CDN to front your updates
macUpdateManifestBaseUrl: `${S3_BASE_URL}/darwin/${arch}`
})
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants