Re: [PATCH 1/3] d3dx9_36: Implement D3DXCreateAnimationController with a stubbed ID3DXAnimationController interface.

[Matteo Bruni]

2015-12-23 7:47 GMT+01:00 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>:
> From: Christian Costa <titan.costa(a)gmail.com>
>
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>

Hi Alistair,

some nitpicking...

Please use the "d3dx9:" prefix for this and the other patches.
> +static HRESULT WINAPI d3dx9_animation_controller_QueryInterface(ID3DXAnimationController *iface, REFIID riid, void **out)
> +{
> +    TRACE("iface %p, riid %s, out %p.\n", iface, debugstr_guid(riid), out);
> +
> +    if (IsEqualGUID(riid, &IID_IUnknown) || IsEqualGUID(riid, &IID_ID3DXAnimationController))
> +    {
> +        IUnknown_AddRef(iface);
> +        *out = iface;

I think this is usually done as "*out =
&animation->ID3DXAnimationController_iface" or similar (where
"animation" is the result of
impl_from_ID3DXAnimationController(iface)).
> +        return D3D_OK;
> +    }
> +
> +    WARN("%s not implemented, returning E_NOINTERFACE\n", debugstr_guid(riid));
> +
> +    *out = NULL;
> +    return E_NOINTERFACE;
> +}

Similarly this is usually written in a slightly different way. Please
follow the style of the other (recent) QueryInterface implementations,
we are pretty consistent now and it would be nice to preserve that
consistency.
> +static DOUBLE WINAPI d3dx9_animation_controller_GetTime(ID3DXAnimationController *iface)

Can this (and the other instances of it in the patch) simply use
"double" instead? Apparently the only difference between the two is
that DOUBLE sometimes forces alignment to 8 bytes. That should only
matter for struct fields though, as far as we are concerned, so we
should probably prefer plain double.
> +/***********************************************************************
> + *           D3DXCreateAnimationController    (D3DX9_36.@)
> + */

It doesn't really matter but this comment is unnecessary.
> +HRESULT WINAPI D3DXCreateAnimationController(UINT MaxNumAnimationOutputs, UINT MaxNumAnimationSets,
> +        UINT MaxNumTracks, UINT MaxNumEvents, ID3DXAnimationController **AnimationController)

Please use the usual "lowercase and underscores" style for the parameter names.
Also check the '*' placement a couple of lines below.