widelands-dev team mailing list archive

[Miroslav Remák <miroslavr256@xxxxxxxxx>]

See diff comments.

Diff comments:

> 
> === modified file 'src/scripting/lua_map.cc'
> --- src/scripting/lua_map.cc	2016-04-11 06:45:29 +0000
> +++ src/scripting/lua_map.cc	2016-04-16 12:42:55 +0000
> @@ -1871,10 +1871,16 @@
>  /* RST
>  	.. attribute:: workarea_radius
>  
> -			(RO) the workarea_radius of the building as an int.
> +			(RO) the first workarea_radius of the building as an int, 
> +                 0 in case bulding has no workareas
>  */
>  int LuaBuildingDescription::get_workarea_radius(lua_State * L) {
> -	lua_pushinteger(L, get()->workarea_info_.begin()->first);
> +    const WorkareaInfo& workareaInfo = get()->workarea_info_;

Please use tabs for indentation.

> +    if (!workareaInfo.empty()) {
> +        lua_pushinteger(L, workareaInfo.begin()->first);
> +    } else {
> +        lua_pushinteger(L, 0);

I think it would be safer to use lua_pushnil here to indicate that there is NO work area defined. Returning 0 could be understood as a work area with 0 radius, even if it may not make much sense. This behavior is also consistent with many of our other functions. You don't have to modify any Lua code because it already checks for a nil value.

> +    }
>  	return 1;
>  }
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands.