2015-12-23 7:47 GMT+01:00 Alistair Leslie-Hughes leslie_alistair@hotmail.com:
From: Christian Costa titan.costa@gmail.com
Signed-off-by: Alistair Leslie-Hughes leslie_alistair@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.