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

Client side js mods. Modding! #255

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

Client side js mods. Modding! #255

wants to merge 3 commits into from

Conversation

zardoy
Copy link
Owner

@zardoy zardoy commented Jan 28, 2025

PR Type

Enhancement, Tests


Description

  • Introduced a client-side modding system with mod management.

    • Added database support for mods and repositories.
    • Implemented mod activation, installation, and updates.
  • Enhanced UI with mod-related features.

    • Added a "Client Mods" button in the options menu.
    • Created a dedicated mods page for managing mods.
  • Added configuration options for mod support and updates.

  • Included a new HTML configuration page for client settings.


Changes walkthrough 📝

Relevant files
Configuration changes
1 files
rsbuild.config.ts
Added support for copying `config.html` to the build output.
+1/-0     
Enhancement
8 files
clientMods.ts
Implemented client-side modding system with database and activation
logic.
+295/-0 
index.ts
Integrated mod startup logic into the application initialization.
+2/-0     
optionsGuiScheme.tsx
Added "Client Mods" button to the options menu.                   
+7/-0     
optionsStorage.ts
Added mod-related configuration options.                                 
+3/-0     
ModsPage.tsx
Created a new mods management page.                                           
+13/-0   
storageProvider.ts
Added storage support for mod update checks.                         
+1/-0     
reactUi.tsx
Integrated the mods page into the React UI structure.       
+2/-0     
config.html
Added a new HTML configuration page for client settings. 
+25/-0   

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 Jan 28, 2025

    Review or Edit in CodeSandbox

    Open the branch in Web EditorVS CodeInsiders

    Open Preview

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 Security concerns

    Remote Code Execution:
    The mod system allows downloading and executing arbitrary JavaScript code from remote sources without proper validation or sandboxing (src/clientMods.ts lines 139-144). This could allow malicious code to be executed in the context of the application. Consider implementing code signing, content security policies, and proper sandboxing mechanisms.

    ⚡ Recommended focus areas for review

    Security Risk

    The mod installation process loads and executes arbitrary JavaScript code from mod repositories without proper validation or sandboxing, which could allow malicious code execution.

    const blob = new Blob([mod.scriptMainUnstable], { type: 'application/javascript' })
    const url = URL.createObjectURL(blob)
    try {
      const module = await import(url)
      module.default?.(structuredClone(mod))
      window.loadedMods[mod.name] = module
    Incomplete Implementation

    The configuration page has non-functional buttons and lacks proper event handlers for critical data management operations.

    <button>Reset all settings</button>
    <button>Remove all user data (worlds, resourcepacks)</button>
    <button>Remove all mods</button>
    <button>Remove all mod repositories</button>
    Error Handling

    The mod activation and installation functions lack comprehensive error handling and recovery mechanisms, which could lead to unstable application state.

    const activateMod = async (mod: ClientMod, reason: string) => {
      console.debug(`Activating mod ${mod.name} (${reason})...`)
      if (window.loadedMods[mod.name]) {
        console.warn(`Mod is ${mod.name} already loaded, skipping activation...`)
        return false
      }
      if (mod.stylesGlobal) {
        const style = document.createElement('style')
        style.textContent = mod.stylesGlobal
        style.id = `mod-${mod.name}`
        document.head.appendChild(style)
      }
      if (mod.scriptMainUnstable) {
        const blob = new Blob([mod.scriptMainUnstable], { type: 'application/javascript' })
        const url = URL.createObjectURL(blob)
        try {
          const module = await import(url)
          module.default?.(structuredClone(mod))
          window.loadedMods[mod.name] = module
        } catch (e) {
          console.error(`Error loading mod ${mod.name}:`, e)
        }
      }
      return true
    }

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Prevent memory leaks in blob URLs

    Add error handling for the case when the blob URL creation fails. The current code
    could leak memory if URL.createObjectURL fails, as the blob URL is never revoked.

    src/clientMods.ts [139-148]

     const blob = new Blob([mod.scriptMainUnstable], { type: 'application/javascript' })
    -const url = URL.createObjectURL(blob)
    +let url
     try {
    +  url = URL.createObjectURL(blob)
       const module = await import(url)
       module.default?.(structuredClone(mod))
       window.loadedMods[mod.name] = module
     } catch (e) {
       console.error(`Error loading mod ${mod.name}:`, e)
    +} finally {
    +  if (url) URL.revokeObjectURL(url)
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a potential memory leak by properly cleaning up blob URLs. This is an important fix for resource management and preventing memory issues in long-running applications.

    8
    Add missing button functionality

    The buttons are defined but lack onclick handlers, making them non-functional. Add
    event handlers to implement the intended functionality for each button.

    assets/config.html [18-21]

     <div style="display: flex;gap: 10px;">
    -    <button>Reset all settings</button>
    -    <button>Remove all user data (worlds, resourcepacks)</button>
    -    <button>Remove all mods</button>
    -    <button>Remove all mod repositories</button>
    +    <button onclick="removeSettings()">Reset all settings</button>
    +    <button onclick="removeUserData()">Remove all user data (worlds, resourcepacks)</button>
    +    <button onclick="removeMods()">Remove all mods</button>
    +    <button onclick="removeModRepositories()">Remove all mod repositories</button>
     </div>
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion fixes non-functional UI elements by adding necessary onclick handlers. This is important for user interaction and making the configuration interface work as intended.

    7
    Validate version strings before comparison

    Add validation for the mod version format before using semver.gt() comparison to
    prevent potential crashes with invalid version strings.

    src/clientMods.ts [200-201]

    -if (modExisting?.version && gt(mod.version, modExisting.version)) {
    +const isValidVersion = (v: string) => /^\d+\.\d+\.\d+/.test(v);
    +if (modExisting?.version && isValidVersion(mod.version) && isValidVersion(modExisting.version) && gt(mod.version, modExisting.version)) {
       modsUpdateStatus[mod.name] = [modExisting.version, mod.version]
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion adds important input validation to prevent potential crashes when comparing version strings. This improves the robustness of the mod update system.

    7

    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.

    1 participant