SmylerMC / litemapy

Litemapy's goal is to provide an easy to use way to read and edit Litematica's schematic file format
GNU General Public License v3.0
52 stars 6 forks source link

IndexError in Entity class due to inconsistent handling of 'Rotation' data #50

Closed Kingburrito777 closed 3 months ago

Kingburrito777 commented 3 months ago

title: IndexError in Entity class due to inconsistent handling of 'Rotation' data labels: bug

Description

There's an inconsistency in the Entity class in how it handles the 'Rotation' data. The class initializes 'Rotation' with two elements if it's not present, but later attempts to access a third element, causing an IndexError.

Code Causing the Issue

In the Entity class (litemapy/minecraft.py):

class Entity:
    def __init__(self, str_or_nbt: Union[str, Compound]) -> None:
        # ...
        if 'Rotation' not in keys:
            self._data['Rotation'] = List[Double]([Double(0.), Double(0.)])
        # ...
        rotation = [float(coord) for coord in self._data['Rotation']]
        self._rotation = (rotation[0], rotation[1], rotation[2])  # This line causes the error
        # ...

Error Message

IndexError: list index out of range

Expected Behavior

The Entity class should consistently handle 'Rotation' data, either always expecting two elements or always expecting three elements.

Actual Behavior

The class initializes 'Rotation' with two elements but later attempts to access a third element, causing an IndexError when the third element doesn't exist.

Possible Solutions

  1. Modify the initialization to always include three elements:

    if 'Rotation' not in keys:
       self._data['Rotation'] = List[Double]([Double(0.), Double(0.), Double(0.)])
  2. Modify the assignment to self._rotation to only use two elements:

    self._rotation = (rotation[0], rotation[1])
  3. Use a more flexible approach that works with either two or three elements:

    self._rotation = tuple(rotation + [0.0] * (3 - len(rotation)))

Additional Context

This issue affects the loading of litematic files that contain entity data. It's possible that different versions of the litematic format or Minecraft itself use different numbers of rotation elements for entities.

Steps to Reproduce

  1. Create a litematic file that includes entity data with only two rotation elements.
  2. Attempt to load this file using the LitematicSchematic.load() method.

Environment

SmylerMC commented 3 months ago

Thank you for reporting this bug.

As far as I know rotations are supposed to have two elements in Minecraft (yaw and pitch). This is what was originally implemented in the constructor. Somewhere along the way, it appears we ended-up considering it as a 3-float tuple instead of a 2-float tuple in almost every place. I don't think that's correct.

SmylerMC commented 3 months ago

Hi @Kingburrito777 , do you want to check-out #51 and confirm it fixes your issue?

Kingburrito777 commented 3 months ago

Hi @Kingburrito777 , do you want to check-out #51 and confirm it fixes your issue?

I'll do so shortly. I'll be sure to test a few different test cases thanks Liam