← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

 

Yep, I liked the beer too - it's a balancing thing tough.

2) A flag sounds good. The reason for the inconsistency is that until now, this was used to fill working positions, where allowing higher ranking workers is important.

3) It can be resolved like this:

    bool found = false;
    for (const WareAmount& input : inputs) {
        if (input.first == ware_index) {
            count_max += input.second;
            found = true;
            break;
        }
    }
    if (!found) {
        throw GameDataError("%s is not declared as an input (\"%s=<count>\" was not "
                            "found in the [inputs] section)",
                            ware, ware);
    }

We generally try to get away from using iterators unless they are necessary, because it makes the code easier to read. We could also get rid of having the loop twice:

    WareWorker type = wwWARE;
    DescriptionIndex input_index = tribes.ware_index(ware);
    if (!tribes.ware_exists(input_index)) {
        input_index = tribes.worker_index(ware);
        if (!tribes.worker_exists(input_index)) {
            throw GameDataError("Unknown ware or worker type \"%s\"", ware);
        } else {
            // It is a worker
            type = wwWORKER;
        }
    }
    // Now loop

4) NOCOM is short for "no commit" - all NOCOMs have to be removed before merging into trunk, either by fixing the issue or turning them into TODO comments. TODO comments have a uniform format ti make it easier to search for them:

    // TODO(<nick>): Juicy comment

5) We are now saving things to the savegame that weren't there before. This will crash Widelands if we try to load the savegame with an older version. So, we use exceptions here to show a message to the user and abort loading the savegame. We got rid of compatibility code during the last release cycle, so old savegames won't load any more. We're providing compatibility again from now on though, so we check the range.
-- 
https://code.launchpad.net/~widelands-dev/widelands/casern_workersqueue/+merge/309763
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands.


References