Arty has proposed merging
lp:~widelands-dev/widelands/bug-1753230-working-with-tempfiles into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1753230 in widelands: "Scenarios: Some files get removed on autosave"
https://bugs.launchpad.net
Arty has proposed merging lp:~widelands-dev/widelands/filesystem-errors into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/filesystem-errors/+merge/358219
--
Your team Widelands Developers
Arty has proposed merging lp:~widelands-dev/widelands/robust-saving into
lp:widelands with
lp:~widelands-dev/widelands/bug-1753230-working-with-tempfiles as a
prerequisite.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands
Oops, I just realized that I my commit log isn't quite correct.
GameMainMenuSaveGame actually uses GenericSaveHandler directly, not via
SaveHandler. I did it this way, so the formatted message (to show the player if
something went wrong) could be accessed without having to change SaveHandler to
1. Regarding the autosave stuff:
This wasn't anything this branch tried to solve. It only solved the the
conflicts that arose from trying to modify files where the map fs pointed to,
resulting in either some crash or error (if the file was currently locked) or
resulting in corrupt savefiles.
Btw, this autosave-thing you observed was a (I think even relatively recently
introduced and somewhat dirty) error handling attempt by someone. When the
wl_autosave_00.wgf couldn't be renamed (for backup purposes) then there was
some "let's just try other filenames" approach, and the game would
That keypresses affect the game window from the save menu is a more general
issue that should be resolved. Sure, the name edit box grabs the keys when it
has the focus, but as soon as the filename table is focued (which happends
naturally when someone selects a name there), key handling (aside
Or rather than letting the save menu (and possibly some others) deal with
"let's not accidentally refer key handling to the game window" issues, maybe
the game window needs a "state" variable to distinguish between "game is
running normally", "game is paused in background behind some other
Bug report is up.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1800182-focus-save-menu/+merge/358284
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1800182-focus-save-menu.
___
Mailing list:
Arty has proposed merging lp:~widelands-dev/widelands/robust-file-saving into
lp:widelands with
lp:~widelands-dev/widelands/bug-1753230-working-with-tempfiles as a
prerequisite.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net
Disclosure: I actually don't speak the language nor do I know anyone who does.
I only stumbled upon this by accident when I checked RTL stuff and found weird
behaviour: The language entry in the options menu was arabic looking and RTL,
but all the translations were in Latin script and - even
Arty has proposed merging lp:~widelands-dev/widelands/RTL-alignment-fix into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/RTL-alignment-fix/+merge/358596
--
Your team Widelands Developers
Arty has proposed merging lp:~widelands-dev/widelands/fix-language-entry-ms
into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/fix-language-entry-ms/+merge/358597
--
Your team Widelands
Arty has proposed merging lp:~widelands-dev/widelands/bugfix-1801208 into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1801208 in widelands: "Message boxes with long unbreakable strings show
empty "
https://bugs.launchpad.net/wide
regarding code review:
There're plenty more long lines and if-blocks without curly braces in there.
I'll fix this tonight.
Do we generally follow a strict rule of curly braces even for short single line
blocks?
--
Oh okay, then I'll leave it.
I am still somewhat fuzzy about some specifics with bazaar, specifically merge
conflicts.
Let's say for examply I'd fix all those little things in disk_filesystem.cc,
then merge the result into my two child branches (which itself should be fine
because those don't
Arty has proposed merging lp:~widelands-dev/widelands/robust-file-saving into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/robust-file-saving/+merge/358718
--
Your team Widelands Developers
I admit the bitmask stuff is still my default way from before C++11, without
having to re-define operators or typecast all the time. But you are right, I
should do this properly with enum class now. I'll do that.
As for the code duplication between the game saving and map saving, I feel that
Fixed those things.
Sure, I can do code reviews. I have looked at other active reviews before but
mostly ignored them because they were either trivial one-line fixes or part of
the code I wasn't familiar with at all yet.
Anything in particular I should look out for in code reviews?
Diff
Arty has proposed merging
lp:~widelands-dev/widelands/handling-various-fs-errors into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1798816 in widelands: "Replay menu does not respect Show filenames after
deleting a replay&quo
Some of the "handling" is just catching and logging.
In GameClient::handle_packet, when renaming a file to a backup fails, we could
possibly handle this by finding another filename to save to, but I think this
would have to be agreed upon by the participating clients. I didn't attempt
this,
Huh? I had already fixed them. Did I miss something?
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1753230-working-with-tempfiles/+merge/357656
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1753230-working-with-tempfiles.
Oh, I see you had some other nits that you fixed. Fair enough. I should have
checked more thoroughly. Or did you mention some specifics before somewhere and
I had missed them?
As for the naming of the temp dir, I also had it named "temp" first but felt
that might encourage players to just
I am not actually sure there is a place where this currently makes any
difference visually.
Most text areas are used in automove mode and automatically change their
position and size based on the text, so text alignment doesn't matter unless
they are resized from the outside. In the cases
Good to know, thanks. So I guess I shouldn't even make any child branches at
all before the parent branch is fully revised and scheduled for merging into
trunk. Or maybe just not push them online or at least not bother anyone else
with them via merge requests; and then later resolve merge
Oops, forgot to enable codecheck again. Will be fixed in the next round.
--
https://code.launchpad.net/~widelands-dev/widelands/robust-file-saving/+merge/358718
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/robust-file-saving into
Merry Christmas, and thanks for testing.
As for "ways to break it", there *should* at least be no new ways to do so.
After all, the main purpose of this branch is to prevent any such breaking when
we save maps/games. (And there aren't a lot of new things that could break on
their own.) The
Arty has proposed merging
lp:~widelands-dev/widelands/bugfix-1805042-S2map-preloader-checks into
lp:widelands.
Commit message:
validity checks for S2 map headers to avoid crashes when preloading invalid S2
map files
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug
The proposal to merge lp:~widelands-dev/widelands/io-dependency-fix into
lp:widelands has been updated.
Commit message changed to:
Remove a few superfluous includes which also breaks a small dependency cycle
For more details, see:
Review: Approve
Looks good now.
The comment with the assciativity is kinda obsolete now though.
And using int64_t wasn't really necessary now that you use the weights linearly.
--
https://code.launchpad.net/~widelands-dev/widelands/terrain_affinity_as_int/+merge/358299
Your team Widelands
Review: Needs Fixing
I reviewed the code and also tested somewhat. Still has a few issues, most of
them minor though. See diff comments.
The file data\world\immovables\bush1\init.lua still has a few comments (lines
86-94) referring to the floating point values, this should also be changed in
Code looks good in principle, but haven't tested yet whether it really works as
intended.
Two minor nits:
1) see diff comment
2) All the new lines (except when they were copied over) are indented with
spaces instead of tabs.
Diff comments:
> === modified file
Arty has proposed merging lp:~widelands-dev/widelands/io-dependency-fix into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/io-dependency-fix/+merge/359658
--
Your team Widelands Developers
Review: Needs Fixing
Reviewed the code and tested quite a lot. Looks pretty good, but still doesn't
work quite right. Not hard to fix though.
1) Now that you changed the design to requiring the buildcost table to be empty
instead of listing wares with amount 0, the dismantle button does not
Yes, the exponential function is the problem. Approximating it with highish
precision using only integer calculations would require a lot of computation,
which isn't really feasible. There are reasonable options though if we don't
mind straying a little. The function we use here is basically a
I got ninja'd yesterday when I reviewed the code. :-)
As for the "needs positive return on dismantle" condition, I don't have a
strong opinion, but I'd prefer to not have it, even though in most
circumstances dismantling without return would be useless. My reasons:
1) The condition - even
Switched to a type safe error type.
--
https://code.launchpad.net/~widelands-dev/widelands/robust-file-saving/+merge/358718
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/robust-file-saving into lp:widelands.
Started reviewing the code. Have one diff comment so far. Will continue later
tonight.
Diff comments:
>
> === added file 'src/logic/map_objects/draw_text.h'
> --- src/logic/map_objects/draw_text.h 1970-01-01 00:00:00 +
> +++ src/logic/map_objects/draw_text.h 2018-11-24 07:43:42 +
> @@
Code LGTM, just some minor indentation irregularities (see comments below).
Not tested because I don't have a working Linux system atm.
Diff comments:
> === modified file 'src/base/i18n.cc'
> --- src/base/i18n.cc 2018-08-12 06:47:11 +
> +++ src/base/i18n.cc 2018-11-16 06:30:53 +
> @@
I finished looking over it but admittedly not too deeply because it's just old
code re-enabled. Code looks fine (aside from the ~ operator mentioned in the
diff comment).
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1796364-blinking-buildings/+merge/359348
Your team Widelands
Sorry, I am very busy lately and was not able to do anything Widelands-related
or even look here at all. This will likely continue for quite some time, but
I'll have a look at it this weekend.
--
https://code.launchpad.net/~widelands-dev/widelands/robust-file-saving/+merge/358718
Your team
41 matches
Mail list logo
