Re: [PATCH 1/1] ddraw: Return the primary legacy ddraw device last.

[Stefan Dösinger]

There are a few things I am wondering about and where the test can be extended:

The existing test
> /* Test with valid callback parameter and count the number of primary devices */
> callbackCount = 0;
> ret = pDirectDrawEnumerateExA(test_count_callbackExA, &callbackCount, 0);
> ok(ret == DD_OK, "Expected DD_OK, got %d\n", ret);
> ok(callbackCount == 1, "Expected 1 primary device, got %d\n", callbackCount);

Combined with your change that suggests that without any flag only the NULL guid is enumerated. If so I think that's worth an explicit check.

Is the NULL guid enumerated when DDENUM_DETACHEDSECONDARYDEVICES and/or DDENUM_NONDISPLAYDEVICES are set?

Regarding the specific changes you make:
> dlls/ddraw/tests/ddrawmodes.c |   35 +++++++++++++++++++++++++++++++++++

I'd prefer that this test be moved to ddraw1.c
> - static char DriverDescription[] = "DirectDraw HAL";
> - static char DriverName[] = "display";

Please do not use CamelCase or lpFoo in d3d-related code.

Somewhat related, but that can be a separate patch: d3d3_EnumDevices() mentions that some games modify the strings. In your code (and the current code), such modifications would persist for the runtime of the application. It would be interesting to add a test for this.
> +static BOOL WINAPI test_last_callbackExA(GUID *lpGUID, char *lpDriverDescription,
> -
>    char *lpDriverName, void *lpContext, HMONITOR hm)

You should be able to combine the callbacks. Just declare a structure and put a counter in it.
> +{
> - GUID **context_guid = (GUID **)lpContext;
> - static GUID last_guid;

I guess this works, but it is somewhat ugly. If you use a struct you can put a GUID and a BOOL guid_was_null in it. Or a GUID and a GUID *.

Please extend the test to DirectDrawEnumerateA. MSDN suggests it is equivalent to DirectDrawEnumerateExA(DDENUM_NONDISPLAYDEVICES), but I have my doubts here. Comparing the strings of the non-NULL device against "DirectDraw HAL" and "display" (in the Ex and non-Ex case) would also be a good idea.

Am 13.10.2014 um 21:16 schrieb Erich E. Hoover :
> Fixes bug #37307.  There are a couple of other bugs that are
> preventing Urban Assault from working, but this fixes the problem with
> returning the interfaces in the wrong order.
> <0001-ddraw-Return-the-primary-legacy-ddraw-device-last.patch>