Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

save_as() method for every BspClass #15

Open
3 of 14 tasks
snake-biscuits opened this issue Aug 19, 2021 · 10 comments
Open
3 of 14 tasks

save_as() method for every BspClass #15

snake-biscuits opened this issue Aug 19, 2021 · 10 comments
Assignees
Labels
enhancement New feature or request need test We need to write a test for regression testing slow burn lots of work & will take a long time
Milestone

Comments

@snake-biscuits
Copy link
Owner

snake-biscuits commented Aug 19, 2021

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

  • IdTechBsp
    • Genesis3DBsp
    • InfinityWardBsp
      • D3DBsp
    • RavenBsp
    • RitualBsp
  • QuakeBsp
    • GoldSrcBsp
    • ReMakeQuakeBsp
      • Quake64Bsp
  • ValveBsp
    • RespawnBsp

Further TODOS

@snake-biscuits snake-biscuits added the enhancement New feature or request label Aug 19, 2021
@snake-biscuits snake-biscuits added this to the v0.3.0 milestone Aug 19, 2021
@snake-biscuits snake-biscuits self-assigned this Aug 19, 2021
@snake-biscuits
Copy link
Owner Author

snake-biscuits commented Dec 20, 2021

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

snake-biscuits added a commit that referenced this issue Feb 5, 2022
tested on Team Fortress 2/test2.bsp
should work on all Source Engine titles
@snake-biscuits
Copy link
Owner Author

snake-biscuits commented Feb 5, 2022

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
Copy link
Owner Author

snake-biscuits commented Feb 5, 2022

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
Copy link
Owner Author

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 snake-biscuits pinned this issue Oct 11, 2022
@snake-biscuits snake-biscuits moved this from Todo: Research to Todo: Code in bsp_tool Core Functionality Oct 11, 2022
@snake-biscuits snake-biscuits added the slow burn lots of work & will take a long time label Dec 17, 2022
@snake-biscuits snake-biscuits modified the milestones: v0.4.0, v0.5.0 Jan 25, 2023
@snake-biscuits snake-biscuits modified the milestones: v0.5.0, v1.0.0 Mar 17, 2023
@snake-biscuits snake-biscuits changed the title Save_as method for all .bsps .save_as() method for every branch Apr 17, 2023
@snake-biscuits snake-biscuits changed the title .save_as() method for every branch save_as() method for every BspClass Apr 17, 2023
@snake-biscuits snake-biscuits unpinned this issue Apr 24, 2023
@snake-biscuits
Copy link
Owner Author

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

@snake-biscuits
Copy link
Owner Author

#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
Copy link
Owner Author

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
Copy link
Owner Author

#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 snake-biscuits added the need test We need to write a test for regression testing label Aug 24, 2023
@snake-biscuits
Copy link
Owner Author

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
Copy link
Owner Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request need test We need to write a test for regression testing slow burn lots of work & will take a long time
Projects
Status: Todo: Code
Development

No branches or pull requests

1 participant