Review: Approve Code looks okay, three (minor) comments in the diff. It works as expected on my system with en_US and de_DE locale.
Diff comments: > === modified file 'src/graphic/gl/initialize.cc' > --- src/graphic/gl/initialize.cc 2018-10-15 05:26:10 +0000 > +++ src/graphic/gl/initialize.cc 2018-10-15 06:50:28 +0000 > @@ -178,11 +179,26 @@ > glGetIntegerv(GL_MAX_TEXTURE_SIZE, max_texture_size); > log("Graphics: OpenGL: Max texture size: %u\n", *max_texture_size); > > - // 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)") == 0) { > + log("Graphics: OpenGL: ShadingLanguage: \"%s\"\n", > shading_language_version_string); This might now be printed before the "Unable to detect version" message. No objection, though. > + > + std::vector<std::string> shading_language_version_vector; > + boost::split(shading_language_version_vector, > shading_language_version_string, boost::is_any_of(".")); > + if (shading_language_version_vector.size() >= 2) { >= or == ? Also, this might break in the future if "2" is reported as shading version. But I guess it is fine for now and we can fix it if this ever happens. Also it would be problematic if some driver reports the version as "1.2". > + // The shading language version has been detected properly. > Exit if the shading language version is too old. > + const int major_shading_language_version = > atoi(shading_language_version_vector.front().c_str()); > + const int minor_shading_language_version = > atoi(shading_language_version_vector.at(1).c_str()); > + if (major_shading_language_version < 1 || > (major_shading_language_version == 1 && minor_shading_language_version < 20)) > { > + log("ERROR: Shading language version is too old!\n"); > + SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "OpenGL > Error", > + > "Widelands won’t work because your graphics driver is too old.\nThe " > + > "Shading language needs to be version 1.20 or newer.", Maybe move the "The" into the next line? Doesn't matters, but looks strange in the source code. ;-) "Shading" upper or lower case (is in middle of sentence)? > + NULL); > + exit(1); > + } > + } else { > + // Exit because we couldn't detect the shading language > version, so there must be a problem communicating with the graphics adapter. > 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 " -- https://code.launchpad.net/~widelands-dev/widelands/bug-1797792-shading-language-version-comparison/+merge/356699 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1797792-shading-language-version-comparison. _______________________________________________ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp