Error for misspelled / invalid window attributes in love.window.setMode (resolves issue #736)

src/modules/graphics/opengl/Graphics.cpp

@@ -31,8 +31,6 @@
 #include <algorithm>
 #include <iterator>
 
-using love::window::WindowFlags;
-
 namespace love
 {
 namespace graphics

src/modules/graphics/opengl/Graphics.h

@@ -47,8 +47,6 @@
 #include "Shader.h"
 #include "Mesh.h"
 
-using love::window::WindowFlags;
-
 namespace love
 {
 namespace graphics

src/modules/graphics/opengl/wrap_Graphics.cpp

@@ -27,8 +27,6 @@
 #include "scripts/graphics.lua.h"
 #include <cassert>
 
-using love::window::WindowFlags;
-
 namespace love
 {
 namespace graphics

src/modules/window/Window.cpp

@@ -38,7 +38,7 @@ void Window::swapBuffers()
 {
 }
 
-WindowFlags::WindowFlags()
+WindowAttributes::WindowAttributes()
 	: fullscreen(false)
 	, fstype(Window::FULLSCREEN_TYPE_NORMAL)
 	, vsync(true)
@@ -62,6 +62,32 @@ bool Window::getConstant(Window::FullscreenType in, const char *&out)
 	return fullscreenTypes.find(in, out);
 }
 
+bool Window::getConstant(const char *in, Window::Attribute &out)
+{
+	return attributes.find(in, out);
+}
+
+bool Window::getConstant(Window::Attribute in, const char *&out)
+{
+	return attributes.find(in, out);
+}
+
+StringMap<Window::Attribute, Window::ATTRIB_MAX_ENUM>::Entry Window::attributeEntries[] =
+{
+	{"fullscreen", ATTRIB_FULLSCREEN},
+	{"fullscreentype", ATTRIB_FULLSCREEN_TYPE},
+	{"vsync", ATTRIB_VSYNC},
+	{"fsaa", ATTRIB_FSAA},
+	{"resizable", ATTRIB_RESIZABLE},
+	{"minwidth", ATTRIB_MIN_WIDTH},
+	{"minheight", ATTRIB_MIN_HEIGHT},
+	{"borderless", ATTRIB_BORDERLESS},
+	{"centered", ATTRIB_CENTERED},
+	{"display", ATTRIB_DISPLAY}
+};
+
+StringMap<Window::Attribute, Window::ATTRIB_MAX_ENUM> Window::attributes(Window::attributeEntries, sizeof(Window::attributeEntries));
+
 StringMap<Window::FullscreenType, Window::FULLSCREEN_TYPE_MAX_ENUM>::Entry Window::fullscreenTypeEntries[] =
 {
 	{"normal", Window::FULLSCREEN_TYPE_NORMAL},

src/modules/window/Window.h

@@ -37,12 +37,28 @@ namespace window
 
 // Forward-declared so it can be used in the class methods. We can't define the
 // whole thing here because it uses the Window::Type enum.
-struct WindowFlags;
+struct WindowAttributes;
 
 class Window : public Module
 {
 public:
 
+	// Types of window attributes.
+	enum Attribute
+	{
+		ATTRIB_FULLSCREEN,
+		ATTRIB_FULLSCREEN_TYPE,
+		ATTRIB_VSYNC,
+		ATTRIB_FSAA,
+		ATTRIB_RESIZABLE,
+		ATTRIB_MIN_WIDTH,
+		ATTRIB_MIN_HEIGHT,
+		ATTRIB_BORDERLESS,
+		ATTRIB_CENTERED,
+		ATTRIB_DISPLAY,
+		ATTRIB_MAX_ENUM
+	};
+
 	enum FullscreenType
 	{
 		FULLSCREEN_TYPE_NORMAL,
@@ -58,8 +74,8 @@ public:
 
 	virtual ~Window();
 
-	virtual bool setWindow(int width = 800, int height = 600, WindowFlags *flags = 0) = 0;
-	virtual void getWindow(int &width, int &height, WindowFlags &flags) = 0;
+	virtual bool setWindow(int width = 800, int height = 600, WindowAttributes *attribs = 0) = 0;
+	virtual void getWindow(int &width, int &height, WindowAttributes &attribs) = 0;
 
 	virtual bool setFullscreen(bool fullscreen, FullscreenType fstype) = 0;
 	virtual bool setFullscreen(bool fullscreen) = 0;
@@ -103,6 +119,9 @@ public:
 	//virtual static Window *getSingleton() = 0;
 	// No virtual statics, of course, but you are supposed to implement these statics.
 
+	static bool getConstant(const char *in, Attribute &out);
+	static bool getConstant(Attribute in, const char *&out);
+
 	static bool getConstant(const char *in, FullscreenType &out);
 	static bool getConstant(FullscreenType in, const char *&out);
 
@@ -112,14 +131,17 @@ protected:
 
 private:
 
+	static StringMap<Attribute, ATTRIB_MAX_ENUM>::Entry attributeEntries[];
+	static StringMap<Attribute, ATTRIB_MAX_ENUM> attributes;
+
 	static StringMap<FullscreenType, FULLSCREEN_TYPE_MAX_ENUM>::Entry fullscreenTypeEntries[];
 	static StringMap<FullscreenType, FULLSCREEN_TYPE_MAX_ENUM> fullscreenTypes;
 
 }; // Window
 
-struct WindowFlags
+struct WindowAttributes
 {
-	WindowFlags();
+	WindowAttributes();
 	bool fullscreen; // = false
 	Window::FullscreenType fstype; // = FULLSCREEN_TYPE_NORMAL
 	bool vsync; // = true

src/modules/window/sdl/Window.cpp

@@ -69,21 +69,21 @@ Window::~Window()
 Window::_currentMode::_currentMode()
 	: width(800)
 	, height(600)
-	, flags()
+	, attribs()
 	, icon(0)
 {
 }
 
-bool Window::setWindow(int width, int height, WindowFlags *flags)
+bool Window::setWindow(int width, int height, WindowAttributes *attribs)
 {
 	graphics::Graphics *gfx = (graphics::Graphics *) Module::findInstance("love.graphics.");
 	if (gfx)
 		gfx->unSetMode();
 
-	WindowFlags f;
+	WindowAttributes f;
 
-	if (flags)
-		f = *flags;
+	if (attribs)
+		f = *attribs;
 
 	f.minwidth = std::max(f.minwidth, 1);
 	f.minheight = std::max(f.minheight, 1);
@@ -130,7 +130,7 @@ bool Window::setWindow(int width, int height, WindowFlags *flags)
 		wflags &= (SDL_WINDOW_OPENGL | SDL_WINDOW_FULLSCREEN_DESKTOP | SDL_WINDOW_RESIZABLE | SDL_WINDOW_BORDERLESS);
 
 		if (sdlflags != wflags || width != curMode.width || height != curMode.height
-			|| f.display != curdisplay || f.fsaa != curMode.flags.fsaa)
+			|| f.display != curdisplay || f.fsaa != curMode.attribs.fsaa)
 		{
 			SDL_DestroyWindow(window);
 			window = 0;
@@ -188,7 +188,7 @@ bool Window::setWindow(int width, int height, WindowFlags *flags)
 
 	created = true;
 
-	updateWindowFlags(f);
+	updateAttributes(f);
 
 	if (gfx)
 		gfx->setMode(curMode.width, curMode.height);
@@ -256,8 +256,8 @@ bool Window::setContext(int fsaa, bool vsync)
 		fsaa = (buffers > 0) ? samples : 0;
 	}
 
-	curMode.flags.fsaa = fsaa;
-	curMode.flags.vsync = SDL_GL_GetSwapInterval() != 0;
+	curMode.attribs.fsaa = fsaa;
+	curMode.attribs.vsync = SDL_GL_GetSwapInterval() != 0;
 
 	return true;
 }
@@ -277,7 +277,7 @@ void Window::setWindowGLAttributes(int fsaa) const
 	SDL_GL_SetAttribute(SDL_GL_MULTISAMPLESAMPLES, (fsaa > 0) ? fsaa : 0);
 }
 
-void Window::updateWindowFlags(const WindowFlags &newflags)
+void Window::updateAttributes(const WindowAttributes &newattribs)
 {
 	Uint32 wflags = SDL_GetWindowFlags(window);
 
@@ -286,44 +286,44 @@ void Window::updateWindowFlags(const WindowFlags &newflags)
 
 	if ((wflags & SDL_WINDOW_FULLSCREEN_DESKTOP) == SDL_WINDOW_FULLSCREEN_DESKTOP)
 	{
-		curMode.flags.fullscreen = true;
-		curMode.flags.fstype = FULLSCREEN_TYPE_DESKTOP;
+		curMode.attribs.fullscreen = true;
+		curMode.attribs.fstype = FULLSCREEN_TYPE_DESKTOP;
 	}
 	else if ((wflags & SDL_WINDOW_FULLSCREEN) == SDL_WINDOW_FULLSCREEN)
 	{
-		curMode.flags.fullscreen = true;
-		curMode.flags.fstype = FULLSCREEN_TYPE_NORMAL;
+		curMode.attribs.fullscreen = true;
+		curMode.attribs.fstype = FULLSCREEN_TYPE_NORMAL;
 	}
 	else
 	{
-		curMode.flags.fullscreen = false;
-		curMode.flags.fstype = newflags.fstype;
+		curMode.attribs.fullscreen = false;
+		curMode.attribs.fstype = newattribs.fstype;
 	}
 
 	// The min width/height is set to 0 internally in SDL when in fullscreen.
-	if (curMode.flags.fullscreen)
+	if (curMode.attribs.fullscreen)
 	{
-		curMode.flags.minwidth = newflags.minwidth;
-		curMode.flags.minheight = newflags.minheight;
+		curMode.attribs.minwidth = newattribs.minwidth;
+		curMode.attribs.minheight = newattribs.minheight;
 	}
 	else
-		SDL_GetWindowMinimumSize(window, &curMode.flags.minwidth, &curMode.flags.minheight);
+		SDL_GetWindowMinimumSize(window, &curMode.attribs.minwidth, &curMode.attribs.minheight);
 
-	curMode.flags.resizable = (wflags & SDL_WINDOW_RESIZABLE) != 0;
-	curMode.flags.borderless = (wflags & SDL_WINDOW_BORDERLESS) != 0;
-	curMode.flags.centered = newflags.centered;
-	curMode.flags.display = std::max(SDL_GetWindowDisplayIndex(window), 0);
+	curMode.attribs.resizable = (wflags & SDL_WINDOW_RESIZABLE) != 0;
+	curMode.attribs.borderless = (wflags & SDL_WINDOW_BORDERLESS) != 0;
+	curMode.attribs.centered = newattribs.centered;
+	curMode.attribs.display = std::max(SDL_GetWindowDisplayIndex(window), 0);
 }
 
-void Window::getWindow(int &width, int &height, WindowFlags &flags)
+void Window::getWindow(int &width, int &height, WindowAttributes &attribs)
 {
 	// Window position may be different from creation - update display index.
 	if (window)
-		curMode.flags.display = std::max(SDL_GetWindowDisplayIndex(window), 0);
+		curMode.attribs.display = std::max(SDL_GetWindowDisplayIndex(window), 0);
 
 	width = curMode.width;
 	height = curMode.height;
-	flags = curMode.flags;
+	attribs = curMode.attribs;
 }
 
 bool Window::setFullscreen(bool fullscreen, Window::FullscreenType fstype)
@@ -331,9 +331,9 @@ bool Window::setFullscreen(bool fullscreen, Window::FullscreenType fstype)
 	if (!window)
 		return false;
 
-	WindowFlags newflags = curMode.flags;
-	newflags.fullscreen = fullscreen;
-	newflags.fstype = fstype;
+	WindowAttributes newattribs = curMode.attribs;
+	newattribs.fullscreen = fullscreen;
+	newattribs.fstype = fstype;
 
 	Uint32 sdlflags = 0;
 
@@ -357,7 +357,7 @@ bool Window::setFullscreen(bool fullscreen, Window::FullscreenType fstype)
 	if (SDL_SetWindowFullscreen(window, sdlflags) == 0)
 	{
 		SDL_GL_MakeCurrent(window, context);
-		updateWindowFlags(newflags);
+		updateAttributes(newattribs);
 		return true;
 	}
 
@@ -366,7 +366,7 @@ bool Window::setFullscreen(bool fullscreen, Window::FullscreenType fstype)
 
 bool Window::setFullscreen(bool fullscreen)
 {
-	return setFullscreen(fullscreen, curMode.flags.fstype);
+	return setFullscreen(fullscreen, curMode.attribs.fstype);
 }
 
 int Window::getDisplayCount() const

src/modules/window/sdl/Window.h

@@ -41,8 +41,8 @@ public:
 	Window();
 	~Window();
 
-	bool setWindow(int width = 800, int height = 600, WindowFlags *flags = 0);
-	void getWindow(int &width, int &height, WindowFlags &flags);
+	bool setWindow(int width = 800, int height = 600, WindowAttributes *attribs = 0);
+	void getWindow(int &width, int &height, WindowAttributes &attribs);
 
 	bool setFullscreen(bool fullscreen, FullscreenType fstype);
 	bool setFullscreen(bool fullscreen);
@@ -91,8 +91,8 @@ private:
 	bool setContext(int fsaa, bool vsync);
 	void setWindowGLAttributes(int fsaa) const;
 
-	// Update the window flags based on the window's actual state.
-	void updateWindowFlags(const WindowFlags &newflags);
+	// Update the saved window attribs based on the window's actual state.
+	void updateAttributes(const WindowAttributes &newattribs);
 
 	std::string windowTitle;
 
@@ -102,7 +102,7 @@ private:
 
 		int width;
 		int height;
-		WindowFlags flags;
+		WindowAttributes attribs;
 		love::image::ImageData *icon;
 
 	} curMode;

src/modules/window/wrap_Window.cpp

@@ -34,6 +34,13 @@ int w_getDisplayCount(lua_State *L)
 	return 1;
 }
 
+static const char *attribName(Window::Attribute attrib)
+{
+	const char *name = nullptr;
+	Window::getConstant(attrib, name);
+	return name;
+}
+
 int w_setMode(lua_State *L)
 {
 	int w = luaL_checkint(L, 1);
@@ -47,83 +54,99 @@ int w_setMode(lua_State *L)
 
 	luaL_checktype(L, 3, LUA_TTABLE);
 
-	WindowFlags flags;
+	// We want to error for invalid / misspelled window attributes.
+	lua_pushnil(L);
+	while (lua_next(L, 3))
+	{
+		if (lua_type(L, -2) != LUA_TSTRING)
+			return luax_typerror(L, -2, "string");
+
+		const char *key = luaL_checkstring(L, -2);
+		Window::Attribute attrib;
+
+		if (!Window::getConstant(key, attrib))
+			return luaL_error(L, "Invalid window attribute: %s", key);
+
+		lua_pop(L, 1);
+	}
+
+	WindowAttributes attribs;
 
-	lua_getfield(L, 3, "fullscreentype");
+	lua_getfield(L, 3, attribName(Window::ATTRIB_FULLSCREEN_TYPE));
 	if (!lua_isnoneornil(L, -1))
 	{
 		const char *typestr = luaL_checkstring(L, -1);
 
-		if (!Window::getConstant(typestr, flags.fstype))
+		if (!Window::getConstant(typestr, attribs.fstype))
 			return luaL_error(L, "Invalid fullscreen type: %s", typestr);
 	}
 	else
 	{
 		// Default to "normal" fullscreen.
-		flags.fstype = Window::FULLSCREEN_TYPE_NORMAL;
+		attribs.fstype = Window::FULLSCREEN_TYPE_NORMAL;
 	}
 	lua_pop(L, 1);
 
-	flags.fullscreen = luax_boolflag(L, 3, "fullscreen", false);
-	flags.vsync = luax_boolflag(L, 3, "vsync", true);
-	flags.fsaa = luax_intflag(L, 3, "fsaa", 0);
-	flags.resizable = luax_boolflag(L, 3, "resizable", false);
-	flags.minwidth = luax_intflag(L, 3, "minwidth", 1);
-	flags.minheight = luax_intflag(L, 3, "minheight", 1);
-	flags.borderless = luax_boolflag(L, 3, "borderless", false);
-	flags.centered = luax_boolflag(L, 3, "centered", true);
-	flags.display = luax_intflag(L, 3, "display", 1);
+	attribs.fullscreen = luax_boolflag(L, 3, attribName(Window::ATTRIB_FULLSCREEN), false);
+	attribs.vsync = luax_boolflag(L, 3, attribName(Window::ATTRIB_VSYNC), true);
+	attribs.fsaa = luax_intflag(L, 3, attribName(Window::ATTRIB_FSAA), 0);
+	attribs.resizable = luax_boolflag(L, 3, attribName(Window::ATTRIB_RESIZABLE), false);
+	attribs.minwidth = luax_intflag(L, 3, attribName(Window::ATTRIB_MIN_WIDTH), 1);
+	attribs.minheight = luax_intflag(L, 3, attribName(Window::ATTRIB_MIN_HEIGHT), 1);
+	attribs.borderless = luax_boolflag(L, 3, attribName(Window::ATTRIB_BORDERLESS), false);
+	attribs.centered = luax_boolflag(L, 3, attribName(Window::ATTRIB_CENTERED), true);
+	attribs.display = luax_intflag(L, 3, attribName(Window::ATTRIB_DISPLAY), 1);
 
 	// Display index is 1-based in Lua and 0-based internally.
-	flags.display--;
+	attribs.display--;
 
-	EXCEPT_GUARD(luax_pushboolean(L, instance->setWindow(w, h, &flags));)
+	EXCEPT_GUARD(luax_pushboolean(L, instance->setWindow(w, h, &attribs));)
 	return 1;
 }
 
 int w_getMode(lua_State *L)
 {
 	int w, h;
-	WindowFlags flags;
-	instance->getWindow(w, h, flags);
+	WindowAttributes attribs;
+	instance->getWindow(w, h, attribs);
 	lua_pushnumber(L, w);
 	lua_pushnumber(L, h);
 
 	lua_newtable(L);
 
 	const char *fstypestr = "normal";
-	Window::getConstant(flags.fstype, fstypestr);
+	Window::getConstant(attribs.fstype, fstypestr);
 
 	lua_pushstring(L, fstypestr);
-	lua_setfield(L, -2, "fullscreentype");
+	lua_setfield(L, -2, attribName(Window::ATTRIB_FULLSCREEN_TYPE));
 
-	luax_pushboolean(L, flags.fullscreen);
-	lua_setfield(L, -2, "fullscreen");
+	luax_pushboolean(L, attribs.fullscreen);
+	lua_setfield(L, -2, attribName(Window::ATTRIB_FULLSCREEN));
 
-	luax_pushboolean(L, flags.vsync);
-	lua_setfield(L, -2, "vsync");
+	luax_pushboolean(L, attribs.vsync);
+	lua_setfield(L, -2, attribName(Window::ATTRIB_VSYNC));
 
-	lua_pushinteger(L, flags.fsaa);
-	lua_setfield(L, -2, "fsaa");
+	lua_pushinteger(L, attribs.fsaa);
+	lua_setfield(L, -2, attribName(Window::ATTRIB_FSAA));
 
-	luax_pushboolean(L, flags.resizable);
-	lua_setfield(L, -2, "resizable");
+	luax_pushboolean(L, attribs.resizable);
+	lua_setfield(L, -2, attribName(Window::ATTRIB_RESIZABLE));
 
-	lua_pushinteger(L, flags.minwidth);
-	lua_setfield(L, -2, "minwidth");
+	lua_pushinteger(L, attribs.minwidth);
+	lua_setfield(L, -2, attribName(Window::ATTRIB_MIN_WIDTH));
 
-	lua_pushinteger(L, flags.minheight);
-	lua_setfield(L, -2, "minheight");
+	lua_pushinteger(L, attribs.minheight);
+	lua_setfield(L, -2, attribName(Window::ATTRIB_MIN_HEIGHT));
 
-	luax_pushboolean(L, flags.borderless);
-	lua_setfield(L, -2, "borderless");
+	luax_pushboolean(L, attribs.borderless);
+	lua_setfield(L, -2, attribName(Window::ATTRIB_BORDERLESS));
 
-	luax_pushboolean(L, flags.centered);
-	lua_setfield(L, -2, "centered");
+	luax_pushboolean(L, attribs.centered);
+	lua_setfield(L, -2, attribName(Window::ATTRIB_CENTERED));
 
 	// Display index is 0-based internally and 1-based in Lua.
-	lua_pushinteger(L, flags.display + 1);
-	lua_setfield(L, -2, "display");
+	lua_pushinteger(L, attribs.display + 1);
+	lua_setfield(L, -2, attribName(Window::ATTRIB_DISPLAY));
 
 	return 3;
 }
@@ -179,14 +202,14 @@ int w_setFullscreen(lua_State *L)
 int w_getFullscreen(lua_State *L)
 {
 	int w, h;
-	WindowFlags flags;
-	instance->getWindow(w, h, flags);
+	WindowAttributes attribs;
+	instance->getWindow(w, h, attribs);
 
 	const char *typestr;
-	if (!Window::getConstant(flags.fstype, typestr))
+	if (!Window::getConstant(attribs.fstype, typestr))
 		luaL_error(L, "Unknown fullscreen type.");
 
-	luax_pushboolean(L, flags.fullscreen);
+	luax_pushboolean(L, attribs.fullscreen);
 	lua_pushstring(L, typestr);
 	return 2;
 }