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: reimplement auto version - fix bugs, +config, make faster!! #262

Merged
merged 4 commits into from
Feb 3, 2025

Conversation

zardoy
Copy link
Owner

@zardoy zardoy commented Feb 3, 2025

User description

  • Allow to set different preferrable auto version!!!
  • Better messages on loading screen
  • Auto select is now 2x faster
  • Fix bug that was cause server disconnect during long data loading
  • Now Minecraft data are loaded before connecting to server
  • Make client less detectable

PR Type

Enhancement, Bug fix


Description

  • Implemented auto-version selection for server connections.

  • Improved loading screen messages and connection feedback.

  • Fixed server disconnect issue during long data loading.

  • Refactored and optimized Minecraft data handling.


Changes walkthrough 📝

Relevant files
Enhancement
8 files
appParams.ts
Improved query string parameter handling with fallback.   
+1/-1     
connect.ts
Added auto-version selection and improved connection flow.
+19/-7   
index.ts
Enhanced connection logic and loading screen updates.       
+21/-9   
minecraft-protocol-extra.ts
Added server version ping utility for auto-versioning.     
+27/-0   
optionsGuiScheme.tsx
Added GUI slider for server version selection.                     
+30/-0   
optionsStorage.ts
Introduced `serversAutoVersionSelect` option for auto-versioning.
+1/-1     
minecraftData.ts
Removed hardcoded Minecraft initial data handling.             
+4/-4     
makeOptimizedMcData.mjs
Commented out unused Minecraft initial data generation.   
+1/-1     
Bug fix
1 files
importsWorkaround.js
Added global fallback for `window` and `localStorage`.     
+2/-0     
Dependencies
2 files
[email protected]
Removed outdated patch for Minecraft protocol.                     
+0/-188 
[email protected]
Removed outdated patch for Minecraft protocol.                     
+6/-49   

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link

    codesandbox bot commented Feb 3, 2025

    Review or Edit in CodeSandbox

    Open the branch in Web EditorVS CodeInsiders

    Open Preview

    @zardoy zardoy changed the title reimplement fast and auto version!!! reimplement auto version - fix bugs, make faster Feb 3, 2025
    @zardoy
    Copy link
    Owner Author

    zardoy commented Feb 3, 2025

    /deploy

    @qodo-merge-pro-for-open-source qodo-merge-pro-for-open-source bot changed the title reimplement auto version - fix bugs, make faster reimplement fast and auto version!!! Feb 3, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The auto version selection logic may fail silently if server ping fails or returns invalid version. Should add proper error handling and fallback logic.

    Resource Cleanup

    The pingServerVersion function creates a fake client but doesn't properly clean up resources by calling client.end() after getting the version.

    export const pingServerVersion = async (ip: string, port?: number, preferredVersion?: string) => {
      const fakeClient = new EventEmitter() as any
      const options = {
        host: ip,
        port,
        version: preferredVersion,
        noPongTimeout: Infinity // disable timeout
      }
      // let latency = 0
      // fakeClient.autoVersionHooks = [(res) => {
      //   latency = res.latency
      // }]
    
      // TODO! use client.socket.destroy() instead of client.end() for faster cleanup
      await clientAutoVersion(fakeClient, options)
    
      await new Promise<void>((resolve, reject) => {
        fakeClient.once('connect_allowed', resolve)
      })
      return {
        version: fakeClient.version,
        // latency,
      }
    }

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Remove hardcoded version number

    The auto version selection logic hardcodes '1.20.4' as default version. This
    should be derived from supportedVersions to avoid maintenance issues when new
    versions are added.

    src/connect.ts [29-37]

     export const getVersionAutoSelect = (autoVersionSelect = options.serversAutoVersionSelect) => {
       if (autoVersionSelect === 'auto') {
    -    return '1.20.4'
    +    return supportedVersions.at(-2)! // Use second-to-last version for better stability
       }
       if (autoVersionSelect === 'latest') {
         return supportedVersions.at(-1)!
       }
       return autoVersionSelect
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Using hardcoded version numbers makes code brittle and requires manual updates. Using supportedVersions array makes the code more maintainable and automatically adapts to version changes.

    7
    Improve error message clarity

    The error handling for socket disconnection uses a generic 'socketClosed'
    message. Provide more specific error information to help users understand
    connection issues.

    src/index.ts [646-648]

     if (endReason === 'socketClosed') {
    -  endReason = 'Connection with server lost'
    +  endReason = 'Connection with server lost - Please check your internet connection and server status'
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 4

    Why: Adding more descriptive error messaging helps users better understand and troubleshoot connection issues, though the impact is relatively minor as it only affects user experience.

    4

    @zardoy
    Copy link
    Owner Author

    zardoy commented Feb 3, 2025

    /deploy

    @zardoy zardoy changed the title reimplement fast and auto version!!! feat: reimplement auto version - fix bugs, make faster!!! Feb 3, 2025
    @zardoy zardoy changed the title feat: reimplement auto version - fix bugs, make faster!!! feat: reimplement auto version - fix bugs, +config, make faster!! Feb 3, 2025
    @zardoy zardoy merged commit 72058f1 into next Feb 3, 2025
    2 checks passed
    @zardoy zardoy deleted the auto-version-fix branch February 3, 2025 07:20
    Copy link

    github-actions bot commented Feb 3, 2025

    Deployed to Vercel Preview: https://prismarine-rhmmcohn7-zaro.vercel.app
    Playground
    Storybook

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

    Successfully merging this pull request may close these issues.

    1 participant