expose all BlockState properties and allow passing them back in

[svensven]

e.g. I wanted to un-waterlog a load of blocks with other properties I wanted to keep, so with this change I could do

props = b.properties
props['waterlogged'] = 'false'
newblock = BlockState(b.blockid, properties=props)
reg.setblock(x, y, z, newblock)

[SmylerMC]

Hello, thank you for your proposal. However, the BlockState class is supposed to be immutable. By exposing the properties this way, you are making them mutable again by allowing external code to mutate the internal property dictionary.

The example code you provided would in-fact un-waterlog all blocks that share the exact same blockstate as the one you are trying to change. Litemapy used to work this way, but it was changed to make it more forgiving to users that are not so familiar with programming concepts like references and mutability and just want to make simple Litematic edits.

Still, the use case you are highlighting is perfectly valid and it would be great to have an easy way to accomplish it. What would you think about adding a with_property(key, value) method to BlockState? This method would create a copy of the internal property dictionary, changes the desired property there, and then creates and returns a new BlockState using the new properties.

I'm closing this PR for now, you are more than welcome to create a new one if you want to implement something like I suggested above. Otherwise I'll probably do it myself in the near future, something is definitely needed.

[svensven]

I don't really understand how it makes it mutable, but I'm just coding python by copy-and-paste... What I was trying to do, was just get all of the properties out as a dict or similar, so I could adjust them before creating a new BlockState object. I couldn't see an existing way that would give me that. One might conceivably want to change multiple properties on a block at once, too.

This is what I was doing, extended from the example code, and it worked perfectly for me (with the PR modifications, and however inelegant it might be). But was it not in fact working on each block in the region in turn, replacing some with new blocks, but doing some global replace of each distinct BlockState with the subsequent replacements being redundant?

for y in reg.yrange():
    for x in reg.xrange():
        for z in reg.zrange():
            b = reg.getblock(x, y, z)
            if b.blockid == "minecraft:air":
                print(" ", end="")
            elif b.blockid in [
                "minecraft:water",
                "minecraft:sand",
                "minecraft:dirt",
                "minecraft:gravel",
                "minecraft:kelp",
                "minecraft:kelp_plant",
                "minecraft:seagrass",
                "minecraft:tall_seagrass",
            ]:
                print("~", end="")
                airblock = BlockState("minecraft:air")
                reg.setblock(x, y, z, airblock)
            else:
                try:
                    if b['waterlogged'] == 'true':
                        props = b.properties
                        props['waterlogged'] = 'false'
                        newblock = BlockState(b.blockid, properties=props)
                        reg.setblock(x, y, z, newblock)
                        print("w", end="")
                    else:
                        print("#", end='')
                except KeyError:
                    print("#", end='')
        print()
    print()
    print()

[SmylerMC]

To save space, litematics store blockstates using a palette. The actual content array contains integers, which are the index of the blockstate in the palette. When you access the block at a given position, litemapy looks up the number in the integer array, and then looks up the blockstate in the palette at that index.

See the getblock() method:

    def getblock(self, x, y, z):
        """
        Get a :class:`~litemapy.BlockState` in the region.

        :param x:   the X coordinate to get the block at
        :type x:    int
        :param y:   the Y coordinate to get the block at
        :type y:    int
        :param z:   the Z coordinate to get the block at
        :type z:    int

        :rtype:     ~litemapy.BlockState
        """
        x, y, z = self.__regcoordinates2storecoords(x, y, z)
        return self.__palette[self.__blocks[x, y, z]]

So, if the following was allowed, it would change all air blocks to stone:

state1 = reg.getblock(0, 0, 0)  # Let's say there is air at that position
state2 = reg.getblock(1, 0, 0)  # Let's say there also is air there

print(state1.blockid, state2.blockid)   # >>> "minecraft:air",  "minecraft:air"

# Now, if we could change state1 (which is not allowed to prevent the issue we are demonstrating):
state1.blockid = "minecraft:stone"

# All the region's air blocks were changed to stone
print(state1.blockid, state2.blockid)   # >>> "minecraft:stone",  "minecraft:stone"

This is because state1 and state2 are two references to the same object (the same place in the computer's memory). You can check this using the is operator in Python: state1 is state2 # True.

While experienced programmers know about this and could even use it to their advantage, it brings complexity for those less familiar with those concepts. I used the ID as an example here, but the problem is the same regarding the properties. I hope that clears things a bit for.

One might conceivably want to change multiple properties on a block at once, too.

You are absolutely right, maybe something like this would be best, with additional logic to handle removing properties:

class BlockState:

    ...

    def with_properties(self, **kwargs):
        other = BlockState(self.blockid)  # We create a new blockstate
        other.__properties.update(self.__properties)  # We give it the same properties
        other.__properties.update(kwargs)   # And then change the properties using what the caller supplied
        return other

You could use it like so:

state1 = BlockState("minecraft:oak_log", properties={"facing": "north"})
state2 = state1.with_properties(facing="west", stripped="true")

print(state1)  # >>> minecraft:oak_log[facing=north]
print(state2)  # >>> minecraft:oak_log[facing=west, stripped=true]
print(state1 is state2)  # >>> False