widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

I like the implementation :) - some nits though.

There are 2 more things that we need to look at:

1. Double-check the worker programs to see how the geologists distinguish between searching for water vs. mountain resources, to make sure that this is not hard-coded on the resource type. Maybe this works on terrain type already anyway, but I don't remember right now.

2. Resource indicators - how do we ensure which resource gets linked to which resource indicator, and that the tribes have one for each resource? Maybe we can find a solution here that's more clever, i.e. moving their definitions from the tribes to the world. An additional check during tribe loading might also be sufficient here, in case we would like to have the possibility of individual graphics per tribe in the future.

Point 1 should be looked into for this branch, point 2 can be dealt with separately.

Diff comments:

> === modified file 'data/world/resources/init.lua'
> --- data/world/resources/init.lua	2017-08-30 17:07:20 +0000
> +++ data/world/resources/init.lua	2018-07-24 19:34:27 +0000
> @@ -50,6 +50,18 @@
>  --
>  --            detectable = true,
>  --
> +--    **timeout_millis**

I'd prefer to have this either spelled out (timeout_milliseconds) or use the default measuring unit (timeout_ms)

> +--        *Mandatory*. Defines the time for which geologists messages for this
> +--        resource will be muted within this area after a find, e.g.::
> +--
> +--            timeout_millis = 30000,
> +--
> +--    **timeout_radius**
> +--        *Mandatory*. Defines the radius within which geologists messages for this
> +--        resource will be muted after a find, e.g.::
> +--
> +--            timeout_radius = 8,
> +--
>  --    **representative_image**
>  --        *Mandatory*. Path to an image file that will represent the resource in menus
>  --        etc., e.g.::
> @@ -142,6 +164,8 @@
>     descname = _ "Fish",
>     max_amount = 20,
>     detectable = false,
> +   timeout_millis = 0,

We should have a timeout here too, in case the player missed it and it ends up buried below lots of other messages. All messages will be deleted anyway when the building is destroyed or dismantled.

> +   timeout_radius = 0,
>     representative_image = pics_dir .. "fish.png",
>     editor_pictures = {
>        [5] = pics_dir .. "fish1.png",
> 
> === modified file 'src/logic/message.h'
> --- src/logic/message.h	2018-04-07 16:59:00 +0000
> +++ src/logic/message.h	2018-07-24 19:34:27 +0000
> @@ -129,15 +131,13 @@
>  		} else if (type_ >= Widelands::Message::Type::kEconomy &&
>  		           type_ <= Widelands::Message::Type::kEconomySiteOccupied) {
>  			return Widelands::Message::Type::kEconomy;
> -		} else if (type_ >= Widelands::Message::Type::kGeologists &&
> -		           type_ <= Widelands::Message::Type::kGeologistsWater) {
> -			return Widelands::Message::Type::kGeologists;
>  		}
>  		return type_;
>  	}
>  
>  private:
>  	Message::Type type_;
> +	const char* type_detail_;

How about calling this "subtype_"?

>  	const std::string title_;
>  	const std::string icon_filename_;
>  	const Image* icon_;  // Pointer to icon into picture stack


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