widelands-dev team mailing list archive

[Benedikt Straub <benedikt-straub@xxxxxx>]

Replied to the diff comment.
Also found and fixed another flaw in the inputqueuedisplay.

Diff comments:

> 
> === modified file 'src/logic/playercommand.cc'
> --- src/logic/playercommand.cc	2019-06-23 11:41:17 +0000
> +++ src/logic/playercommand.cc	2019-06-23 14:34:42 +0000
> @@ -1131,39 +1139,43 @@
>                                         PlayerImmovable& imm,
>                                         const DescriptionIndex index,
>                                         const WareWorker type,
> -                                       const uint32_t max_fill)
> +                                       const uint32_t max_fill,
> +                                       bool cs_setting)
>     : PlayerCommand(init_duetime, init_sender),
>       serial_(imm.serial()),
>       index_(index),
>       type_(type),
> -     max_fill_(max_fill) {
> +     max_fill_(max_fill),
> +     is_constructionsite_setting_(cs_setting) {
>  }
>  
>  void CmdSetInputMaxFill::execute(Game& game) {
>  	MapObject* mo = game.objects().get_object(serial_);
> -	if (upcast(ConstructionSite, cs, mo)) {
> -		if (upcast(ProductionsiteSettings, s, cs->get_settings())) {
> -			switch (type_) {
> -			case wwWARE:
> -				for (auto& pair : s->ware_queues) {
> -					if (pair.first == index_) {
> -						assert(pair.second.max_fill >= max_fill_);
> -						pair.second.desired_fill = max_fill_;
> -						return;
> -					}
> -				}
> -				NEVER_HERE();
> -			case wwWORKER:
> -				for (auto& pair : s->worker_queues) {
> -					if (pair.first == index_) {
> -						assert(pair.second.max_fill >= max_fill_);
> -						pair.second.desired_fill = max_fill_;
> -						return;
> -					}
> +	if (is_constructionsite_setting_) {
> +		if (upcast(ConstructionSite, cs, mo)) {

This cannot be asserted because the constructionsite might have been destroyed or completed in the short interval between sending and executing the playercommand

> +			if (upcast(ProductionsiteSettings, s, cs->get_settings())) {
> +				switch (type_) {
> +				case wwWARE:
> +					for (auto& pair : s->ware_queues) {
> +						if (pair.first == index_) {
> +							assert(pair.second.max_fill >= max_fill_);
> +							pair.second.desired_fill = max_fill_;
> +							return;
> +						}
> +					}
> +					NEVER_HERE();
> +				case wwWORKER:
> +					for (auto& pair : s->worker_queues) {
> +						if (pair.first == index_) {
> +							assert(pair.second.max_fill >= max_fill_);
> +							pair.second.desired_fill = max_fill_;
> +							return;
> +						}
> +					}
> +					NEVER_HERE();
>  				}
>  				NEVER_HERE();
>  			}
> -			NEVER_HERE();
>  		}
>  	} else if (upcast(Building, b, mo)) {
>  		if (b->owner().player_number() == sender()) {


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