[PATCH v2] d3d10: Implement ID3D10EffectScalarVariable::{Get, Set}{Int, Float, Bool}
Henri Verbeet
hverbeet at gmail.com
Tue Mar 12 06:25:36 CDT 2019
Hi Nick,
Thank you for the patch. I do have a few comments though:
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.
> 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".
> + if (!iface)
> + return D3DERR_INVALIDCALL;
This check shouldn't be needed.
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.
More information about the wine-devel
mailing list