Code review comment for lp:~widelands-dev/widelands/toggle_immovables

Revision history for this message
toptopple (7010622-q-deactivatedaccount) wrote :

Very nice! I don't see a technical problem which hasn't been there in the unmodified program. :)
When I look into the specs of this task, however, there is no mention of the Animals class. So with your implementation we actually are dealing with an over-fulfillment of the task.

When I was designing the task, I didn't think of the Animals as something different to Immovables, probably because they don't move in the editor and better handling of the editor was my only purpose. We have to re-think. Anyways, short-key 'a' was targeted for the Animals tool and hence is placed falsely here (for the VIEW). I surmise we should discuss this matter in the task thread first and then continue here.

« Back to merge proposal