widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

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'
> --- src/scripting/lua_map.cc	2014-10-27 10:14:10 +0000
> +++ src/scripting/lua_map.cc	2014-11-26 20:51:03 +0000
> @@ -3501,6 +3501,7 @@
>  	PROP_RO(LuaField, viewpoint_y),
>  	PROP_RW(LuaField, resource),
>  	PROP_RW(LuaField, resource_amount),
> +	PROP_RO(LuaField, starting_resource_amount),

that is a useful addition for scripters in any way, so I support it being added :).

>  	PROP_RO(LuaField, claimers),
>  	PROP_RO(LuaField, owner),
>  	{nullptr, nullptr, nullptr},
> @@ -3660,10 +3661,21 @@
>  		report_error(L, "Illegal amount: %i, must be >= 0 and <= %i", amount, max_amount);
>  
>  	field->set_resources(res, amount);
> +	field->set_starting_res_amount(amount);
>  
>  	return 0;
>  }
> -
> +/* RST
> +	.. attribute:: starting_resource_amount
> +
> +		(RO) Starting value of resource. It is set be resource_amount
> +
> +		:see also: :attr:`resource`

this tag is not supported I think. This is layouted using http://sphinx-doc.org/

> +*/
> +int LuaField::get_starting_resource_amount(lua_State * L) {
> +	lua_pushuint32(L, fcoords(L).field->get_starting_res_amount());
> +	return 1;
> +}
>  /* RST
>  	.. attribute:: immovable
>  
> 
> === modified file 'src/scripting/lua_map.h'
> --- src/scripting/lua_map.h	2014-09-14 11:31:58 +0000
> +++ src/scripting/lua_map.h	2014-11-26 20:51:03 +0000
> @@ -942,6 +942,7 @@
>  	int set_resource(lua_State *);
>  	int get_resource_amount(lua_State *);
>  	int set_resource_amount(lua_State *);
> +	int get_starting_resource_amount(lua_State *);
>  	int get_claimers(lua_State *);
>  	int get_owner(lua_State *);
>  
> 
> === modified file 'test/maps/lua_testsuite.wmf/scripting/efield.lua'
> --- test/maps/lua_testsuite.wmf/scripting/efield.lua	2014-01-12 19:06:22 +0000
> +++ test/maps/lua_testsuite.wmf/scripting/efield.lua	2014-11-26 20:51:03 +0000
> @@ -11,5 +11,13 @@
>     assert_equal(0, self.f.resource_amount)
>  end
>  
> +function field_resources_tests:test_starting_resource_in_editor()
> +	-- making sure that (set_)	resource_amount sets also starting resource 
> +	assert_equal("coal", self.f.resource)

add a check that the starting_resource_amount is different than the resource_amount here (i.e. before you set it)

> +	self.f.resource_amount=10
> +	assert_equal(self.f.starting_resource_amount, self.f.resource_amount)
> +end
> +
> +
>  
>  
> 


-- 
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.