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: custom block models via custom channel #277

Merged
merged 24 commits into from
Feb 17, 2025
Merged

feat: custom block models via custom channel #277

merged 24 commits into from
Feb 17, 2025

Conversation

zardoy
Copy link
Owner

@zardoy zardoy commented Feb 17, 2025

PR Type

Enhancement, Tests


Description

  • Implement custom block models with a new channel.

  • Add support for dynamic block model updates.

  • Update mesher and world logic for custom models.

  • Introduce new configuration options for custom channels.


Changes walkthrough 📝

Relevant files
Enhancement
7 files
mesher.ts
Add handling for custom block models in mesher                     
+9/-2     
shared.ts
Define `CustomBlockModels` type and integrate with mesher
+5/-0     
world.ts
Extend world logic to support custom block models               
+14/-5   
worldrendererCommon.ts
Update world renderer to handle custom block models           
+29/-6   
customChannels.ts
Implement custom channel for block model updates                 
+75/-0   
index.ts
Register custom channels in the main entry point                 
+1/-0     
resourcePack.ts
Update resource pack logic for asset updates                         
+1/-1     
Miscellaneous
1 files
viewer.ts
Remove unused block provider in demoModel                               
+0/-1     
Configuration changes
1 files
optionsStorage.ts
Add configuration option for custom channels                         
+1/-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 Feb 17, 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
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Memory Leak

    The block cache grows unbounded as new custom block models are added. Consider implementing cache size limits or cleanup.

    const cacheKey = modelOverride ? `${stateId}:${modelOverride}` : stateId
    
    if (!this.blockCache[cacheKey]) {
      const b = column.getBlock(locInChunk) as unknown as WorldBlock
      if (modelOverride) {
        b.name = modelOverride
      }
      b.isCube = isCube(b.shapes)
      this.blockCache[cacheKey] = b
    Error Handling

    Missing error handling for channel registration and message processing. Network errors or malformed messages could crash the application.

    bot._client.registerChannel(CHANNEL_NAME, packetStructure, true)
    
    bot._client.on(CHANNEL_NAME as any, (data) => {
      const { worldName, x, y, z, model } = data
      console.debug('Received model data:', { worldName, x, y, z, model })
    
      if (viewer?.world) {
        const chunkX = Math.floor(x / 16) * 16
        const chunkZ = Math.floor(z / 16) * 16
        const chunkKey = `${chunkX},${chunkZ}`
        const blockPosKey = `${x},${y},${z}`
    
        const chunkModels = viewer.world.customBlockModels.get(chunkKey) || {}
    
        if (model) {
          chunkModels[blockPosKey] = model
        } else {
          delete chunkModels[blockPosKey]
        }
    
        if (Object.keys(chunkModels).length > 0) {
          viewer.world.customBlockModels.set(chunkKey, chunkModels)
        } else {
          viewer.world.customBlockModels.delete(chunkKey)
        }
    
        // Trigger update
        const block = worldView!.world.getBlock(new Vec3(x, y, z))
        if (block) {
          worldView!.world.setBlockStateId(new Vec3(x, y, z), block.stateId)
        }
      }
    })
    Race Condition

    Potential race condition between worker messages and custom block model updates. Consider adding synchronization mechanisms.

    const chunkKey = `${x},${z}`
    const customBlockModels = this.customBlockModels.get(chunkKey)
    
    for (const worker of this.workers) {
      worker.postMessage({
        type: 'chunk',
        x,
        z,
        chunk,
        customBlockModels: customBlockModels || undefined
      })
    }

    Copy link

    qodo-merge-pro-for-open-source bot commented Feb 17, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add null check for cache access

    Add null check for this.blockCache[cacheKey] before accessing it. The current
    code assumes the cache entry exists but it might not, potentially causing
    runtime errors.

    renderer/viewer/lib/mesher/world.ts [174-176]

     const block = this.blockCache[cacheKey]
    +if (!block) return null
     
     if (block.models === undefined && blockProvider) {
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion addresses a critical potential runtime error by adding a necessary null check before accessing the cache entry, which could prevent application crashes in production.

    Medium
    General
    Add error handling for channel operations

    Add error handling for the channel registration and message processing to
    prevent potential crashes if the channel registration fails or receives
    malformed data.

    src/customChannels.ts [40-42]

    -bot._client.registerChannel(CHANNEL_NAME, packetStructure, true)
    +try {
    +  bot._client.registerChannel(CHANNEL_NAME, packetStructure, true)
     
    -bot._client.on(CHANNEL_NAME as any, (data) => {
    +  bot._client.on(CHANNEL_NAME as any, (data) => {
    +    try {
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    __

    Why: Adding error handling for channel registration and message processing is important for application stability, preventing potential crashes from network-related issues or malformed data.

    Medium
    • Update

    @zardoy zardoy merged commit 5f6ce69 into next Feb 17, 2025
    2 checks passed
    @zardoy zardoy deleted the custom-blocks branch February 17, 2025 16:40
    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