widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Codereview done.

I disagree that this is not a new feature, the code changes are more than trivial. Yes, it does fix a bug, but it's an old bug, so I'm not sure that it is critical enough to risk introducing new bugs.

Diff comments:

> === modified file 'data/scripting/win_conditions/territorial_functions.lua'
> --- data/scripting/win_conditions/territorial_functions.lua	2019-02-12 17:30:21 +0000
> +++ data/scripting/win_conditions/territorial_functions.lua	2019-02-17 19:27:39 +0000
> @@ -14,24 +14,88 @@
>  local wc_had_territory = _"%1$s had %2$3.0f%% of the land (%3$i of %4$i)."
>  
>  -- RST
> --- .. function:: get_buildable_fields()
> ---
> ---    Collects all fields that are buildable
> ---
> ---    :returns: a table with the map's buildable fields
> ---
> -function get_buildable_fields()
> +-- .. function:: get_valuable_fields()
> +--
> +--    Collects all fields that are valuable
> +--
> +--    :returns: a table with the map's valuable fields
> +--
> +function get_valuable_fields()
>     local fields = {}
> +   local check = {}
>     local map = wl.Game().map
> -   for x=0, map.width-1 do
> -      for y=0, map.height-1 do
> -         local f = map:get_field(x,y)
> -         if f.buildable then
> -            table.insert(fields, f)
> -         end
> -      end
> -   end
> -   return fields
> +   local plrs = wl.Game().players
> +   for idx, player in ipairs(plrs) do
> +      local sf = map.player_slots[idx].starting_field
> +   -- initialize the fields table and the check area with the regions around the starting field of each Player
> +      for idx, fg in ipairs(sf:region(9)) do
> +         local index = fg.x * 1000 + fg.y
> +         fields[index] = fg
> +         check[index] = fg
> +      end
> +   end
> +   if map.allows_seafaring == true then
> +      -- TODO: this check is a little bit consuming, if possible we should read the port spaces from the map

Could be fixed easily by exposing Map::get_port_spaces() to Lua

> +      for x=0, map.width-1 do
> +         for y=0, map.height-1 do
> +            local f = map:get_field(x,y)
> +            if f:has_max_caps("port") then
> +               for idx, fg in ipairs(f:region(5)) do
> +                  local index = fg.x * 1000 + fg.y
> +                  fields[index] = fg
> +                  check[index] = fg
> +               end
> +            end
> +         end
> +      end
> +   end
> +   local loop = true
> +   while loop do
> +      loop = false
> +      local new = {}
> +      -- checking the check region for buildcaps and add fields that can be conquered
> +      for idx, f in pairs(check) do 
> +         if f:has_max_caps("big") then
> +            local radius = f:region(9)
> +            for idx, fg in ipairs(radius) do
> +               local index = fg.x * 1000 + fg.y
> +               if fields[index] == nil and check[index] == nil and fg:has_max_caps("walkable") then 
> +                  -- if new buildable fields are discovered, add them to the fields table and mark them for checking next loop
> +                  new[index] = fg
> +                  fields[index] = fg
> +                  loop = true
> +               end
> +            end
> +         elseif f:has_max_caps("medium") then
> +            local radius = f:region(7)

This line is the only difference between if and else

> +            for idx, fg in ipairs(radius) do
> +               local index = fg.x * 1000 + fg.y

We should expose Coords:hash() to the Lua interface and use that instead.

> +               if fields[index] == nil and check[index] == nil and fg:has_max_caps("walkable") then 
> +                  new[index] = fg
> +                  fields[index] = fg
> +                  loop = true
> +               end
> +            end
> +         elseif f:has_max_caps("small") then
> +            local radius = f:region(5)
> +            for idx, fg in ipairs(radius) do
> +               local index = fg.x * 1000 + fg.y
> +               if fields[index] == nil and check[index] == nil and fg:has_max_caps("walkable") then 
> +                  new[index] = fg
> +                  fields[index] = fg
> +                  loop = true
> +               end
> +            end
> +         end
> +      end
> +      check = new
> +   end
> +   local result = {}
> +   -- as our fields table is not continuosly indexed we need to build a properly indexed table
> +   for idx,f in pairs(fields) do
> +      table.insert(result, f)
> +   end
> +   return result
>  end
>  
>  -- RST
> 
> === modified file 'src/scripting/lua_map.cc'
> --- src/scripting/lua_map.cc	2018-12-14 15:55:40 +0000
> +++ src/scripting/lua_map.cc	2019-02-17 19:27:39 +0000
> @@ -6497,6 +6497,49 @@
>  	return 1;
>  }
>  
> +/* RST
> +   .. method:: has_max_caps(capname)
> +
> +      Returns :const:`true` if the field has this maximum caps (not taking immovables into account) associated
> +      with it, otherwise returns false.
> +
> +      :arg capname: can be either of
> +
> +      * :const:`small`: Can a small building be build here?

build -> built

> +      * :const:`medium`: Can a medium building be build here?
> +      * :const:`big`: Can a big building be build here?
> +      * :const:`mine`: Can a mine be build here?
> +      * :const:`port`: Can a port be build here?
> +      * :const:`flag`: Can a flag be build here?
> +      * :const:`walkable`: Is this field passable for walking bobs?
> +      * :const:`swimmable`: Is this field passable for swimming bobs?
> +*/
> +int LuaField::has_max_caps(lua_State* L) {
> +	FCoords f = fcoords(L);
> +	std::string query = luaL_checkstring(L, 2);
> +
> +	if (query == "walkable")
> +		lua_pushboolean(L, f.field->maxcaps() & MOVECAPS_WALK);
> +	else if (query == "swimmable")
> +		lua_pushboolean(L, f.field->maxcaps() & MOVECAPS_SWIM);
> +	else if (query == "small")
> +		lua_pushboolean(L, f.field->maxcaps() & BUILDCAPS_SMALL);
> +	else if (query == "medium")
> +		lua_pushboolean(L, f.field->maxcaps() & BUILDCAPS_MEDIUM);
> +	else if (query == "big")
> +		lua_pushboolean(L, (f.field->maxcaps() & BUILDCAPS_BIG) == BUILDCAPS_BIG);
> +	else if (query == "port") {
> +		lua_pushboolean(
> +		   L, (f.field->maxcaps() & BUILDCAPS_PORT) && get_egbase(L).map().is_port_space(f));
> +	} else if (query == "mine")
> +		lua_pushboolean(L, f.field->maxcaps() & BUILDCAPS_MINE);
> +	else if (query == "flag")
> +		lua_pushboolean(L, f.field->maxcaps() & BUILDCAPS_FLAG);
> +	else
> +		report_error(L, "Unknown caps queried: %s!", query.c_str());
> +
> +	return 1;
> +}
>  /*
>   ==========================================================
>   C METHODS


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1810062-territorial-calculations.