[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