widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Needs Fixing



Diff comments:

> 
> === modified file 'src/logic/player.h'
> --- src/logic/player.h	2016-10-21 08:21:41 +0000
> +++ src/logic/player.h	2017-01-07 20:28:54 +0000
> @@ -522,6 +522,8 @@
>  		return economies_.size();
>  	}
>  
> +	uint32_t get_current_produced_statistics_(uint8_t);

please add a comment. And what is up with the '_' at the end of the function name?

> +
>  	// Military stuff
>  	void drop_soldier(PlayerImmovable&, Soldier&);
>  	void change_training_options(TrainingSite&, TrainingAttribute attr, int32_t val);
> 
> === modified file 'src/scripting/lua_game.cc'
> --- src/scripting/lua_game.cc	2016-11-30 20:02:32 +0000
> +++ src/scripting/lua_game.cc	2017-01-07 20:28:54 +0000
> @@ -872,6 +873,67 @@
>  	return 0;
>  }
>  
> +/* RST
> +   .. method:: produced_wares_count(what)
> +
> +      Returns count of wares produced byt the player up to now.
> +      'what' can be either an "all" or single ware name or an array of names. If single
> +      ware name is given, integer is returned, otherwise the table is returned.
> +     
> +*/
> +int LuaPlayer::get_produced_wares_count(lua_State* L) {

as discussed in the forums, I think you should reuse the parsing functions that are already there and not recode the logic here again.

> +	Player& p = get(L, get_egbase(L));
> +	const TribeDescr& tribe = p.tribe();
> +	int32_t nargs = lua_gettop(L);
> +	if (nargs != 2) {
> +		report_error(L, "One argument is required for produced_wares_count()");
> +	}
> +	std::vector<DescriptionIndex> requested_wares;
> +	DescriptionIndex single_ware = INVALID_INDEX;
> +	if (lua_isstring(L, 2)) {
> +		std::string what = luaL_checkstring(L, -1);
> +		if (what != "all") {
> +			single_ware = tribe.ware_index(what);
> +			if (single_ware == INVALID_INDEX) {
> +				report_error(L, "Unrecognized ware %s", what.c_str());
> +			}
> +		}
> +	} else {
> +		/* array of names */
> +		luaL_checktype(L, 2, LUA_TTABLE);
> +		lua_pushnil(L);
> +		while (lua_next(L, 2) != 0) {
> +			std::string what = luaL_checkstring(L, -1);
> +			lua_pop(L, 1);
> +			requested_wares.push_back(tribe.ware_index(what));
> +			if (requested_wares.back() == INVALID_INDEX) {
> +				report_error(L, "Unrecognized ware %s", what.c_str());
> +			}
> +		}
> +	}
> +
> +	if (single_ware != INVALID_INDEX) {
> +		lua_pushuint32(L, p.get_current_produced_statistics_(single_ware));
> +	} else if (requested_wares.empty()) {
> +		// we return all wares
> +		lua_newtable(L);
> +		for (const DescriptionIndex& idx : tribe.wares()) {
> +			lua_pushstring(L, tribe.get_ware_descr(idx)->name());
> +			lua_pushuint32(L, p.get_current_produced_statistics_(idx));
> +			lua_settable(L, -3);
> +		}
> +	} else {
> +		// we return requested wares
> +		lua_newtable(L);
> +		for (const DescriptionIndex& idx : requested_wares) {
> +			lua_pushstring(L, tribe.get_ware_descr(idx)->name());
> +			lua_pushuint32(L, p.get_current_produced_statistics_(idx));
> +			lua_settable(L, -3);
> +		}
> +	}
> +	return 1;
> +}
> +
>  /*
>   ==========================================================
>   C METHODS
> 
> === modified file 'test/maps/expedition.wmf/scripting/init.lua'
> --- test/maps/expedition.wmf/scripting/init.lua	2016-01-28 05:24:34 +0000
> +++ test/maps/expedition.wmf/scripting/init.lua	2017-01-07 20:28:54 +0000
> @@ -316,6 +316,7 @@
>     hq:set_wares("log", 100)
>     port:set_wares("blackwood", 100)
>  
> +   log_starting_count = p1:get_produced_wares_count("log")

Make a separate test instead of putting this into the seafaring scenario?

>  
>     port:start_expedition()
>     wait_for_message("Expedition")


-- 
https://code.launchpad.net/~widelands-dev/widelands/production_statistics/+merge/314283
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/production_statistics.