Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/remove_cached_resizing into lp:widelands
Review: Approve Does work well on win. I am for merging and getting rid of the old MinGW Makfile completely. It does miss all SDL2 related changes and CMake+Ninja|Make work fine on windows. -- https://code.launchpad.net/~widelands-dev/widelands/remove_cached_resizing/+merge/243584 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/remove_cached_resizing. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1380286 into lp:widelands
All this is new for me, so it is easily possible I declared something on wrong place or so. As long as it compiles and worked. Do you really want to reimplement all that is available in C to lua? Lua scripting is used quite a little so I am not sure it is worth the effort. But all right, I will look at it, and I prefer option 1. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1380286/+merge/242975 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1380286. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-859245 into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-859245 into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-859245/+merge/241549 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-859245. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-859245 into lp:widelands
Review: Approve looks good. merged. -- https://code.launchpad.net/~widelands-dev/widelands/bug-859245/+merge/241549 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-859245. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1281823 into lp:widelands
Review: Approve Looks good now. Merged. > "What happens when amount > initial_amount" - I dont think is it good aproach > to allow such situation and in addition, there is no set_initial_res_amount > available via LUA - but I will change it, no problem... I understand what you say. If a scripter sets res > initial that could be bad for the engine. But then again, why should initial be changed if somebody changes res? initial is the value at the beginning of the game, it is not the max value. Why should the current value not be higher than the initial value? For fish it could even make logical sense. So I think your implementation is correct - let's see if it makes any trouble. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1281823/+merge/242837 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1281823. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1281823 into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1281823 into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1281823/+merge/242837 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1281823. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1380286 into lp:widelands
Review: Needs Fixing I added a bunch of NOCOM(#codereview) comments in the code. I disagree with the has_warehouse method. Also you added a bunch of method declarations in lua_bases.h, but did not implement these - I removed them, but that was sloppy. Do you look over the diff when you send something for review? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1380286/+merge/242975 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1380286. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1281823 into lp:widelands
@SirVer - I changed it. Test works as well. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1281823/+merge/242837 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1281823. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/remove_cached_resizing into lp:widelands
SirVer has proposed merging lp:~widelands-dev/widelands/remove_cached_resizing
into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/remove_cached_resizing/+merge/243584
Suggested commit message:
- Remove ImageTransformations::resize(). This is now done on the fly by the
GPU, this is just as fast and more memory efficient.
- Removes the dependency to sdl_gfx and all mentions of it in the project.
- Buttons and tab panels will now shrink images to fit in their drawing area.
This exposes some overly big graphics we use in some places.
- Increase the size of the buildings in the BuildGrid preview from 34 pixels to
50. I think that looks much nicer
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/remove_cached_resizing into lp:widelands.
=== modified file 'CMakeLists.txt'
--- CMakeLists.txt 2014-11-12 21:01:23 +
+++ CMakeLists.txt 2014-12-03 19:38:51 +
@@ -44,7 +44,6 @@
find_package(OpenGL REQUIRED)
find_package(PNG REQUIRED)
find_package(SDL2 REQUIRED)
-find_package(SDL2_gfx REQUIRED)
find_package(SDL2_image REQUIRED)
find_package(SDL2_mixer REQUIRED)
find_package(SDL2_net REQUIRED)
=== removed file 'cmake/Modules/FindSDL2_gfx.cmake'
--- cmake/Modules/FindSDL2_gfx.cmake 2014-10-13 15:04:50 +
+++ cmake/Modules/FindSDL2_gfx.cmake 1970-01-01 00:00:00 +
@@ -1,60 +0,0 @@
-# Locate SDL2_gfx library
-# This module defines
-# SDL2GFX_LIBRARY, the name of the library to link against
-# SDL2GFX_FOUND, if false, do not try to link to SDL2
-# SDL2GFX_INCLUDE_DIR, where to find SDL2/SDL.h
-#
-# $SDLDIR is an environment variable that would
-# correspond to the ./configure --prefix=$SDL2DIR
-# used in building SDL2.
-#
-# Created by Eric Wing. This was influenced by the FindSDL.cmake
-# module, but with modifications to recognize OS X frameworks and
-# additional Unix paths (FreeBSD, etc).
-
-FIND_PATH(SDL2GFX_INCLUDE_DIR SDL2_framerate.h
- SDL2_gfxPrimitives.h
- SDL2_imageFilter.h SDL2_rotozoom.h
- HINTS
- $ENV{SDL2GFXDIR}
- $ENV{SDL2DIR}
- PATH_SUFFIXES include SDL2
- PATHS
- ~/Library/Frameworks
- /Library/Frameworks
- /usr/local/include/SDL2
- /usr/include/SDL2
- /usr/local/include
- /usr/include
- /sw/include/SDL2 # Fink
- /sw/include
- /opt/local/include/SDL2 # DarwinPorts
- /opt/local/include
- /opt/csw/include/SDL2 # Blastwave
- /opt/csw/include
- /opt/include/SDL2
- /opt/include
-)
-
-FIND_LIBRARY(SDL2GFX_LIBRARY
- NAMES SDL2_gfx
- HINTS
- $ENV{SDL2GFXDIR}
- $ENV{SDL2DIR}
- PATH_SUFFIXES lib64 lib
- PATHS
- ~/Library/Frameworks
- /Library/Frameworks
- /usr/local
- /usr
- /sw
- /opt/local
- /opt/csw
- /opt
-)
-
-SET(SDL2GFX_FOUND "NO")
-IF(SDL2GFX_LIBRARY AND SDL2GFX_INCLUDE_DIR)
- SET(SDL2GFX_FOUND "YES")
-ENDIF(SDL2GFX_LIBRARY AND SDL2GFX_INCLUDE_DIR)
-
=== modified file 'cmake/WlFunctions.cmake'
--- cmake/WlFunctions.cmake 2014-11-12 20:12:48 +
+++ cmake/WlFunctions.cmake 2014-12-03 19:38:51 +
@@ -10,7 +10,6 @@
USES_OPENGL
USES_PNG
USES_SDL2
-USES_SDL2_GFX
USES_SDL2_IMAGE
USES_SDL2_MIXER
USES_SDL2_NET
@@ -118,11 +117,6 @@
target_link_libraries(${NAME} ${SDL2IMAGE_LIBRARY})
endif()
- if(ARG_USES_SDL2_GFX)
-wl_include_system_directories(${NAME} ${SDL2GFX_INCLUDE_DIR})
-target_link_libraries(${NAME} ${SDL2GFX_LIBRARY})
- endif()
-
if(ARG_USES_SDL2_TTF)
wl_include_system_directories(${NAME} ${SDL2TTF_INCLUDE_DIR})
target_link_libraries(${NAME} ${SDL2TTF_LIBRARY})
=== modified file 'src/editor/ui_menus/categorized_item_selection_menu.h'
--- src/editor/ui_menus/categorized_item_selection_menu.h 2014-10-14 06:30:20 +
+++ src/editor/ui_menus/categorized_item_selection_menu.h 2014-12-03 19:38:51 +
@@ -122,15 +122,7 @@
horizontal->add_space(kSpacing);
++nitems_handled;
}
-
- const Image* category_picture = category.picture();
- const int kCategoryImageSize = 24;
- if (category_picture->width() > kCategoryImageSize ||
- category_picture->height() > kCategoryImageSize) {
- category_picture =
- ImageTransformations::resize(category_picture, kCategoryImageSize, kCategoryImageSize);
- }
- tab_panel->add(category.name(), category_picture, vertical, category.descname());
+ tab_panel->add(category.name(), category.picture(), vertical, category.descname());
}
add(¤t_selection_names_, UI::Align_Center, true);
}
=== modified file 'src/editor/ui_menus/editor_tool_set_terrain_options_menu.cc'
--- src/editor/ui_menus/editor_tool_set_terrain_options_menu.cc 2014-11-27 21:29:21 +
+++ src/editor/ui_menus/editor_tool_set_terrain_options_menu.cc 2014-12-03 19:38:51 +
@@ -76,44 +76,55 @@
const Texture& terrain_texture = terrain_descr.get_texture(0);
Texture* texture = new Texture(terrain_texture.width(), terrain_texture.height());
- texture->blit(Point(0, 0),
+ texture->blit(Rect
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/seafaring-ai into lp:widelands
> No more big branches, this is why I enjoyed these two (or three) small > branches I did lately (including this one and tha LUA resource think - though > this gone wild from one-line change). This has happened to me too and will keep happening - one day I will learn that there are no quick fixes. I sometimes find it frustrating at first as well, but when the branch is finished, I am happy, because the code is better. -- https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/seafaring-ai into lp:widelands. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1281823 into lp:widelands
"What happens when amount > initial_amount" - I dont think is it good aproach to allow such situation and in addition, there is no set_initial_res_amount available via LUA - but I will change it, no problem... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1281823/+merge/242837 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1281823. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/seafaring-ai into lp:widelands
I found it yet :) No more big branches, this is why I enjoyed these two (or three) small branches I did lately (including this one and tha LUA resource think - though this gone wild from one-line change). I have 2 fixes for AI in may mind (both military-related) but I will wait till this one is merged. Having too many branches and multitasking between them is another think that I dont like much :) -- https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/seafaring-ai into lp:widelands. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1281823 into lp:widelands
No, in the editor it is correct to always change the initial_res_amount too - there is no difference between the too there. And in game you should never touch that. What happens when amount > initial_amount I do not know but there is no reason to change the value if the scripter wants exactly that. So you will need to check if you are in the editor or in game. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1281823/+merge/242837 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1281823. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/seafaring-ai into lp:widelands
I think you can maximize the fun by making smaller branches. They are faster to merge, clean and review. -- https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/seafaring-ai into lp:widelands. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1281823 into lp:widelands
all right to have common design for both, I the new design will be: change the initial_res_amount to res_amount only if initial_res_amount would be lower then new value of res_amount Agreed? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1281823/+merge/242837 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1281823. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/f_for_fullscreen into lp:widelands
Merged and filed bug 1398733. -- https://code.launchpad.net/~widelands-dev/widelands/f_for_fullscreen/+merge/243227 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/f_for_fullscreen. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/f_for_fullscreen into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/f_for_fullscreen into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/f_for_fullscreen/+merge/243227 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/f_for_fullscreen. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/seafaring-ai into lp:widelands
I have, I like the 'productive' work, but all that overhead, cleaning the code and what I like the least is conflict resolution - I am never sure if I make it right - all right it will compile, but will the functionality be exactly preserved? It would take further tens of hours of testing to say... But generally I like it, I would not be 'here' otherwise. :) -- https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/seafaring-ai into lp:widelands. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1281823 into lp:widelands
Review: Needs Fixing Spoke to early. The fix is flawed - it also changes initial_res_amount in a game. That leads to wrong results if a scenario changes a fields resource amount. I added a failing test. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1281823/+merge/242837 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1281823. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1281823 into lp:widelands
Review: Approve You missed one rename :). I did it for you and merged. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1281823/+merge/242837 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1281823. ___ 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
