widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

> * When setting some options I get:
> 
> [] Section [global], key 'depth' not used (did you spell the name correctly?)
> [] Section [global], key 'ui_font' not used (did you spell the name
> correctly?)
> [] Section [global], key 'speed_of_new_game' not used (did you spell the name
> correctly?)
> [] Section [global], key 'remove_replays' not used (did you spell the name
> correctly?)
> [] Section [global], key 'remove_syncstreams' not used (did you spell the name
> correctly?)
> [] Section [global], key 'opengl' not used (did you spell the name correctly?)
> 
> But that may  work as intended.

When you get something that's seemingly unrelated, test it on trunk as well - I bet it's the same.

> * OK, I cannot use an old replay, as of UnhandledVersionError

We broke savegame compatibility with

http://bazaar.launchpad.net/~widelands-dev/widelands/trunk/revision/7954


> * Got a crash in the regression tests, will add info for this later

This definitely needs fixing.

I also replied to your diff comments.

Diff comments:

> 
> === modified file 'src/editor/map_generator.cc'
> --- src/editor/map_generator.cc	2016-04-06 09:23:04 +0000
> +++ src/editor/map_generator.cc	2016-04-11 07:07:30 +0000
> @@ -130,9 +130,9 @@
>  	const auto set_resource_helper = [this, &world, &terrain_description, &fc] (
>  	   const uint32_t random_value, const int valid_resource_index) {
>  		const DescriptionIndex  res_idx = terrain_description.get_valid_resource(valid_resource_index);
> -		const uint32_t max_amount = world.get_resource(res_idx)->max_amount();
> -		uint8_t res_val = static_cast<uint8_t>(random_value / (kMaxElevation / max_amount));
> -		res_val *= static_cast<uint8_t>(map_info_.resource_amount) + 1;
> +		const ResourceAmount max_amount = world.get_resource(res_idx)->max_amount();
> +		ResourceAmount res_val = static_cast<ResourceAmount>(random_value / (kMaxElevation / max_amount));
> +		res_val *= static_cast<ResourceAmount>(map_info_.resource_amount) + 1;

If the old casts would have limited the values, that would have been a bug IMO. So, no problem if the calculation changed here.

>  		res_val /= 3;
>  		if (map_.is_resource_valid(world, fc, res_idx)) {
>  			map_.initialize_resources(fc, res_idx, res_val);
> 
> === modified file 'src/editor/tools/set_resources_tool.cc'
> --- src/editor/tools/set_resources_tool.cc	2016-04-06 09:23:04 +0000
> +++ src/editor/tools/set_resources_tool.cc	2016-04-11 07:07:30 +0000
> @@ -68,11 +68,9 @@
>                                           EditorActionArgs* args,
>                                           Widelands::Map* map) {
>  	for (const auto & res : args->original_resource) {
> -		int32_t amount     = res.amount;
> -		int32_t max_amount = world.get_resource(args->current_resource)->max_amount();
> +		Widelands::ResourceAmount amount     = res.amount;
> +		Widelands::ResourceAmount max_amount = world.get_resource(args->current_resource)->max_amount();
>  
> -		if (amount < 0)
> -			amount = 0;

These lines should not have been deleted - I'll add them back in.

>  		if (amount > max_amount)
>  			amount = max_amount;
>  
> 
> === modified file 'src/logic/map.cc'
> --- src/logic/map.cc	2016-04-06 09:23:04 +0000
> +++ src/logic/map.cc	2016-04-11 07:07:30 +0000
> @@ -1869,7 +1869,7 @@
>  }
>  
>  bool Map::is_resource_valid
> -	(const Widelands::World& world, const TCoords<Widelands::FCoords>& c, int32_t const curres)
> +	(const Widelands::World& world, const TCoords<Widelands::FCoords>& c, DescriptionIndex curres)

It's a basic datatype, so const doesn't matter in the interface here. We actually have a codecheck rule in place to weed them out, but it doesn't catch all instances.

>  {
>  	if (curres == Widelands::kNoResource)
>  		return true;


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1545243-plnum-lua/+merge/291481
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1545243-plnum-lua.