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.

Sign up for GitHub

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

Jump to bottom

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

Open
zardoy opened this issue Dec 17, 2023 · 9 comments
Open

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

zardoy opened this issue Dec 17, 2023 · 9 comments

Comments

@zardoy
Copy link
Contributor

zardoy commented Dec 17, 2023

As said in PrismarineJS/minecraft-data#771 (comment) 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:

  • use indexes from mc-assets block states to match the block state needed to render for a block variation. Sometimes it doesn't work correctly
  • regenerate the mapping file so it uses old (pre-flattening format) and the mappings are correct
  • regenerate the mapping file so it still uses new (post-flattening format) and the mappings are correct & use post-flattening block states & models for render - easier to do and I think this is preferable way since it will also make scripts & anything else that expects properties in post-flattening format work.
@rom1504
Copy link
Member

rom1504 commented Dec 17, 2023

some pointers on fixing legacy.json

it was added in PrismarineJS/minecraft-data#397

PrismarineJS/minecraft-data#216 has more contexts

the correct way here is to fix this file imo

@rom1504
Copy link
Member

rom1504 commented Dec 17, 2023

  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
Copy link
Member

extremeheat commented Dec 18, 2023 •
edited
Loading

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
Copy link
Member

rom1504 commented Dec 18, 2023

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
Copy link
Contributor Author

zardoy commented Dec 18, 2023

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
Copy link
Member

rom1504 commented Dec 18, 2023 via email

@zardoy
Copy link
Contributor Author

zardoy commented Dec 18, 2023

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
Copy link
Member

rom1504 commented Dec 18, 2023 via email

@zardoy
Copy link
Contributor Author

zardoy commented May 19, 2024 •
edited
Loading

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@rom1504 @extremeheat @zardoy