Code review comment for lp:~widelands-dev/widelands/remove-compatibility-wares

Revision history for this message
SirVer (sirver) wrote :

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.

review: Approve

« Back to merge proposal