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

Block state ids greater than 256 are converted to 256 in chunk sections that use the global palette #445

Open
garboit opened this issue Aug 8, 2021 · 0 comments · May be fixed by #514
Open

Comments

@garboit
Copy link
Contributor

garboit commented Aug 8, 2021

Description

If a chunk section has just switched to use the global palette and an additional block with block-state id > 256 is added, it will instead be registered as the block state with id 256 (BlockId { kind: NoteBlock, state: 6 })

Reproduction Steps

Run the following test in base/src/chunk.rs

    #[test]
    fn fill_chunk_section_with_unique_states() {
        let mut section = ChunkSection::default();

        //maximum amount of blocks before the global palette is used
        let max_block_states = (1 << MAX_BITS_PER_BLOCK) - 1;

        //place unique block states
        for i in 0..max_block_states {
            section.set_block_at(0, 0, 0, BlockId::from_vanilla_id(i + 1)).unwrap()
        }

        //the chunk section should still use a local palette.
        assert!(section.blocks.palette().is_some());

        //the next block should cause this chunk to use the global palette

        let highest_block_state = BlockId::from_vanilla_id(257);
        section.set_block_at(0,0,0, highest_block_state);
        assert_eq!(section.block_at(0, 0,0).unwrap(), highest_block_state);
    }

Output:

Left:  BlockId { kind: NoteBlock, state: 6 }
Right: BlockId { kind: NoteBlock, state: 9 }

The reason for this is that when the chunk section switches to the global palette, it still returns the old index:

let index = p.index_or_insert(block);
self.resize_if_needed();
index

An easy fix would be:

let index = p.index_or_insert(block);
self.resize_if_needed();
if self.palette.is_none() {
    block.vanilla_id() as usize
} else {
    index
}

It would probably be good, if the palette returned a slice instead of an index, because then the compiler would have caught this error, but I don't have enough experience with rust to say whether this is really a better approach.

@garboit garboit changed the title Block states ids > 256 are converted to 256 in chunk sections that use the global palette Block state ids greater than 256 are converted to 256 in chunk sections that use the global palette Aug 8, 2021
@Iaiao Iaiao linked a pull request Jan 10, 2022 that will close this issue
10 tasks
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 a pull request may close this issue.

1 participant