snake-biscuits / bsp_tool

Python library for analysing .bsp files
GNU General Public License v3.0
104 stars 8 forks source link

Refactor BspLump #23

Open snake-biscuits opened 2 years ago

snake-biscuits commented 2 years ago

Editing attrs of an object returned by a BspLump don’t update BspLump.changes, need to do some soft copies probably Slice overriding & insertion don’t seem to work either. Both of these need tests and more

>>> BspLump[i].x = new_val
>>> BspLump[i].x
old_val

editing a lump should be as easy as modifying the contents of a list

snake-biscuits commented 2 years ago

Need to let copied list elements behave as if BspLump classes are real lists Changes can be made at any time to any touched return value Slices should return copies

__setitem__(self, index, value) should allow insertion when index is a slice Tracking insertions with minimal changes stored would be ideal (track insertions and offset indices for __getitem__)

each BspLump type will need a test for this, do not have RespawnBsp bsp_lumps to test Titanfall2 repo has r2 mp_lobby so use that

snake-biscuits commented 2 years ago

Changes can be made at any time to any touched return value

This wouldn't work with bytes objects, as they are immutable, but allowing changes to RawBspLump would be nice

snake-biscuits commented 1 year ago

should really address this before tackling #15 as making small edits & seeing their affects in-engine is very powerful for research but when the bsp_tool implementation differs from user expectations you're gonna see a lot of bugs & wacky workarounds

gotta love writing my own kernel to handle big data

snake-biscuits commented 1 year ago

gotta love writing my own kernel to handle big data

No, really, we need to do garbage collection of some kind here Each reference to an object collected will need to feed back into the changes attribute of the BspLump it came from

bsp.VERTICES[0].z += 1 # doesn't work rn. but should
{print(v) for v in bsp.VERTICES[:64]}  # makes a copy and deletes it within scope; no changes possible

Maybe we could override the __del__ method of each LumpClass instance to remove itself from changes if unchanged?

We would also need to accurately mimic how Python creates copies & deep-copies Ideally we hook existing Python memory management systems to handle all of this for us. Some people smarter than us may have already tackled this, idk. Could be a bunch of talks on messing with memory management & working with large files, I haven't found them yet tho.

The odds someone smarter than me has already tried this are quite high. Research time should be put into finding examples tackling this problem.

snake-biscuits commented 1 year ago

BspLumps could use a pretty serious refactor at this point tbh. They're not the list analogue I was hoping, & RawBspLump needs to be more like bytearray

In #80 & #68 I've planned to use BspLump to defer translating StaticPropClasses The best way to do this would be with an alternate __init__ (@classmethod from_struct_and_count)

Generic __init__ methods w/ alternate @classmethod .from_ __init__s are nice for testing anyway

Maybe we could even rethink inheritance & external lumps enough to simplify lumps/__init__.py down to lumps.py

snake-biscuits commented 1 year ago

Setting a read-only flag on BspLumps could be useful (checked when getting & setting) That way anything like extensions.diff / decompile, which only care about parsing can have minimal memory impact

IMO the best way to set this would be as a kwarg when opening a .bsp

bsp_tool.load_bsp("filepath/maps/filename.bsp", read_only=True)
bsp_tool.ValveBsp(bsp_tool.branches.valve.source, "filepath/maps/filename.bsp", read_only=True)

The read_only flag could lock updates to BspLump._changes (though enforcing could be difficult) Dealing with read_only changing dynamically seems pretty complicated...

But still, I would like to avoid creating entries in _changes when the user promises that they will only read Otherwise, any loop called over a lump will load the entire lump into memory in it's bloated python object form

Whatever path we take, I want to give the user some control about how much RAM they use, and have good default behaviour.

snake-biscuits commented 1 year ago

Rather than worrying about potential performance impacts, test & measure different implementations

Some manual garbage collection on cached changes could be nice, idk if automation is possible

snake-biscuits commented 1 year ago

https://github.com/snake-biscuits/bsp_tool/commit/616f08dff9b555e146c38bbf7c6254740b32f997 needs more tests

Don't 100% understand when _changes caches the bulk of a lump when we don't want it to In the case of [x for x in bsp.LUMP], mutating x shouldn't update bsp.LUMP._changes

Also undecided as to whether or not slices are copies or not Not to mention copy vs. deep copy

Behaviour should match: