nvs / map

Warcraft 3 map management tools
MIT License
18 stars 6 forks source link

Revamp `unpack` to use `position` #99

Open nvs opened 2 years ago

nvs commented 2 years ago

Basically, let unpack in the various file formats take an additional position argument like string.unpack does. They also need to return the updated position as well. The assertion will be removed. The intention of that particular assert was to catch updates to file formats or other issues. Unit testing should probably be implemented to catch those issues (cough #53).

nvs commented 2 years ago

This issue probably does not need to be addressed. All of the supported file formats are processed as full files. The exception, and what prompted consideration of this change, is W3O support. Its format is a bunch of object modification files shoved together.

Instead of adjusting every file format to work in a way that will never be needed, it is probably best to do one of the following: