On Wed, Mar 13, 2019, 10:15 AM Józef Kucia joseph.kucia@gmail.com wrote:
"This" is used in the old code. We don't use "This" anymore in D3D code.
Στις Τετ, 13 Μαρ 2019 στις 4:25 μ.μ., ο/η Henri Verbeet hverbeet@gmail.com έγραψε:
And more broadly, uppercase or mixed case variable names.
Cool. The style did seem out of place.
It would be great if you could write some tests to go along with this patch. Also, this patch implements 6 different functions; please split it so that each patch only makes a single conceptual change.
I have no issue with doing that, but is it _really_ necessary for something so simple? If so, can I split to 3 commits, grouping Set/Get functions together?
I think that 6 commits are preferred. It should result in a cleaner commit messages, i. e. "Implement ID3D10EffectScalarVariable::GetInt()."
Also, tests would be useful. Your current implementation might be incorrect.
My bad, I was only referring the commit separation comment, not the tests. I would consider even > 1 commit separation an utter waste of my time and yours. I am and have worked on many open-source projects, and never come across this. Anyway, if those are the rules, so be it.
Is it legal to call SetFloat() on boolean variables?
As far as I'm aware the tests test against hardcoded values and not real D3D dlls, so how would a test help with that, if I don't even _know_ what the real library does (and I'm not allowed to reverse it + the docs don't explicitly mention)? The only thing I can think of is to write a test against the real D3D dlls or take a leap of faith and either do nothing, or return D3DERR_INVALIDCALL.