search

flowtsohg / mdx-m3-viewer

A WebGL viewer for MDX and M3 files used by the games Warcraft 3 and Starcraft 2 respectively.
MIT License
132 stars 47 forks source link

UnknownChunk.writeMdx: this.chunk.byteLength -> this.getByteLength() ? #41

Closed EzraZebra closed 4 years ago

EzraZebra commented 4 years ago

As in the title, shouldn't this.getByteLength() be used here instead? As far as I can tell, this function isn't used anywhere right now.

https://github.com/flowtsohg/mdx-m3-viewer/blob/3c94d049d76ee6f15d938f3020ccd777b66ce83d/src/parsers/mdlx/unknownchunk.ts#L17

EzraZebra commented 4 years ago

Btw, hope I'm not bothering you here. I'm reimplementing the Mdlx parser into C++. The MDX part is functional and pretty much done. If you're interested, source code is on GitLab.

flowtsohg commented 4 years ago

getByteLength() here is the total size of the chunk including the header, while the size the header needs is only of the data itself. The former is used to calculate the final size of the MDX file when writing - https://github.com/flowtsohg/mdx-m3-viewer/blob/3c94d049d76ee6f15d938f3020ccd777b66ce83d/src/parsers/mdlx/model.ts#L617 Perhaps I should have used better naming.

I did actually write a C++ copy of the MDLX parser a while ago. Even had nearly the same syntax with block-reading iterators, but it was somewhat ugly so I changed to ye-olde normal while loops. Wouldn't care to share it if you're interested, it also includes BLP/TGA/INI/SLK/MPQ/CASC to some extent or another.

EzraZebra commented 4 years ago

Ah right, of course. I got confused because you don't need the filesize when writing with std::ofstream so I didn't add a getByteLength for the Model class, and IIRC unknownchunks are the only chunks which don't use their getByteLength for anything else.

Oh I would love to see your C++ code ^^ It's probably better than mine in a lot of ways. The other parsers might also come in handy in the future, but don't really need them now.

flowtsohg commented 4 years ago

If you want the full working project I can send it to you on discord or something like that, the dependencies are a couple of tens of megabytes. I suggest you to glance at the binary stream class :P War.zip

EzraZebra commented 4 years ago

Awesome, thanks! I looked up why it's better to buffer the whole file, and yeah it's much faster xD Thanks, I'll be able to improve my code a lot :D

flowtsohg commented 4 years ago

Each has its positives, albeit when you are handling small files usually it makes sense to read it all in one go. Either way I was more looking at the fact you made separate read/write functions like I did in JS, even though C++ has the power of templates :D

EzraZebra commented 4 years ago

Yeah, I'm stealing that wholesale ;') Other than that we seem to have mostly found the same solutions :D