← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/remove-compatibility-wares into lp:widelands

 

Review: Approve

Looked over the FIXMEs and the merge request in general. LGTM. Some comments on the FIXMEs

The first was the problem that name was a plain old c string (i.e. a pointer to an array of characters). Constructing a c++ std::string out of it gives you access to more utility methods. Generally speaking, const char* should not show up in our code - they are pretty legacy. I refactored the area around that code to use std::string, instead of const char *.

The second FIXME was a dirty hack that would fail a workers program if it was currently running a program that was removed from the engine. It did this to check for program_name=fail in the conf file, if this was found, a function was queued that would be executed as soon as loading was done and that would send the worker a fail signal. The worker would then go to its 'default action' which is usually running to a warehouse. This code is no longer necessary and it made a bunch of other methods unnecessary too. I removed them.

If you think the changes are fine, feel free to merge.

-- 
https://code.launchpad.net/~widelands-dev/widelands/remove-compatibility-wares/+merge/210885
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/remove-compatibility-wares.


References