Re: [RFC] gameux.dll implementation stub [try 2]
2010/6/19 Vincent Povirk madewokh...@gmail.com: I would change the second argument type of the instance creation functions to IUnknown**. When I see an LPVOID* argument on a COM-related function, I immediately expect to see a REFIID argument and a QueryInterface somewhere in the function. Actually, when I see something like LPCRAP lpSomeVar I mostly think *Please* don't do that. :-\
Re: [RFC] gameux.dll implementation stub [try 2]
I don't have as much to say this time. You should release the interfaces you create in your tests, so there are no memory leaks. I would change the second argument type of the instance creation functions to IUnknown**. When I see an LPVOID* argument on a COM-related function, I immediately expect to see a REFIID argument and a QueryInterface somewhere in the function. Really, all the caller expects is an IUnknown pointer (it takes care of querying the correct interface), so the signature should reflect that. The way the IGameExplorer and IGameExplorer2 interfaces are shared looks good to me. I would probably combine those patches and start with a stub IGameExplorer/IGameExplorer2 implementation. But it's easy to combine patches and not so easy to split them, so maybe you should err on the side of keeping things separate.
Re: gameux.dll implementation stub
Am 08.06.2010 12:56, schrieb Mariusz Pluciński: Hello As you maybe know, I'm working on implementation of Game Explorer in the Google Summer of Code. Main part of my work is now implementing gaming APIs stored in gameux.dll library. I already sent patches with headers, and I'm currently in the stage when stubs are ready to start real implementation. Unfortunately (for me) Wine is now in freeze stage, so I can't send patches with new library to wine-patches. Instead, Vincent Povirk suggested me to send them here to get feedback. It's very important to me to know is my way of implementation valid, cause changing something now is probably easier than modifying it in future. Attached patches creates initial implementation of gameux library, adds implementation of IClassFactory interface and stubs of four interfaces' implementations: IGameExplorer, IGameExplorer2, IGameStatistics and IGameStatisticsMgr. They also add initial tests for each interface. Each test now checks behavior of CoCreateInstance function while trying to create given interface. To create library, I used parts of code created by Alistair Leslie-Hughes, attached by him to bug 21261. Thanks in advance Hi Mariusz, It looks like you are mixing tabs and spaces. I guess spaces are always preferred, since tabs could be equal to 4 or 8 spaces (or any other value), which could make the code less readable. Cheers Rico
Re: gameux.dll implementation stub
Mariusz Pluciński vsha...@gmail.com wrote at Jun 8, 2010 6:56 AM Hello Greetings. As you maybe know, I'm working on implementation of Game Explorer in the Google Summer of Code. Main part of my work is now implementing gaming APIs stored in gameux.dll library. I already sent patches with headers, and I'm currently in the stage when stubs are ready to start real implementation. To prevent some of us out here from thinking you sent your patches to the wrong place, could you preface the subject of your messages like this one with RFC: That will allow us to run the patches and look closely at them to give them a critical review. Also, you can still submit your patches, it looks like Alexandre committed the first two and might commit the remaining three you submitted. Lastly, it looks like Release Candidate three for Wine 1.2 will be out on Friday and this may be the last one before the final release. James McKenzie
Re: gameux.dll implementation stub
+HRESULT IGameExplorer_create(IUnknown *pUnkOuter, REFIID riid, LPVOID *ppObj)
+{
+TRACE((%p, %s, %p)\n, pUnkOuter, debugstr_guid(riid), ppObj);
+
+if( IsEqualIID( riid, IID_IGameExplorer ))
This won't work if the caller asks for IUnknown. In general, if you
get a REFIID and void**, you should use QueryInterface so the logic
for getting an interface to your object is in one place.
+static HRESULT WINAPI GameExplorerImpl_VerifyAccess(IGameExplorer *iface,
+BSTR sGDFBinaryPath, BOOL *pHasAccess)
+{
+TRACE((%p, %s, %p)\n, iface, debugstr_w(sGDFBinaryPath), pHasAccess);
+FIXME(stub\n);
+*pHasAccess = TRUE;
+MESSAGE(returning value: %d\n, *pHasAccess);
+/*return E_NOTIMPL;*/
+return S_OK;
+}
The MESSAGE() call is a bit odd. That should probably be a TRACE().
You seem to have some FIXME()s in calls that look fully implemented to
me, such as DllMain() and your object constructors.
There's an official description of the debug levels here (although not
everyone strictly follows them):
http://www.winehq.org/docs/winedev-guide/debugging#DBG-CLASSES
Your implementation of IGameStatistics appears to be impossible to
create when it's added. You should probably wait and add it when you
implement GetGameStatistics.
Your IGameExplorer and IGameExplorer2 objects should be combined into
one implementation. One should be able to query that object for either
interface. Since IGameExplorer2 is based on IGameExplorer, they can
both use the same vtable (just like IUnknown can use any COM object
vtable). This will be much simpler if you use QueryInterface in your
class constructor.
