diff --git a/CHANGELOG.md b/CHANGELOG.md index bef4bb34..000415f8 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/bsp_tool/branches/id_software/quake.py b/bsp_tool/branches/id_software/quake.py index 68ceb94d..cb80873f 100644 --- a/bsp_tool/branches/id_software/quake.py +++ b/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): diff --git a/bsp_tool/lumps.py b/bsp_tool/lumps.py index f42dfd94..9ba28445 100644 --- a/bsp_tool/lumps.py +++ b/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)]""" diff --git a/tests/branches/valve/test_orange_box.py b/tests/branches/valve/test_orange_box.py index 414fea16..d5ff4e03 100644 --- a/tests/branches/valve/test_orange_box.py +++ b/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 diff --git a/tests/test_BspLump.py b/tests/test_BspLump.py index 22f6733d..d17ae9b8 100644 --- a/tests/test_BspLump.py +++ b/tests/test_BspLump.py @@ -1,15 +1,16 @@ -import pytest +from . import utils -from bsp_tool import load_bsp, lumps +from bsp_tool import lumps +from bsp_tool import ValveBsp, IdTechBsp from bsp_tool.branches.id_software import quake, quake3 +from bsp_tool.branches.valve import orange_box + +import pytest # TODO: collect valid lumps of desired type for each test, rather than hardcoded lump names -global bsps -bsps = {"q3_lobby": load_bsp("tests/maps/Quake 3 Arena/mp_lobby.bsp"), - "tf2_test2": load_bsp("tests/maps/Team Fortress 2/test2.bsp"), - "tf2_test_displacement_decompile": load_bsp("tests/maps/Team Fortress 2/test_displacement_decompile.bsp"), - "tf2_test_physcollide": load_bsp("tests/maps/Team Fortress 2/test_physcollide.bsp")} +bsps = [*utils.get_test_maps(ValveBsp, {orange_box: ["Team Fortress 2"]}), + *utils.get_test_maps(IdTechBsp, {quake3: ["Quake 3 Arena"]})] def raw_lump_of(bsp) -> lumps.RawBspLump: @@ -22,72 +23,66 @@ def raw_lump_of(bsp) -> lumps.RawBspLump: class TestRawBspLump: - raw_lumps = list(map(raw_lump_of, bsps.values())) - - def test_its_raw(self): - for lump in self.raw_lumps: - assert isinstance(lump, lumps.RawBspLump), f"it's not raw! it's {type(lump)}" - - def test_list_conversion(self): - for map_name, lump in zip(bsps, self.raw_lumps): - assert list(lump) == [int(b) for b in lump], f"{map_name} failed" - - def test_indexing(self): - for map_name, lump in zip(bsps, self.raw_lumps): - assert isinstance(lump[0], int), f"{map_name} failed" - assert isinstance(lump[:1], bytes), f"{map_name} failed" - assert len(lump[:1]) == 1, f"{map_name} failed" - # ^ all three just look at the first byte - # expecting behaviour the same as if lump was a bytestring - assert len(lump[-2:]) == 2, f"{map_name} failed" - with pytest.raises(TypeError): - assert lump["one"], f"{map_name} failed" + raw_lumps = list(map(raw_lump_of, bsps)) + @pytest.mark.parametrize("raw_lump", raw_lumps, ids=[b.filename for b in bsps]) + def test_its_raw(self, raw_lump): + assert isinstance(raw_lump, lumps.RawBspLump) -class TestBspLump: - def test_list_conversion(self): - for map_name in bsps: - lump = bsps[map_name].VERTICES - assert list(lump) == [b for b in lump], f"{map_name}.VERTICES failed" + @pytest.mark.parametrize("raw_lump", raw_lumps, ids=[b.filename for b in bsps]) + def test_list_conversion(self, raw_lump): + assert list(raw_lump) == [int(b) for b in raw_lump] - def test_indexing(self): - for map_name in bsps: - lump = bsps[map_name].VERTICES - LumpClass = quake.Vertex if map_name != "q3_lobby" else quake3.Vertex - assert isinstance(lump[0], LumpClass), f"{map_name} failed" - assert isinstance(lump[:1], list), f"{map_name} failed" - assert len(lump[:1]) == 1, f"{map_name} failed" - # ^ all three just look at the first byte - # expecting behaviour the same as if lump was a bytestring - assert len(lump[-2:]) == 2, f"{map_name} failed" - # TODO: check negative indices line up [_remap_negative_index] - # TODO: check slice cases (negative step, wide step, invalid slice) [_remap_slice] - with pytest.raises(TypeError): - assert lump["one"], f"{map_name} failed" + @pytest.mark.parametrize("raw_lump", raw_lumps, ids=[b.filename for b in bsps]) + def test_indexing(self, raw_lump): + assert isinstance(raw_lump[0], int) + assert isinstance(raw_lump[:1], bytearray) + assert len(raw_lump[:1]) == 1 + assert len(raw_lump[-2:]) == 2 + with pytest.raises(TypeError): + assert raw_lump["one"] - def test_del(self): - for map_name in bsps: - lump = bsps[map_name].VERTICES - initial_length = len(lump) - del lump[0] - assert len(lump) == initial_length - 1, f"{map_name} failed" - initial_length = len(lump) - del lump[:2] - assert len(lump) == initial_length - 2, f"{map_name} failed" - - def test_setitem(self): - for map_name in bsps: - lump = bsps[map_name].VERTICES - empty_entry = lump.LumpClass() - lump[0] = empty_entry - assert lump[0] == empty_entry, f"{map_name} failed" - lump[:2] = [empty_entry, empty_entry] - assert lump[:2] == [empty_entry, empty_entry], f"{map_name} failed" - # TODO: allow for insert via slice & test for this - # TODO: test changes to object attrs for Issue #23 - # -- e.g. bsp.LUMP[index].attr = val (uses soft copies) - # TODO: test_iadd (__iadd__ method; overrides +=) +class TestBspLump: + @pytest.mark.parametrize("bsp", bsps, ids=[b.filename for b in bsps]) + def test_list_conversion(self, bsp): + lump = bsp.VERTICES + assert list(lump) == [b for b in lump] + + @pytest.mark.parametrize("bsp", bsps, ids=[b.filename for b in bsps]) + def test_indexing(self, bsp): + lump = bsp.VERTICES + LumpClass = quake.Vertex if bsp.branch != quake3 else quake3.Vertex + assert isinstance(lump[0], LumpClass) + assert isinstance(lump[:1], list) + assert len(lump[:1]) == 1 + assert len(lump[-2:]) == 2 + with pytest.raises(TypeError): + assert lump["one"] + + @pytest.mark.parametrize("bsp", bsps, ids=[b.filename for b in bsps]) + def test_del(self, bsp): + lump = bsp.VERTICES + initial_length = len(lump) + # delete single index + del lump[0] + assert len(lump) == initial_length - 1 + # delete slice + initial_length = len(lump) + del lump[:2] + assert len(lump) == initial_length - 2 + + @pytest.mark.parametrize("bsp", bsps, ids=[b.filename for b in bsps]) + def test_setitem(self, bsp): + lump = bsp.VERTICES + empty_entry = lump.LumpClass() + lump[0] = empty_entry + assert lump[0] == empty_entry + lump[:2] = [empty_entry, empty_entry] + assert lump[:2] == [empty_entry, empty_entry] + # TODO: allow for insert via slice & test for this + # TODO: test changes to object attrs for Issue #23 + # -- e.g. bsp.LUMP[index].attr = val (uses soft copies) class TestExternalBspLump: # TODO: ship bespoke RespawnBsp .bsp_lump files with tests @@ -95,29 +90,26 @@ class TestExternalBspLump: # TODO: ship bespoke RespawnBsp .bsp_lump files with class TestBasicBspLump: - def test_its_basic(self): - for map_name in bsps: - lump = bsps[map_name].LEAF_FACES - assert isinstance(lump, lumps.BasicBspLump), f"it's not basic! it's {type(lump)}" - - def test_list_conversion(self): - for map_name in bsps: - lump = bsps[map_name].LEAF_FACES - assert list(lump) == [b for b in lump], f"{map_name} vis lump does not match" - - def test_indexing(self): + @pytest.mark.parametrize("bsp", bsps, ids=[b.filename for b in bsps]) + def test_its_basic(self, bsp): + lump = bsp.LEAF_FACES + assert isinstance(lump, lumps.BasicBspLump), type(lump) + + @pytest.mark.parametrize("bsp", bsps, ids=[b.filename for b in bsps]) + def test_list_conversion(self, bsp): + lump = bsp.LEAF_FACES + assert list(lump) == [b for b in lump] + + @pytest.mark.parametrize("bsp", bsps, ids=[b.filename for b in bsps]) + def test_indexing(self, bsp): for map_name in bsps: - lump = bsps[map_name].LEAF_FACES - LumpClass = bsps[map_name].branch.BASIC_LUMP_CLASSES["LEAF_FACES"] + lump = bsp.LEAF_FACES + LumpClass = bsp.branch.BASIC_LUMP_CLASSES["LEAF_FACES"] if isinstance(LumpClass, dict): # ValveBsp branches use dicts for multiple lump versions - LumpClass = list(LumpClass.values())[0] - assert isinstance(lump[0], LumpClass), f"{map_name} failed" - assert isinstance(lump[:1], list), f"{map_name} failed" - assert len(lump[:1]) == 1, f"{map_name} failed" - # ^ all three just look at the first byte - # expecting behaviour the same as if lump was a bytestring - assert len(lump[-2:]) == 2, f"{map_name} failed" - # TODO: check negative indices line up - # TODO: check slice cases (negative step, wide step, invalid slice) + LumpClass = LumpClass[bsp.headers["LEAF_FACES"].version] + assert isinstance(lump[0], LumpClass) + assert isinstance(lump[:1], list) + assert len(lump[:1]) == 1 + assert len(lump[-2:]) == 2 with pytest.raises(TypeError): - assert lump["one"], f"{map_name} failed" + assert lump["one"] diff --git a/tests/test_lumps.py b/tests/test_lumps.py index a12add36..08f9ccf4 100644 --- a/tests/test_lumps.py +++ b/tests/test_lumps.py @@ -7,28 +7,32 @@ import pytest +# TODO: more mutability & _changes testing +# TODO: merge with tests/test_BspLump.py (or divide concerns more clearly) + + class TestRemapNegativeIndex: def test_guess(self): - assert lumps._remap_negative_index(-1, 50) == 49 + assert lumps._remap_index(-1, 50) == 49 def test_range(self): arr = [*range(128)] for i in range(-128, 0): - positive_i = lumps._remap_negative_index(i, 128) + positive_i = lumps._remap_index(i, 128) assert arr[i] == arr[positive_i] def test_negative_out_of_range(self): with pytest.raises(IndexError): - [*range(8)][lumps._remap_negative_index(-16, 8)] + [*range(8)][lumps._remap_index(-16, 8)] -class TestRemapSlice: +class TestRemapSliceToRange: def test_lazy(self): - assert lumps._remap_slice(slice(None, None, None), 50) == slice(0, 50, 1) - assert lumps._remap_slice(slice(0, None, None), 50) == slice(0, 50, 1) - assert lumps._remap_slice(slice(0, 50, None), 50) == slice(0, 50, 1) - assert lumps._remap_slice(slice(0, 50, 1), 50) == slice(0, 50, 1) - assert lumps._remap_slice(slice(0, 69, 1), 50) == slice(0, 50, 1) + assert lumps._remap_slice_to_range(slice(None, None, None), 50) == range(0, 50, 1) + assert lumps._remap_slice_to_range(slice(0, None, None), 50) == range(0, 50, 1) + assert lumps._remap_slice_to_range(slice(0, 50, None), 50) == range(0, 50, 1) + assert lumps._remap_slice_to_range(slice(0, 50, 1), 50) == range(0, 50, 1) + assert lumps._remap_slice_to_range(slice(0, 69, 1), 50) == range(0, 50, 1) class TestDecompress: @@ -47,18 +51,10 @@ class LumpClass_basic(base.MappedArray): class TestRawBspLump: """test the changes system & bytearray-like behaviour""" # TODO: tests to ensure RawBspLump behaves like a bytearray - def test_concat(self): - header = LumpHeader_basic(offset=0, length=8) - stream = io.BytesIO(bytes([*range(8)])) - lump = lumps.RawBspLump.from_header(stream, header) - lump.extend(bytes([*range(8, 16)])) - assert len(lump) == 16 - for i, x in enumerate(lump): - assert i == x + ... class TestBspLump: - @pytest.mark.xfail(reason="not yet implemented") def test_implicit_change(self): header = LumpHeader_basic(offset=0, length=6) stream = io.BytesIO(b"\x01\x00\x02\x00\x03\x00")