DffParser: Bug fixes and improvements #20
Conversation
Co-authored-by: Timotej <20747466+Timic3@users.noreply.github.com>
Co-authored-by: Timotej <20747466+Timic3@users.noreply.github.com>
Co-authored-by: Timotej <20747466+Timic3@users.noreply.github.com>
Co-authored-by: Timotej <20747466+Timic3@users.noreply.github.com>
Co-authored-by: Timotej <20747466+Timic3@users.noreply.github.com>
Co-authored-by: Timotej <20747466+Timic3@users.noreply.github.com>
Add new readFrameData() method
Some models do not have BinMesh section at all, so we will not try to read it necessarily if it is missing.
For new version with multi-clump support
Allign reading geometry extensions.
*Fix reading empty / unknown sections while reading extension chunks. *Clean up
Major update that stabilizes DFF model parsing by handling various reading errors. Several important improvements and refinements have been implemented. Changes: * Moved FrameData reading to a separate method * Added multi-clump support * Made BinMesh optional to prevent parsing errors in certain models * Updated tests to support multi-clumps * Added export of isTriangleStrip flag for correct geometry construction * Fixed various geometry reading issues
|
Oh, sorry for the bunch of commits, I didn't know they would end up here |
Timic3
left a comment
Timic3
left a comment
There was a problem hiding this comment.
This is great, thank you! I've suggested a few format changes and addition of comments for magic numbers etc. The logic itself looks good👍.
With the output structure change, we will likely bump the package version to 2.0.0, because it introduces breaking changes.
Optionally, we can add multi clump model tests. But this is totally not necessary at the moment and can be done later on.
src/renderware/dff/DffParser.ts
Outdated
| } | ||
| const startPosition = this.getPosition(); | ||
|
|
||
| while (this.getPosition() - startPosition < clumpSize - 24) { |
There was a problem hiding this comment.
I would add a comment here explaining this. I assume 24 is the size of the clump header?
There was a problem hiding this comment.
it's a comparison that takes into account the size of two headers - the clump and the section.
It's acceptable to write it this way for clarity:
(this.getPosition() - startPosition) + 12 < clumpSize - 12
| let relativePosition = 0; | ||
| while (relativePosition < sectionSize) { | ||
| const header = this.readSectionHeader(); | ||
| relativePosition += header.sectionSize + 12; |
There was a problem hiding this comment.
I would add a comment explaining the + 12. I think the rest is pretty self-explanatory (looping over extensions).
There was a problem hiding this comment.
Yes, you are right. 12 bytes is the header size
Co-authored-by: Timotej <20747466+Timic3@users.noreply.github.com>
Co-authored-by: Timotej <20747466+Timic3@users.noreply.github.com>
Co-authored-by: Timotej <20747466+Timic3@users.noreply.github.com>
2 participants
Major update that stabilizes DFF model parsing by handling various reading errors related to different geometry section structures in models. Several important improvements and refinements have also been implemented.
Changes: