snake-biscuits / bsp_tool

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

Can't peek lumps with dot notation #69

Closed kristiker closed 1 year ago

kristiker commented 1 year ago

Shouldn't use setattr imo, as there is no type hinting on what sorts of lumps a bsp contains. https://github.com/snake-biscuits/bsp_tool/blob/2a3e1e7d10a6d94c7686b2799d3c7c2db898b4b6/bsp_tool/valve.py#L57

snake-biscuits commented 1 year ago

Can't believe you went and just took #69 like that.

Looking here I assume you're using the last PyPI build, which is far behind the current system for lump loading I genuinely don't remember how 0.3.1 works, so much has changed since then source1import looks like a really cool project tho, and I don't get many use-cases like that so, I'll try and explain the current system of how lumps work here:

BspLump comes in 5 flavours (x2 for ExternalBspLump, which are near identical): 1) RawBspLump emulates a bytestring

The reason for this approach is everything is loaded dynamically at runtime, but I also intend to allow adding you own lump data at runtime

If a lump fails to load, the error will be added to bsp.loading_errors, you can raise bsp.loading_errors["LUMP_NAME"]

Actually, that isn't very clear to users, instead:

The definitions for each lump type appear at the bottom of each branch_script (wiki) since branch_script is a variable passed in at runtime lump types can be literally anything, Users can write their own branch scripts after all You can also get a list of all lump types from the docs:

The docs lists don't give you the lump type (BspLump, BasicBspLump, SpecialLumpClass), except for GameLump.

To be fair, this is one of the most important parts of usingbsp_tool and it's poorly documented BspLump types are also largely a complex way to provide a list-like interface

IIRC in most implementations I use hasattr & isinstance to ensure lumps are available when it comes up but I mostly assume the data is there and the type I expect, which isn't great

If you're expecting to get type hints in an IDE I can't really help there, the type hints are just hints, not rules Right now the approach that makes sense to me

Btw you can see other code that hooks bsp_tool here:

This is all super unclear to the beginner so I can understand wanting a different system However, changing the way lumps load again would take months & might not be worth it imo

For now reading the docs is the best solution I can offer (and I should definitely make the docs clearer) If you have an suggestions for smaller changes such as:

I can look into those because they're good Quality of Life changes that don't require restructuring the way the whole tool works

TL;DR: Type hinting dynamic objects is hard, read the docs (I should write better docs)

kristiker commented 1 year ago

Ah, I see its due to the modularity, and that the available lumps are defined in bsp.branch.LUMP. Thanks.