widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Approve

:). I do not really know myself where to draw the line - it costs the reviewer very little to read a few lines but having the delay in the push process is sometimes annoying for the coder. I pushed some bug fixes yesterday also without reviews... so, I just don't know.

if (Building * building = flag->get_building()) -> I once was bitten by a refactoring bug which didn't handle assignements in if correctly Since them I prefer to split this out into two lines - just to be safe. Your call.
FindImmovable finder_; -> Can be const I think.
CheckStepWalkOn is now CheckStepDefault -> Can you elaborate? I think this changes the semantics of when you can attack, or not?

otherwise lgtm.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug995011/+merge/147776
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug995011.