Skip to content

Tallgrass#468

Merged
iceiix merged 4 commits intomasterfrom
tallgrass
Jan 11, 2021
Merged

Tallgrass#468
iceiix merged 4 commits intomasterfrom
tallgrass

Conversation

@iceiix
Copy link
Copy Markdown
Owner

@iceiix iceiix commented Jan 10, 2021

Attempting to fix #467 grass is fire

@iceiix
Copy link
Copy Markdown
Owner Author

iceiix commented Jan 10, 2021

This fixes the wrong block state IDs amongst the "grass" variants #467 (comment), but it still shows up as fire. Needs more investigation...

We also have a bigger problem here. The block states are not stable between versions. Consider the "fern":

1.13: 1042 https://github.com/PrismarineJS/minecraft-data/blob/master/data/pc/1.13/blocks.json#L2961
1.14.4: 1342 https://github.com/PrismarineJS/minecraft-data/blob/master/data/pc/1.14.4/blocks.json#L2993
1.15.2: 1342 https://github.com/PrismarineJS/minecraft-data/blob/master/data/pc/1.15.2/blocks.json#L2993
1.16.2: 1343 https://github.com/PrismarineJS/minecraft-data/blob/master/data/pc/1.16.2/blocks.json#L2993

blocks/src/lib.rs does not hardcode the block state IDs, but generates them sequentially based on the offset function. If new states are added, they will shift other states forward. This will needed to be accounted for on a per-version basis, such maybe by adding a tag for "minimum supported protocol version" for each block and/or block state, and generating the states when connecting to a server of a given version.

However this all seems to be separate problem than the grass = fire issue... not to mention the #71 1.13/1.14/1.15/1.16 assets, update issue (yet another incompatibility: the resource version number).

@iceiix
Copy link
Copy Markdown
Owner Author

iceiix commented Jan 11, 2021

Aha... tallgrass actually renders (almost) fine in 1.13.2, it only becomes fire in 1.14.4 because of the 1042 -> 1342 block state identifier shift.

This PR does fix a bug, before, the grass was dead:

Screen Shot 2021-01-10 at 4 01 17 PM

now, it is alive:

Screen Shot 2021-01-10 at 4 06 13 PM

but accounting for the shifting block states per version (#467) is going to be a harder problem to solve

@iceiix iceiix merged commit 6371649 into master Jan 11, 2021
@iceiix iceiix deleted the tallgrass branch January 11, 2021 00:12
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.14-1.16.4: tall grass is fire, need per-version block states

1 participant