I just noticed that the compiler warnings come up on trunk as well, so they're
not caused by this particular branch.
--
https://code.launchpad.net/~widelands-dev/widelands/texture_atlas/+merge/243119
Your team Widelands Developers is subscribed to branch
Review: Approve
LGTM, a couple of nits. Could you fix them.
Also starting_res_amount is a bit cumbersome. What do you guys think of
renaming it to initial_resource_amount throughout the codebase (i.e. also in
cpp?).
Diff comments:
=== modified file 'src/scripting/lua_map.cc'
---
They do not come up with clang. Could you fix them in trunk?
Can this be merged then? I think all comments are sorted out.
Am 29.11.2014 um 10:32 schrieb GunChleoc f...@foramnagaidhlig.net:
I just noticed that the compiler warnings come up on trunk as well, so
they're not caused by this
Review: Approve
Couple of nits. I suggest to prefer dynamic_cast wherever possible (i.e. when
you know the type of the object you have) instead of upcast() which is made for
if(upcast()) { situations. dynamic_cast can also cast references which will
make for easier to read code.
Diff
I haven't had time to look over all of the code yet, but I've tested it and it
seems to work fine. If you think that's sufficient, go ahead and merge.
Otherwise, I'll try to have a look tomorrow. Brain's not working today.
--
+1 to renaming. Do we want to do that here or in a separate bug? I think doing
it here might be a good idea, so we will have it in before we expose it to the
Lua interface.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1281823/+merge/242837
Your team Widelands Developers is
no problem for me to rename
@SirVer - see answer to one of your comments below in diff.
Diff comments:
=== modified file 'src/scripting/lua_map.cc'
--- src/scripting/lua_map.cc 2014-10-27 10:14:10 +
+++ src/scripting/lua_map.cc 2014-11-26 20:51:03 +
@@ -3501,6 +3501,7 @@
7 matches
Mail list logo
