[PATCH v2] d3d10: Implement ID3D10EffectScalarVariable::{Get, Set}{Int, Float, Bool}

[Nick Renieris]

On Wed, Mar 13, 2019, 10:15 AM Józef Kucia <joseph.kucia at 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 at 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.