search

PrismarineJS / prismarine-block

Represent a minecraft block with its associated data
29 stars 32 forks source link

Use correct mapping file to map variants to properties for all pre-flattening versions #95

Open zardoy opened 11 months ago

zardoy commented 11 months ago

As said in https://github.com/PrismarineJS/minecraft-data/issues/771#issuecomment-1718502836 the legacy.json returns incorrect mapping by design, the correct mappings should be used instead.

For example lever with the state 69:0:

as you can see legacy.json has incorrect block properties for 69:0 even for a post-flattening format. There are a couple of things that I've considered to solve this issue:

rom1504 commented 11 months ago

some pointers on fixing legacy.json

it was added in https://github.com/PrismarineJS/minecraft-data/pull/397

https://github.com/PrismarineJS/minecraft-data/issues/216 has more contexts

the correct way here is to fix this file imo

rom1504 commented 11 months ago
  1. finding some other implementation that has the info; and extract from there
  2. same but from the vanilla code ; likely that would be annoying cause these things are hardcoded in java pre 1.13
  3. find the info in the wiki
  4. call the java code with a mod
  5. actually run the game with something like a mod (bariton like), and get the info automatically

my bet is in this case option 1 is the easiest cause 1.12 -> 1.13 is so old I bet other people did it already

extremeheat commented 11 months ago

I believe https://github.com/extremeheat/extracted_minecraft_data/blob/client1.13.2/client/net/minecraft/util/datafix/fixes/BlockStateFlatteningMap.java#L85 should have the mapping data. It's what vanilla does to convert the old IDs to block states.

For the IDs, the lower 4 bits are metadata, and other upper bits are the block ID

const blockId = index >> 4
const metadata = index & 0b1111
rom1504 commented 11 months ago

https://github.com/extremeheat/extracted_minecraft_data/blob/client1.13.2/client/net/minecraft/util/datafix/fixes/BlockStateFlatteningMap.java#L533

this specific example you were mentioning with lever seems correct there @zardoy

zardoy commented 11 months ago

extremeheat/extracted_minecraft_data@client1.13.2/client/net/minecraft/util/datafix/fixes/BlockStateFlatteningMap.java#L533

This is the repo I was looking for all that time, spent a few hours reading different files, and this is really good. I see a lot of data can be also extracted like mob models etc, also will use it for reference. Thank you so much! btw the same on the latest version of mc is client/net/minecraft/util/datafix/fixes/BlockStateData.java

So do we change the current legacy.json or add a new json mapping file to the current repo? Whoever does it please let's ensure it's as small as possible, the current legacy.json file is just unnecessary big

rom1504 commented 11 months ago

Yeah let's do it. Do you want to give it a try ?

On Mon, Dec 18, 2023, 18:52 Vitaly @.***> wrote:

@.*** /client/net/minecraft/util/datafix/fixes/BlockStateFlatteningMap.java#L533 https://github.com/extremeheat/extracted_minecraft_data/blob/client1.13.2/client/net/minecraft/util/datafix/fixes/BlockStateFlatteningMap.java?rgh-link-date=2023-12-18T00%3A56%3A22Z#L533

This is the repo I was looking for all that time, spent a few hours reading different files, and this is really good. I see a lot of data can be also extracted like mob models etc, also will use it for reference. Thank you so much! btw the same on the latest version of mc is client/net/minecraft/util/datafix/fixes/BlockStateData.java

So do we change the current legacy.json or add a new json mapping file to the current repo? Whoever does it please let's ensure it's as small as possible, the current legacy.json file is just unnecessary big

— Reply to this email directly, view it on GitHub https://github.com/PrismarineJS/prismarine-block/issues/95#issuecomment-1861187030, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR437UYJUSUPGMYQGH34KLYKB7GZAVCNFSM6AAAAABAYR6Y66VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRRGE4DOMBTGA . You are receiving this because you commented.Message ID: @.***>

zardoy commented 11 months ago

not at the time, also I'm not sure what is id as the first argument and how to get the block variation number instead

rom1504 commented 11 months ago

Block variation number is in order As for the id we can fetch it from blocks.json

On Mon, Dec 18, 2023, 21:16 Vitaly @.***> wrote:

not at the time, also I'm not sure what is id as the first argument and how to get the block variation number instead

— Reply to this email directly, view it on GitHub https://github.com/PrismarineJS/prismarine-block/issues/95#issuecomment-1861530420, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR437X6GEVZIHTB5MQE3MDYKCQCRAVCNFSM6AAAAABAYR6Y66VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRRGUZTANBSGA . You are receiving this because you commented.Message ID: @.***>

zardoy commented 6 months ago

As for the id we can fetch it from blocks.json

The format was easy: id is the number divided by 16 and metadata (variant) is n mod 16 (basically its state id).

However, after fixing this format, I realized that the block-state format for all pre-flat versions is not correct (no variants support like colored wools and some blocks are missing like concrete for 1.12). And instead of fixing these, it was much easier to just use 1.13 block states for all pre-flat versions (and it works perfectly without downsides). So now I have the map from type:metadata to new_block_name[new_props], if you want I can replace the current legacy.json with that format

just made a parser script for that java file: https://gist.github.com/zardoy/b3b6ba9707ee2c94f2b7f0a1594ce3ce