← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/bug-1657117-fishbreeder-messages into lp:widelands

 

Nice branch, I like the change. Testing with the provided save game showed both messages. Also, the code looks okay to me (as far as I understand it). Quite complicated, but I can't come up with a better way.
I have two comments, though:

1) The send message says "if there are some fish left". This is wrong technically as well as from the perspective of the user. When my fishers don't find any more fish I can still build a fish breeder and breed new ones. I am afraid I have no real idea how to say this in a more correct way. Correct would be "when there have been fish before" but this too sounds strange. Maybe "when the water is favorable for fish"? Unfortunately, the user can't really discern between "there once were fish" water and no-fish water. So maybe just keep it as it is.

2) The lua-helptext for the new "resource_not_needed_message" option reads as if all production sites can receive this option. At the moment, it only works for fishers and not for foresters which have the same problem (and maybe the gamekeeper, too?). I think the help text has to be changed to say where it applies (currently only "worker=breed"). While we are at it: Maybe add support for the forester, too, and change the names of the enums?
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1657117-fishbreeder-messages/+merge/317028
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1657117-fishbreeder-messages into lp:widelands.


References