(#23) BspLump can be edited indirectly

CHANGELOG.md

@@ -17,6 +17,10 @@
    - added `append`, `extend` & `insert` methods to `RawBspLump`
    - `BspLump.find(attr=val)` method is now `.search()`
    - removed `.find()` method from `BasicBspLump`
+   - allowed implicit changes (e.g. `bsp.VERTICES[0].z += 1`)
+   - `__iter__` doesn't update `_changes`, reducing unnesecary caching
+   - TODO: `bsp.LUMP[::]` creates a copy & doesn't affect / share `_changes`
+   - `RawBspLump` slices are `bytearray`s
 
 ### Fixed
  * `shared.Entities` the following silent failures are now caught by the parser

bsp_tool/branches/id_software/quake.py

@@ -143,10 +143,12 @@ class ClipNode(base.Struct):  # LUMP 9
 
 
 class Edge(list):  # LUMP 12
+    # TODO: replace w/ MappedArray(_mapping=2)
     _format = "2H"  # List[int]
 
+    # hacky methods for working with other systems
     def as_tuple(self):
-        return self  # HACK
+        return self
 
     @classmethod
     def from_tuple(cls, _tuple):

bsp_tool/lumps.py

@@ -15,7 +15,7 @@
 Stream = Union[io.BufferedReader, io.BytesIO]
 
 
-def _remap_negative_index(index: int, length: int) -> int:
+def _remap_index(index: int, length: int) -> int:
     """simplify to positive integer"""
     if index < 0:
         index = length + index
@@ -24,13 +24,13 @@ def _remap_negative_index(index: int, length: int) -> int:
     return index
 
 
-def _remap_slice(_slice: slice, length: int) -> slice:
+def _remap_slice_to_range(_slice: slice, length: int) -> slice:
     """simplify to positive start & stop within range(0, length)"""
     start, stop, step = _slice.start, _slice.stop, _slice.step
     start = 0 if (start is None) else max(length + start, 0) if (start < 0) else length if (start > length) else start
     stop = length if (stop is None or stop > length) else max(length + stop, 0) if (stop < 0) else stop
     step = 1 if (step is None) else step
-    return slice(start, stop, step)
+    return range(start, stop, step)
 
 
 def decompress_valve_LZMA(data: bytes) -> bytes:
@@ -109,31 +109,49 @@ def from_header(cls, stream: Stream, lump_header: LumpHeader):
     def __repr__(self):
         return f"<{self.__class__.__name__}; {len(self)} bytes at 0x{id(self):016X}>"
 
-    # NOTE: no __delitem__
+    def __delitem__(self, index: Union[int, slice]):
+        if isinstance(index, int):
+            index = _remap_index(index, self._length)
+            self[index:] = self[index + 1:]
+            # NOTE: slice assignment will change length
+        elif isinstance(index, slice):
+            for i in _remap_slice_to_range(index, self._length):
+                del self[i]
+        else:
+            raise TypeError(f"list indices must be integers or slices, not {type(index)}")
+
+    def get(self, index: int, mutable: bool = True) -> int:
+        # NOTE: don't use get! we can't be sure the given index in bounds
+        if index in self._changes:
+            return self._changes[index]
+        else:
+            value = self.get_unchanged(index)
+            if mutable:
+                self._changes[index] = value
+            return value
 
-    def __getitem__(self, index: Union[int, slice]) -> bytes:
+    def get_unchanged(self, index: int) -> int:
+        """no index remapping, be sure to respect stream data bounds!"""
+        self.stream.seek(self.offset + index)
+        return self.stream.read(1)[0]
+
+    def __getitem__(self, index: Union[int, slice]) -> Union[int, bytearray]:
         """Reads bytes from the start of the lump"""
         if isinstance(index, int):
-            index = _remap_negative_index(index, self._length)
-            if index in self._changes:
-                return self._changes[index]
-            else:
-                self.stream.seek(self.offset + index)
-                return self.stream.read(1)[0]  # return 1 0-255 integer, matching bytes behaviour
+            return self.get(_remap_index(index, self._length))
         elif isinstance(index, slice):
-            _slice = _remap_slice(index, self._length)
-            return bytes([self[i] for i in range(_slice.start, _slice.stop, _slice.step)])
+            # TODO: BspLump[::] returns a copy (doesn't update _changes)
+            return bytearray([self[i] for i in _remap_slice_to_range(index, self._length)])
         else:
             raise TypeError(f"list indices must be integers or slices, not {type(index)}")
 
     def __setitem__(self, index: int | slice, value: Any):
         """remapping slices is allowed, but only slices"""
         if isinstance(index, int):
-            index = _remap_negative_index(index, self._length)
+            index = _remap_index(index, self._length)
             self._changes[index] = value
         elif isinstance(index, slice):
-            _slice = _remap_slice(index, self._length)
-            slice_indices = list(range(_slice.start, _slice.stop, _slice.step))
+            slice_indices = list(_remap_slice_to_range(index, self._length))
             length_change = len(list(value)) - len(slice_indices)
             slice_changes = dict(zip(slice_indices, value))
             if length_change == 0:  # replace a slice with an equal length slice
@@ -150,7 +168,7 @@ def __setitem__(self, index: int | slice, value: Any):
             raise TypeError(f"list indices must be integers or slices, not {type(index)}")
 
     def __iter__(self):
-        return iter([self[i] for i in range(self._length)])
+        return iter([self.get(i, mutable=False) for i in range(self._length)])
 
     def __len__(self):
         return self._length
@@ -220,33 +238,19 @@ def from_header(cls, stream: Stream, lump_header: LumpHeader, LumpClass: object)
     def __repr__(self):
         return f"<{self.__class__.__name__}({len(self)} {self.LumpClass.__name__}) at 0x{id(self):016X}>"
 
-    def __delitem__(self, index: Union[int, slice]):
-        if isinstance(index, int):
-            index = _remap_negative_index(index, self._length)
-            self[index:] = self[index + 1:]
-        elif isinstance(index, slice):
-            _slice = _remap_slice(index, self._length)
-            for i in range(_slice.start, _slice.stop, _slice.step):
-                del self[i]
-        else:
-            raise TypeError(f"list indices must be integers or slices, not {type(index)}")
+    def get_unchanged(self, index: int) -> int:
+        """no index remapping, be sure to respect stream data bounds!"""
+        # NOTE: no .from_stream(); BasicLumpClasses only specify _format
+        self.stream.seek(self.offset + (index * self._entry_size))
+        raw_entry = struct.unpack(self.LumpClass._format, self.stream.read(self._entry_size))
+        return self.LumpClass(raw_entry[0])
 
     def __getitem__(self, index: Union[int, slice]):
         """Reads bytes from self.stream & returns LumpClass(es)"""
-        # read bytes -> struct.unpack tuples -> LumpClass
-        # NOTE: BspLump[index] = LumpClass(entry)
         if isinstance(index, int):
-            index = _remap_negative_index(index, self._length)
-            self.stream.seek(self.offset + (index * self._entry_size))
-            raw_entry = struct.unpack(self.LumpClass._format, self.stream.read(self._entry_size))
-            # NOTE: only the following line has changed
-            return self.LumpClass(raw_entry[0])
+            return self.get(_remap_index(index, self._length))
         elif isinstance(index, slice):
-            _slice = _remap_slice(index, self._length)
-            out = list()
-            for i in range(_slice.start, _slice.stop, _slice.step):
-                out.append(self[i])
-            return out
+            return [self[i] for i in _remap_slice_to_range(index, self._length)]
         else:
             raise TypeError(f"list indices must be integers or slices, not {type(index)}")
 
@@ -262,24 +266,14 @@ class BspLump(BasicBspLump):
     _entry_size: int  # sizeof(LumpClass)
     _length: int  # number of indexable entries
 
-    def __getitem__(self, index: Union[int, slice]):
-        """Reads bytes from self.stream & returns LumpClass(es)"""
-        if isinstance(index, int):
-            index = _remap_negative_index(index, self._length)
-            if index in self._changes:
-                return self._changes[index]
-            else:
-                self.stream.seek(self.offset + (index * self._entry_size))
-                _tuple = struct.unpack(self.LumpClass._format, self.stream.read(self._entry_size))
-                return self.LumpClass.from_tuple(_tuple)
-        elif isinstance(index, slice):  # LAZY HACK
-            _slice = _remap_slice(index, self._length)
-            out = list()
-            for i in range(_slice.start, _slice.stop, _slice.step):
-                out.append(self[i])
-            return out
-        else:
-            raise TypeError(f"list indices must be integers or slices, not {type(index)}")
+    def get_unchanged(self, index: int) -> int:
+        """no index remapping, be sure to respect stream data bounds!"""
+        self.stream.seek(self.offset + (index * self._entry_size))
+        # BROKEN: quake.Edge does not support .from_stream()
+        # return self.LumpClass.from_stream(self.stream)
+        # HACK: required for quake.Edge
+        _tuple = struct.unpack(self.LumpClass._format, self.stream.read(self._entry_size))
+        return self.LumpClass.from_tuple(_tuple)
 
     def search(self, **kwargs):
         """Returns all lump entries which have the queried values [e.g. find(x=0)]"""

tests/branches/valve/test_orange_box.py

@@ -9,9 +9,11 @@
 
 
 class TestMethods:
+    displacement_bsps = [b for b in bsps if b.headers["DISPLACEMENT_INFO"].length != 0]
+
     # TODO: def test_vertices_of_face(bsp: ValveBsp):
 
-    @pytest.mark.parametrize("bsp", bsps, ids=[b.filename for b in bsps])
+    @pytest.mark.parametrize("bsp", displacement_bsps, ids=[b.filename for b in displacement_bsps])
     def test_vertices_of_displacement(self, bsp: ValveBsp):
         for disp_info in getattr(bsp, "DISPLACEMENT_INFO", list()):
             face_index = disp_info.face