dust-engine / dot_vox

Rust parser for MagicaVoxel .vox files.
MIT License
51 stars 20 forks source link

add pallete support #3

Closed jice-nospam closed 7 years ago

davidedmonds commented 7 years ago

Thanks very much for the pull request! Unfortunately, this looks like it breaks a couple of my existing tests - most likely down to me using files that don't have valid palletes. If you get a chance to take a look at making the tests pass, that'll be great, otherwise I'll take a look at it in a day or two.

Thanks again!

jice-nospam commented 7 years ago

Hey,

ok I'll have a look at the tests. I wasn't sure they were working before my change. I should have checked it before changing anything :)

2017-09-04 8:42 GMT+02:00 David Edmonds notifications@github.com:

Thanks very much for the pull request! Unfortunately, this looks like it breaks a couple of my existing tests - most likely down to me using files that don't have valid palletes. If you get a chance to take a look at making the tests pass, that'll be great, otherwise I'll take a look at it in a day or two.

Thanks again!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/davidedmonds/dot_vox/pull/3#issuecomment-326878429, or mute the thread https://github.com/notifications/unsubscribe-auth/AHcbDyzM6xEcmXpG8NGuKvvYXOD0Q5y_ks5se5vQgaJpZM4PLJg3 .

jice-nospam commented 7 years ago

Ok so it breaks parsing of files that have no RGBA chunk. For me the right implementation would be to have an optional RGBA chunk :

pallete: switch!(opt!( take!(4) ), Some(b"RGBA") => call!(parsepallete) | => value!(Vec::::new()) ) >>

but for some reason, this does not compile. I'm new to nom so it might be something obvious.

Compiling dot_vox v0.3.0 error[E0282]: type annotations needed --> src\lib.rs:129:1 129 / named!(parse_vox_file <&[u8], DotVoxData>, do_parse!( 130 tag!(MAGIC_NUMBER) >> 131 version: take_u32 >> 132 take!(12) >> ... 145 }) 146 )); ___^ cannot infer type for E
= note: this error originates in a macro outside of the current crate

error: aborting due to previous error(s)

2017-09-04 9:30 GMT+02:00 jice jice.nospam@gmail.com:

Hey,

ok I'll have a look at the tests. I wasn't sure they were working before my change. I should have checked it before changing anything :)

2017-09-04 8:42 GMT+02:00 David Edmonds notifications@github.com:

Thanks very much for the pull request! Unfortunately, this looks like it breaks a couple of my existing tests - most likely down to me using files that don't have valid palletes. If you get a chance to take a look at making the tests pass, that'll be great, otherwise I'll take a look at it in a day or two.

Thanks again!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/davidedmonds/dot_vox/pull/3#issuecomment-326878429, or mute the thread https://github.com/notifications/unsubscribe-auth/AHcbDyzM6xEcmXpG8NGuKvvYXOD0Q5y_ks5se5vQgaJpZM4PLJg3 .

davidedmonds commented 7 years ago

Closing in favour of https://github.com/davidedmonds/dot_vox/pull/4, which contains this commit, some further test cases, and does a little refactoring.

davidedmonds commented 7 years ago

4 was merged, and published to crates.io as dot_vox 0.4.0.

Thanks again for your contribution!