widelands-dev team mailing list archive

[Notabilis <notabilis27@xxxxxx>]

Diff is looking good. I added a few smaller remarks as comments, though.
Testing by modifying the conditions displayed the message boxes for me.

Diff comments:

> === modified file 'src/graphic/gl/initialize.cc'
> --- src/graphic/gl/initialize.cc	2018-09-10 06:11:01 +0000
> +++ src/graphic/gl/initialize.cc	2018-09-27 09:32:24 +0000
> @@ -177,8 +178,30 @@
>  	glGetIntegerv(GL_MAX_TEXTURE_SIZE, max_texture_size);
>  	log("Graphics: OpenGL: Max texture size: %u\n", *max_texture_size);
>  
> -	log("Graphics: OpenGL: ShadingLanguage: \"%s\"\n",
> -	    reinterpret_cast<const char*>(glGetString(GL_SHADING_LANGUAGE_VERSION)));
> +	// TODO(GunChleoc): Localize the on-screen error messages
> +	// Exit if we can't detect the shading language version
> +	const char* const shading_language_version_string = reinterpret_cast<const char*>(glGetString(GL_SHADING_LANGUAGE_VERSION));
> +	if (!strcmp(shading_language_version_string, "(null)")) {

Maybe use `== 0` instead of `!` ? When reading over it quickly it might be read as a check for unequal strings.

> +		log("ERROR: Unable to detect the shading language version!\n");
> +		SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR,
> +								 "OpenGL Error",
> +								 "Widelands won’t work because we were unable to detect the shading language version – there is an unknown problem with reading the information from the graphics driver.",

On my computer a square box is displayed instead of the long hyphen.

> +								 NULL);
> +		exit(1);
> +	}
> +
> +	log("Graphics: OpenGL: ShadingLanguage: \"%s\"\n", shading_language_version_string);
> +
> +	// Exit if the shading language version is too old
> +	const double shading_language_version = atof(shading_language_version_string);
> +	if (shading_language_version < 1.20) {
> +		log("ERROR: Shading language version is too old!\n");

"... too old, at least version 1.20 is required!" ?

> +		SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR,
> +								 "OpenGL Error",
> +								 "Widelands won’t work because your graphics driver is too old.",
> +								 NULL);
> +		exit(1);
> +	}
>  
>  	glDrawBuffer(GL_BACK);
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1536377-fail-gracefully-on-graphics-driver-problems/+merge/355757
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1536377-fail-gracefully-on-graphics-driver-problems into lp:widelands.