The proposal to merge lp:~widelands-dev/widelands/bug-768826 into lp:widelands
has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-768826/+merge/242544
--
Your team Widelands Developers is subscribed to branch
lp:~
No problem. Let me know if you need help with the Lua tests, I have written a
few simple ones for the building descriptions.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-768826/+merge/242544
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-7688
OK, and thanks for saving me from this work, I am struggling with that LUA
testing now...
--
https://code.launchpad.net/~widelands-dev/widelands/bug-768826/+merge/242544
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-768826.
__
Review: Approve
They are, except for the first variable. That looks weird, so I have removed
it. Going to merge now.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-768826/+merge/242544
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-768826.
__
re: "- In the editor, remove the blank space between ( and the first coordinate"
Talking about this line?
static boost::format node_format("(%3i, %3i, %2i)");
Because there is no space. But I can say that numbers between % and i are
flatly ignored, if you know how to make them work...
--
https
LGTM, except for a couple more nits. One functionality change:
- In the editor, remove the blank space between ( and the first coordinate
And two code style nits:
- add operator padding to (is_game)?25:5 (codecheck doesn't catch this case,
but it should look like this: (is_game) ? 25 : 5)
- P
@GunChleoc: done, can check it now :)
--
https://code.launchpad.net/~widelands-dev/widelands/bug-768826/+merge/242544
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-768826.
___
Mailing list: https://launchpad.net
I have had a liik at the code now - maybe you could move the dst.blit( part
outside the if/else again, this way we get less code. const std::string
node_text will then also need to be declared outside and lose the const.
When you remove the gametime from the editor, don't forget to move the FPS
OK, going to hide it
--
https://code.launchpad.net/~widelands-dev/widelands/bug-768826/+merge/242544
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-768826.
___
Mailing list: https://launchpad.net/~widelands-dev
P
No, that was just me being lazy when I added it to the debug screen.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-768826/+merge/242544
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-768826.
___
Mail
Also, I can see that the elapsed time is shown in editor - is it needed?
--
https://code.launchpad.net/~widelands-dev/widelands/bug-768826/+merge/242544
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-768826.
___
Hey, I implemented your comments. Also now the height is shown only in editor.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-768826/+merge/242544
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-768826.
__
Review: Needs Fixing
draw_overlay needs to know if it's drawing for the editor or for a game/replay.
I don't know how the control flow is here and what is accessible from the
function; you might need to add a function parameter.
Some more comments in the diff.
Diff comments:
> === modified fi
TiborB has proposed merging lp:~widelands-dev/widelands/bug-768826 into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #768826 in widelands: "Show altitude level in the editor"
https://bugs.launchpad.net/widelands/+bug/768826
For more details, see:
14 matches
Mail list logo
