search

Tomasu / LuaGlue

C++11 Lua 5.2 Binding Library
zlib License
79 stars 22 forks source link

Fix compiler warning C4800 in Microsoft VC 2013 Express #50

Closed stef145g closed 10 years ago

stef145g commented 10 years ago

Without patch, MSVC 2013 produces this warning: luaglue\include\luaglue\luaglueapplytuple.h(233): warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)

this patch corrects this issue.

Tomasu commented 10 years ago

Hi, thank you for the pull request. All of those commits have been committed in one form or another as far as I can tell. Can you either make a pr that is up to date, or show me exactly what changes are needed to kill that warning?

Thanks!

stef145g commented 10 years ago

I am new to using Git Hub, so I am please that I have actually created a pull request and gotten to the correct place. I am not sure how pull requests are processed or what they look like on your side.

Request Title: Fix compiler warning C4800 in Microsoft VC 2013 Express #50 does not appear in your list.

my patch is a single line patch in file: LuaGlueApplyTupple.h Line 233 230 struct stack { static bool get(LuaGlueBase , lua_State s, int idx) { 233 - return lua_toboolean(s, idx); <------- original line 233 + return lua_toboolean(s, idx) ? true : false; <------ changed line }

236 static void put(LuaGlueBase , lua_State s, bool v)

stef145g commented 10 years ago

I don't know why at the top of this screen it states that stef145g wants to merge 7 commits into when there is only one patch as stated in my previous comment.

Originally this was forked from MoneyManagerEx, and I managed to select the correct source to fork from and finally managed to create a pull request. Perhaps this is the reason.

Tomasu commented 10 years ago

It's still showing as it was.. But I've pushed a commit with the change. Thanks for the fix :)