snake-biscuits / bsp_tool

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

save_as() method for every BspClass #15

Open snake-biscuits opened 3 years ago

snake-biscuits commented 3 years ago

Any changes made to a .bsp should be saved back to that .bsp by bsp_tool The user shouldn't have to worry about specifics like lump containing offsets However confirming the saved map is valid will not be a feature at this time (could use an optional branch script method as a fuzzy validator for engine limit checks etc.)

NOTE: this means Quake / GoldSrc lumps like MIP_TEXTURES need to be handled with SpecialLumpClasses or maybe even a new class in the lumps module

TRACKER: base.Bsp subclasses with a functional & tested .save_as() method

Further TODOS

snake-biscuits commented 2 years ago

Titanfall2 / Apex Legends .save_as doesn’t respect external lumps, see #12 for more

snake-biscuits commented 2 years ago

Titanfall2 / Apex Legends .save_as doesn’t respect external lumps, see #12 for more

This has been addressed now, but we do need automated testing to ensure this remains stable.

All an automated test needs to confirm (for now) is that loading and saving a .bsp matches the source file byte-for-byte

snake-biscuits commented 2 years ago

All base.Bsp subclasses with their unique _preload methods will need their own implementation of the save_as method Inheritance should cover any .bsp this doesn't apply to (e.g. GoldSrcBsp inherits _preload from QuakeBsp)

snake-biscuits commented 2 years ago

All base.Bsp subclasses with their unique _preload methods will need their own implementation of the save_as method

The top comment tracks each base.Bsp subclass this applies to, but their subclasses should also be tested to be sure For ensuring save_as remains stable we will need enough maps for each branch_script to test the contents of every lump Any edge cases that arise in SpecialLumpClasses etc. will also need to be permanently tracked with a map & test for that issue

During the initial development of each .save_as method we should also confirm each map against at least 1 game (test it runs) Occasionally doing this with test maps going forward would also be useful. Keeping a mental image of how the map should function will hopefully limit "coding in the dark"

snake-biscuits commented 1 year ago

Automated testing for .as_bytes() methods will be a big help here Probably something to implement in branch script testing

snake-biscuits commented 1 year ago

112 shows that we need to test all .as_bytes() methods, specifically for SpecialLumpClasses

Explicitly! Not just indirect testing through .save_as()

Building tests to support .save_as() is bringing out the deep code coverage Which is great because it means assumptions are getting drawn out & confronted

snake-biscuits commented 1 year ago

tests/test_save.py needs to be content aware, otherwise saves must be byte-for-byte

refactors to extensions.diff should now cover this still need some specialised classes for diffing SpecialLumpClasses but the refactor has standardised how to do that and is much more modular now

snake-biscuits commented 1 year ago

143 found some bugs w/ RespawnBsp.save_as()

Going to open a new Issue for testing all RespawnBsp's cases in a bit

snake-biscuits commented 5 months ago

IdTechBsp (& by extensions QbismBsp) should include BSPX data All BspClasses should be appending their ._tail(), but BSPX is padded

We might need to categorise other tails to see if they need padding too

>>> import bsp_tool, os, fnmatch
>>> md = "E:/Mod/QuakeII/rerelease/pak0/maps/"
>>> maps = {m[:-4]: bsp_tool.load_bsp(os.path.join(md, m)) for m in fnmatch.filter(os.listdir(md), "*.bsp")}
>>> subfolders = {d for d in os.listdir(md) if os.path.isdir(os.path.join(md, d))}
>>> maps.update({
...     f"{sd}/{m}"[:-4]: bsp_tool.load_bsp(os.path.join(md, sd, m))
...     for sd in subfolders
...     for m in fnmatch.filter(os.listdir(os.path.join(md, sd)), "*.bsp")})
... 
>>> bspx_maps = {m: b for m, b in maps.items() if b"BSPX" in b._tail()}
>>> {b.__class__ for b in bspx_maps.values()}
{bsp_tool.id_software.IdTechBsp, bsp_tool.id_software.QbismBsp}

Related

snake-biscuits commented 3 months ago

tests/test_save.py tests are all muted & XFAIL atm

you can get XFAIL details w/ $ pytest --runxfail

This is because extensions.diff hasn't been hooked up to make errors coherent