PrismarineJS / bedrock-protocol

Minecraft Bedrock protocol library, with authentication and encryption
https://prismarinejs.github.io/minecraft-data/?v=bedrock_1.17.10&d=protocol
MIT License
308 stars 72 forks source link

Issue with Recipes on a relay. #469

Closed AnyBananaGAME closed 9 months ago

AnyBananaGAME commented 9 months ago

I have been using a Relay for quite a while, its alright and works fine but theres one big issue with it that prevents it from being not just a debugging tool.

The recipes are broken. They aren't appearing or working in crafting. Screenshot_20231212_203538_Minecraft.png This was tested on PocketMine-MP I have extracted 1.20.50 recipes and have added them to mcdata but it has done nothing, some recipes work tho and i have a suspicion it may not work on items with multiple recipes but that doesn't make sense.

AnyBananaGAME commented 9 months ago

Are all the recipes sent to the player via crafting_data packet? Do i need to edit it then?

extremeheat commented 9 months ago

There should be no need to extract anything. The Relay is a proxy that passes along all the information it gets without modification, unless the library user implements some overriding behavior. The only obvious issue I could see here is a timing related issue, but otherwise unless a packet is being dropped somewhere, this doesn't make much sense to me.

AnyBananaGAME commented 9 months ago

The crafting_data packet from pocketmine has some weird recipe_id's. image image Could this be an issue for the relay?

AnyBananaGAME commented 9 months ago

Tried the same on the BDS this time and recipes are working fine,

 recipe: {
    recipe_id: 'minecraft:smithing_netherite_boots',
    template: {
      type: 'int_id_meta',
      network_id: 685,
      metadata: 32767,
      expression: undefined,
      version: undefined,
      tag: undefined,
      name: undefined,
      count: 1
    },

It also works fine with gophertunnel so it has to be bedrock-protocol issue.

AnyBananaGAME commented 9 months ago

The crafting_data packet from pocketmine has some weird recipe_id's. image image Could this be an issue for the relay?

Okay i found the problem. It was indeed the recipe_id's

AnyBananaGAME commented 9 months ago

I tried to console.log the recipe_id but it was logging nothing so then saved the packet into a file and it as there. So it came to my mind that those \u0000 just do not work with it so i added some code to replace the ids and yea it fixed the issue.

 console.log(`ID: ${params.recipes[2].recipe.recipe_id}`)
        params.recipes.forEach(recipe => {
            let newId = Utils.generateRandomString(32);
            this.recipes[recipe.recipe.recipe_id] = newId;
            recipe.recipe.recipe_id = newId;
        });
AnyBananaGAME commented 9 months ago

@extremeheat should this be in the bedrock-protocol itself? Since pmmp does not care about recipe_id just the client

extremeheat commented 9 months ago

No, that is not correct code. If the type is wrong, it should be corrected in minecraft-data, but I don't see any errors based on any of the other implementations. You can check if Gophertunnel is returning a correct string, but otherwise it seems to be the server is sending incorrect data. The fact that it decodes correctly in vanilla and not on 3P is indicative that it's a server problem.

AnyBananaGAME commented 9 months ago

No, that is not correct code. If the type is wrong, it should be corrected in minecraft-data, but I don't see any errors based on any of the other implementations. You can check if Gophertunnel is returning a correct string, but otherwise it seems to be the server is sending incorrect data. The fact that it decodes correctly in vanilla and not on 3P is indicative that it's a server problem.

I tested it on both a popular pocketmine server and a fresh one and the ids were same. PocketMine-MP is the most popular sofware so a crafting should be fully working And ad I said that doesnt happen in gophertunnel the recipes do work.

extremeheat commented 9 months ago

And ad I said that doesnt happen in gophertunnel the recipes do work.

Is not helpful information. What works? Where is the output you read with Gophertunnel?

AnyBananaGAME commented 9 months ago

well now that i looked through the Gophertunnel's issues and found the same thing. So it is indeed a pocketmine thing. But wouldn't it be good to add a temporary fix? or an example in examples on how to temporary fix it https://github.com/df-mc/dragonfly/issues/796

extremeheat commented 9 months ago

If you have a correction to make you can open a PR.

AnyBananaGAME commented 9 months ago

crafting_data.txt

 try {
      des = this.server.deserializer.parsePacketBuffer(packet)
      if(des.data.name === "crafting_data"){
        console.log(packet)
        const fs = require("fs")
        fs.writeFileSync(`./crafting_data.txt`, packet)
      }
    }

Was this a safe way to obtain it?

extremeheat commented 9 months ago

The raw bytes are emitted under the normal events, so there is no need to do that, but that works.

I can confirm there's an serialization issue here. It seems related to text encoding. We are reading the string as a UTF-8 string, which is converted to a UTF-16 string per JavaScript, which we then run Buffer.byteLength('utf8') on which will give a different byte length (and break string encoding).

Adding a AsciiString type with latin1 encoding should fix this weird issue, since there is no multi-byte char issues. nickelpro added custom encoding support to ProtoDef with https://github.com/ProtoDef-io/ProtoDef/pull/43 so should be a easy to fix in mc-data.

extremeheat commented 9 months ago

@AnyBananaGAME copy https://github.com/extremeheat/minecraft-data/blob/bedrock-recipe/data/bedrock/1.20.50/protocol.json into your local ./node_modules/minecraft-data/minecraft-data/data/bedrock/1.19.50/protocol.json and see if this problem still exists

AnyBananaGAME commented 9 months ago

It appears to work now! Thank you