-
Notifications
You must be signed in to change notification settings - Fork 113
More build configs & optimise reconnect and immediate game enter #398
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
Conversation
…ion data, this does not affect assets though env: MIN_MC_VERSION MAX_MC_VERSION new SKIP_MC_DATA_RECIPES - if recipes are not used in game fix: refactor QS params handling to ensure panorama & main menu never loaded when immedieate game enter action is expected (eg ?autoConnect=1)
WalkthroughThis update refactors startup logic for game entry, enhances environment variable typing and build configuration, adds version filtering and data skipping to Minecraft data processing, improves error handling in server info retrieval, and introduces a utility for download interest detection. Several props and auto-connect logic are removed from React components. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Startup
participant FS as Filesystem & Config
participant UI as UI Modals
participant Game as Game Logic
participant DL as Download Handler
App->>FS: Wait for fsReady & appConfig
alt Reconnect options exist
App->>Game: Connect using reconnect options
else If reconnect param in dev
App->>Game: Connect using last saved options
else If singleplayer param
App->>Game: Load singleplayer
else If save param
App->>Game: Load save (if exists)
else If ip/proxy param
App->>Game: Auto-connect or show connect modal
else If connectPeer param
App->>UI: Prompt username if needed
UI->>Game: Connect to peer
else If viewerConnect param
App->>Game: Connect as viewer
else If modal param
App->>UI: Show specified modal
else If serversList param
App->>UI: Show servers list modal
else If download param
App->>DL: Trigger download and open file
end
App->>Game: Possibly handle state variable
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
rsbuild.config.ts
(3 hunks)scripts/genLargeDataAliases.ts
(2 hunks)scripts/makeOptimizedMcData.mjs
(8 hunks)src/downloadAndOpenFile.ts
(1 hunks)src/env.d.ts
(1 hunks)src/index.ts
(4 hunks)src/inventoryWindows.ts
(3 hunks)src/react/AddServerOrConnect.tsx
(3 hunks)src/react/ServersListProvider.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/vars-usage.mdc
🧠 Learnings (5)
📓 Common learnings
Learnt from: zardoy
PR: zardoy/minecraft-web-client#373
File: renderer/viewer/three/entities.ts:1120-1120
Timestamp: 2025-06-23T13:33:14.776Z
Learning: In the minecraft-web-client project, team information for entities should be inlined into entity update events from the world data emitter rather than accessing the global bot object from renderer code. This maintains better separation of concerns between the data layer and renderer.
Learnt from: zardoy
PR: zardoy/minecraft-web-client#373
File: renderer/viewer/three/entities.ts:1120-1120
Timestamp: 2025-06-23T13:33:14.776Z
Learning: In the minecraft-web-client project, files under renderer/ directory must not access the global `bot` variable directly according to .cursor/rules/vars-usage.mdc. The updateNameTagVisibility method in renderer/viewer/three/entities.ts currently violates this rule by accessing bot.teamMap and bot.username. Team information should be passed through entity update events from the world data emitter instead.
Learnt from: zardoy
PR: zardoy/minecraft-web-client#391
File: renderer/viewer/three/threeJsUtils.ts:56-56
Timestamp: 2025-07-10T16:03:10.838Z
Learning: The minecraft-web-client project uses core-js auto polyfill injection, which means newer JavaScript features like Promise.withResolvers() are automatically polyfilled for compatibility across different environments.
src/react/ServersListProvider.tsx (1)
Learnt from: zardoy
PR: zardoy/minecraft-web-client#373
File: renderer/viewer/three/entities.ts:1120-1120
Timestamp: 2025-06-23T13:33:14.776Z
Learning: In the minecraft-web-client project, files under renderer/ directory must not access the global `bot` variable directly according to .cursor/rules/vars-usage.mdc. The updateNameTagVisibility method in renderer/viewer/three/entities.ts currently violates this rule by accessing bot.teamMap and bot.username. Team information should be passed through entity update events from the world data emitter instead.
scripts/makeOptimizedMcData.mjs (2)
Learnt from: zardoy
PR: zardoy/minecraft-web-client#373
File: renderer/viewer/three/entities.ts:1120-1120
Timestamp: 2025-06-23T13:33:14.776Z
Learning: In the minecraft-web-client project, files under renderer/ directory must not access the global `bot` variable directly according to .cursor/rules/vars-usage.mdc. The updateNameTagVisibility method in renderer/viewer/three/entities.ts currently violates this rule by accessing bot.teamMap and bot.username. Team information should be passed through entity update events from the world data emitter instead.
Learnt from: zardoy
PR: zardoy/minecraft-web-client#391
File: renderer/viewer/three/threeJsUtils.ts:56-56
Timestamp: 2025-07-10T16:03:10.838Z
Learning: The minecraft-web-client project uses core-js auto polyfill injection, which means newer JavaScript features like Promise.withResolvers() are automatically polyfilled for compatibility across different environments.
src/env.d.ts (1)
Learnt from: CR
PR: zardoy/minecraft-web-client#0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-02T02:50:46.443Z
Learning: Some other global variables that can be used without window prefixes are listed in src/globals.d.ts.
src/index.ts (2)
Learnt from: CR
PR: zardoy/minecraft-web-client#0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-02T02:50:46.443Z
Learning: Applies to src/**/*.ts : You may use the global variable `bot` directly in any file under the `src/` directory (e.g., `src/mineflayer/playerState.ts`).
Learnt from: CR
PR: zardoy/minecraft-web-client#0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-02T02:50:46.443Z
Learning: Applies to renderer/**/*.ts : Do not use the global variable `bot` directly in any file under the `renderer/` directory or its subfolders (e.g., `renderer/viewer/three/worldrendererThree.ts`).
🧬 Code Graph Analysis (2)
src/downloadAndOpenFile.ts (1)
src/appParams.ts (2)
appQueryParams
(81-90)appQueryParamsArray
(92-101)
rsbuild.config.ts (1)
scripts/genLargeDataAliases.ts (1)
genLargeDataAliases
(3-29)
🪛 Biome (1.9.4)
src/index.ts
[error] 924-924: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (20)
src/inventoryWindows.ts (3)
525-525
: LGTM! Good defensive programming.The nullish coalescing operator prevents runtime errors when
loadedData.recipes
is null or undefined.
553-553
: LGTM! Consistent error prevention.This follows the same defensive pattern as the other recipe access points in the file.
598-598
: LGTM! Completes the defensive programming pattern.All three recipe data access points in the file now have proper null/undefined guards.
src/downloadAndOpenFile.ts (1)
14-18
: LGTM! Clean utility function for download detection.This function centralizes the logic for determining download interest based on query parameters. The implementation is clear and follows good practices by checking all relevant parameters in one place.
scripts/genLargeDataAliases.ts (2)
19-20
: LGTM! Good webpack optimization practices.The explicit chunk naming will improve bundle analysis and debugging by providing meaningful names for the generated chunks.
34-34
: LGTM! Global pako access for compatibility.Making
pako.inflate
globally available likely supports existing code patterns or library compatibility requirements.src/react/AddServerOrConnect.tsx (2)
34-34
: LGTM! Good architectural refactoring.Removing the
allowAutoConnect
prop aligns with centralizing auto-connect logic elsewhere in the application, which improves separation of concerns.
226-226
: No import needed fortranslate
Thetranslate
function is provided as an ambient global in src/globals.d.ts (see lines 33–37), so no import is required. Please ignore this suggestion.Likely an incorrect or invalid review comment.
rsbuild.config.ts (3)
142-148
: LGTM! Preparing for performance optimization.The commented-out prefetch configuration shows good planning for future performance improvements targeting the large data chunks.
164-164
: LGTM! Environment variable refactoring.The change from
DEPS_VERSIONS
toALWAYS_MINIMAL_SERVER_UI
appears to be part of a broader environment variable reorganization.
200-200
: LGTM! Enhanced build configuration flexibility.The expanded condition for large data compression provides more granular control over when compression should be applied, supporting both single file builds and explicit compression requests.
src/react/ServersListProvider.tsx (1)
159-175
: Good error handling for WebSocket server info retrieval!The try-catch block properly handles connection failures by providing a fallback data object with appropriate default values, preventing unhandled promise rejections and ensuring the UI can gracefully display connection failures.
src/env.d.ts (1)
5-35
: Excellent type safety improvements for environment variables!The narrowing of boolean-like environment variables to literal union types
'true' | 'false'
provides better compile-time type checking. The new variables are well-organized under clear sections, making the configuration structure more maintainable.scripts/makeOptimizedMcData.mjs (4)
46-66
: Well-implemented version filtering feature!The version clipping logic correctly filters Minecraft versions based on the environment variables, with proper numeric comparison and informative logging of the applied range.
85-87
: Good optimization for conditional data skipping!The implementation cleanly allows skipping language and recipes data processing using environment variables, which can significantly reduce bundle size when these data types aren't needed.
Also applies to: 166-168
274-300
: Better handling of ignored collision shapes!Injecting empty data structures instead of skipping entirely is a safer approach that prevents potential undefined access errors while maintaining consistent data structure.
277-277
: Clearer function naming!Renaming
getData
togetRealData
better distinguishes this function's purpose of retrieving raw data from files.src/index.ts (3)
170-170
: Good state management for reconnect UI!Explicitly setting
showReconnect
to false at the start of connection ensures the UI state is properly reset.
922-1068
: Excellent refactoring of startup logic!The
maybeEnterGame
function successfully consolidates all fragmented startup flows into a single, well-organized function. ThewaitForConfigFsLoad
helper ensures proper initialization order while providing appropriate loading feedback.
1056-1058
: Clean integration with download detection utility!Good use of the
isInterestedInDownload
function to encapsulate download-related query parameter detection logic.
// immediate game enter actions: reconnect or URL QS | ||
const maybeEnterGame = () => { | ||
const waitForConfigFsLoad = (fn: () => void) => { | ||
let unsubscribe: () => void | undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix confusing void in union type
The static analysis correctly identifies that void
in a union type is confusing. The type should indicate that unsubscribe
is either a function or undefined.
- let unsubscribe: () => void | undefined
+ let unsubscribe: (() => void) | undefined
📝 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.
let unsubscribe: () => void | undefined | |
let unsubscribe: (() => void) | undefined |
🧰 Tools
🪛 Biome (1.9.4)
[error] 924-924: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🤖 Prompt for AI Agents
In src/index.ts at line 924, the variable unsubscribe is typed as a union of a
function returning void or undefined, which is confusing. Change the type
annotation so that unsubscribe is either a function with no parameters returning
void or undefined, explicitly indicating it is a function or undefined, not a
union with void.
Thanks for letting me know! |
FYI @Phoenix616 this changes a bit how autoConnect is handled. While it works the same, now technically UI is never opened at all.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores