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

Nick Renieris velocityra at gmail.com
Tue Mar 12 16:59:59 CDT 2019


Hi,

Στις Τρί, 12 Μαρ 2019 στις 1:25 μ.μ., ο/η Henri Verbeet
<hverbeet at gmail.com> έγραψε:
> On Mon, 11 Mar 2019 at 19:40, Nick Renieris <velocityra at gmail.com> wrote:
> > +struct d3d10_effect_scalar_variable
> > +{
> > +    union
> > +    {
> > +        int i;
> > +        float f;
> > +        BOOL b;
> > +    };
> > +};
> Please avoid nameless unions.

Ok, I'll make `d3d10_effect_scalar_variable` a union.

> >  static HRESULT STDMETHODCALLTYPE d3d10_effect_scalar_variable_SetFloat(ID3D10EffectScalarVariable *iface,
> >          float value)
> >  {
> > -    FIXME("iface %p, value %.8e stub!\n", iface, value);
> > +    struct d3d10_effect_variable *This;
> >
> I know a bunch of other code in the same file also does this, but
> please don't name variables "This". This may be one of those rare
> cases where "variable" would be a perfectly reasonable name, or
> perhaps simply "v".

"This" is indeed used everywhere. Can you explain what's different in my case?

> 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?

Thanks,
Nick.



More information about the wine-devel mailing list