search

zturtleman / mm3d

Maverick Model 3D is a 3D model editor and animator for games.
https://clover.moe/mm3d
GNU General Public License v2.0
110 stars 22 forks source link

objfilter.cc maybe missing break statement. Not documented intentional (Not sure myself) #152

Closed m-7761 closed 3 years ago

m-7761 commented 3 years ago

https://github.com/zturtleman/mm3d/blob/8f121241275d3f52dcd0eaab6ef846c8c0b23ded/src/libmm3d/objfilter.cc#L284

zturtleman commented 3 years ago

It looks like there should be a break, otherwise these cases run the wrong state.

https://github.com/zturtleman/mm3d/blob/8f121241275d3f52dcd0eaab6ef846c8c0b23ded/src/libmm3d/objfilter.cc#L274-L281

It looks like . 1 will copy 1 as a decimal digit in case OFWS_AfterDecimal rather than starting a new number in case OFWS_Whitespace. (Armchair theory of course.)

m-7761 commented 3 years ago

I haven't worked with OBJ in 20 years, if you have any idea, is it possible most export jobs don't enter this case? I have exported a few OBJ files with MM3D.

zturtleman commented 3 years ago

writeStripped( "1. 1.000000" ) without break: 1. 1.000000; with break: 1. 1.0 (correctly stripped zeros).

Yeah, after farther inspection it looks like it could only go wrong in MTL map_Kd texture filename (which shouldn't be removing trailing zeros in the first place). Fixed in https://github.com/zturtleman/mm3d/compare/8f121241275d3f52dcd0eaab6ef846c8c0b23ded...7c81c59dcb5d2b640aff749fbb64a4170fc81a8d. Thanks.

m-7761 commented 3 years ago

Thanks for giving this proper attention.

FWIW looking at strdup around that change (been removing these) (a) there's a std::string version replaceSlash (I think) and (b) getRelativePath uses it, opportunistically, on both arguments (so replaceSlash is redundant here.)