Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bugfix-buildings-tooltips into lp:widelands
1 nit, not tested yet.
Diff comments:
> === modified file 'src/logic/map_objects/tribes/production_program.cc'
> --- src/logic/map_objects/tribes/production_program.cc2019-04-24
> 06:01:37 +
> +++ src/logic/map_objects/tribes/production_program.cc2019-04-27
> 13:34:24 +
> @@ -1415,6 +1415,7 @@
> throw wexception("Fail training soldier!!");
> }
> ps.molog(" Training done!\n");
> + ps.set_production_result((boost::format(_("Completed %u")) %
> ps.top_state().program->descname()).str());
You need to use %s and call c_str() on the descname to turn it into a const
char. %u is for unsigned integers.
>
> upcast(TrainingSite, ts, &ps);
> ts->training_successful(attribute, level);
--
https://code.launchpad.net/~widelands-dev/widelands/bugfix-buildings-tooltips/+merge/366607
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bugfix-buildings-tooltips into lp:widelands.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug1642882-clear-space-in-port-vicinity into lp:widelands
Tested. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug1642882-clear-space-in-port-vicinity/+merge/366599 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug1642882-clear-space-in-port-vicinity. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/per-level-soldier-anims into lp:widelands
Maps need a comparison operator, because the elements are sorted. You could try out http://www.cplusplus.com/reference/unordered_map/unordered_map/ -- https://code.launchpad.net/~widelands-dev/widelands/per-level-soldier-anims/+merge/354929 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/per-level-soldier-anims into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug1642882-clear-space-in-port-vicinity into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug1642882-clear-space-in-port-vicinity into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug1642882-clear-space-in-port-vicinity/+merge/366599 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug1642882-clear-space-in-port-vicinity. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange into lp:widelands/build20
Klaus Halfmann has proposed merging
lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange into
lp:widelands/build20.
Commit message:
Fix for #1826669
Requested reviews:
Toni Förster (stonerl): remote network test
Related bugs:
Bug #1826669 in widelands: "R20-rc1 HeapUseAfterFree when changig Map during
Download"
https://bugs.launchpad.net/widelands/+bug/1826669
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug_1826669_MapDownloandChange/+merge/366616
Avoid double delete of _file in gameclient.cc
--
Your team Widelands Developers is subscribed to branch lp:widelands/build20.
=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc 2019-02-23 11:00:49 +
+++ src/network/gameclient.cc 2019-04-28 11:30:30 +
@@ -586,8 +586,10 @@
d->settings.mapfilename.c_str());
// New map was set, so we clean up the buffer of a previously requested file
- if (file_)
+if (file_) {
delete file_;
+file_ = NULL;
+}
break;
}
@@ -637,8 +639,9 @@
s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
d->net->send(s);
- if (file_)
+ if (file_) {
delete file_;
+ }
file_ = new NetTransferFile();
file_->bytes = bytes;
=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc 2019-02-23 11:00:49 +
+++ src/network/gamehost.cc 2019-04-28 11:30:30 +
@@ -539,7 +539,9 @@
d->net.reset();
d->promoter.reset();
delete d;
+d = NULL;
delete file_;
+file_ = NULL;
}
const std::string& GameHost::get_local_playername() const {
=== modified file 'src/network/network.h'
--- src/network/network.h 2019-02-23 11:00:49 +
+++ src/network/network.h 2019-04-28 11:30:30 +
@@ -185,6 +185,14 @@
std::string filename;
std::string md5sum;
std::vector parts;
+
+NetTransferFile() {
+// Allow Debugging
+}
+
+~NetTransferFile() {
+// Allow Debugging
+}
};
class Deserializer {
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange into lp:widelands/build20
This will merge to buld20. I will try to do a bigger refactoring for trunk, but we should merge it there, too. -- https://code.launchpad.net/~widelands-dev/widelands/bug_1826669_MapDownloandChange/+merge/366616 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange into lp:widelands/build20
Review: Approve testing, playing, compiling Tested with Klaus and this fixes the crash. -- https://code.launchpad.net/~widelands-dev/widelands/bug_1826669_MapDownloandChange/+merge/366616 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/per-level-soldier-anims into lp:widelands
Thanks for the suggestion, replaced it with unordered_map and removed the operator<. I´m now using unique_ptr as key or the compiler warns about unsupported hash functions… -- https://code.launchpad.net/~widelands-dev/widelands/per-level-soldier-anims/+merge/354929 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/per-level-soldier-anims into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands.
Commit message:
Fix crash in NetClient when host changes their mind about a custom map in
mid-transfer.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1826669 in widelands: "R20-rc1 HeapUseAfterFree when changig Map during
Download"
https://bugs.launchpad.net/widelands/+bug/1826669
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366617
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands.
=== added file 'WL_RELEASE'
--- WL_RELEASE 1970-01-01 00:00:00 +
+++ WL_RELEASE 2019-04-28 14:24:30 +
@@ -0,0 +1,1 @@
+build-20-rc1
=== modified file 'appveyor.yml'
--- appveyor.yml 2019-04-25 15:49:31 +
+++ appveyor.yml 2019-04-28 14:24:30 +
@@ -38,16 +38,20 @@
- cmd: cd %APPVEYOR_BUILD_FOLDER%
- cmd: md build
- cmd: cd build
+<<< TREE
- cmd: echo %APPVEYOR_BUILD_VERSION%_%CONFIGURATION%_%PLATFORM% > %APPVEYOR_BUILD_FOLDER%\WL_RELEASE
- cmd: "IF \"%PLATFORM%\" == \"x86\" (cmake -G \"Ninja\" -DBoost_NO_BOOST_CMAKE=ON -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DOPTION_USE_GLBINDING=OFF -DOPTION_BUILD_WEBSITE_TOOLS=OFF -DOPTION_ASAN=OFF -DCMAKE_JOB_POOLS=\"linking=1\" -DCMAKE_JOB_POOL_LINK=linking %APPVEYOR_BUILD_FOLDER%) ELSE (cmake -G \"Ninja\" -DBoost_NO_BOOST_CMAKE=ON -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DOPTION_USE_GLBINDING=ON -DOPTION_BUILD_WEBSITE_TOOLS=OFF -DOPTION_ASAN=OFF -DCMAKE_JOB_POOLS=\"linking=1\" -DCMAKE_JOB_POOL_LINK=linking %APPVEYOR_BUILD_FOLDER%)"
+===
+ - cmd: "IF \"%PLATFORM%\" == \"x86\" (cmake -G \"Ninja\" -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DOPTION_USE_GLBINDING=OFF -DOPTION_BUILD_WEBSITE_TOOLS=OFF -DOPTION_ASAN=OFF -DCMAKE_JOB_POOLS=\"linking=1\" -DCMAKE_JOB_POOL_LINK=linking %APPVEYOR_BUILD_FOLDER%) ELSE (cmake -G \"Ninja\" -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DOPTION_USE_GLBINDING=ON -DOPTION_BUILD_WEBSITE_TOOLS=OFF -DOPTION_ASAN=OFF -DCMAKE_JOB_POOLS=\"linking=1\" -DCMAKE_JOB_POOL_LINK=linking %APPVEYOR_BUILD_FOLDER%)"
+>>> MERGE-SOURCE
- cmd: "cmake --build ."
on_success:
- cmd: strip -sv %APPVEYOR_BUILD_FOLDER%\build\src\widelands.exe
- - cmd: ISCC /q /o%APPVEYOR_BUILD_FOLDER% /fWidelands-%APPVEYOR_BUILD_VERSION%-%CONFIGURATION%-%PLATFORM% c:\projects\widelands\utils\win32\innosetup\Widelands.iss
- - appveyor PushArtifact %APPVEYOR_BUILD_FOLDER%\Widelands-%APPVEYOR_BUILD_VERSION%-%CONFIGURATION%-%PLATFORM%.exe
+ - cmd: ISCC /q /o%APPVEYOR_BUILD_FOLDER% /fWidelands-build20-rc1-%PLATFORM% c:\projects\widelands\utils\win32\innosetup\Widelands.iss
+ - appveyor PushArtifact %APPVEYOR_BUILD_FOLDER%\Widelands-build20-rc1-%PLATFORM%.exe
artifacts:
- - path: Widelands-$(APPVEYOR_BUILD_VERSION)-$(CONFIGURATION)-$(PLATFORM).exe
+ - path: Widelands-build20-rc1-$(PLATFORM).exe
name: Widelands Setup
platform:
=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc 2019-04-27 11:21:21 +
+++ src/network/gameclient.cc 2019-04-28 14:24:30 +
@@ -594,8 +594,7 @@
d->settings.mapfilename.c_str());
// New map was set, so we clean up the buffer of a previously requested file
- if (file_)
- delete file_;
+ file_.reset(nullptr);
break;
}
@@ -645,10 +644,7 @@
s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
d->net->send(s);
- if (file_)
- delete file_;
-
- file_ = new NetTransferFile();
+ file_.reset(new NetTransferFile());
file_->bytes = bytes;
file_->filename = path;
file_->md5sum = md5;
=== modified file 'src/network/gameclient.h'
--- src/network/gameclient.h 2019-03-15 15:11:57 +
+++ src/network/gameclient.h 2019-04-28 14:24:30 +
@@ -20,6 +20,8 @@
#ifndef WL_NETWORK_GAMECLIENT_H
#define WL_NETWORK_GAMECLIENT_H
+#include
+
#include "chat/chat.h"
#include "logic/game_controller.h"
#include "logic/game_settings.h"
@@ -123,7 +125,7 @@
bool sendreason = true,
bool showmsg = true);
- NetTransferFile* file_;
+ std::unique_ptr file_;
GameClientImpl* d;
bool internet_;
};
=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc 2019-03-15 15:11:57 +
+++ src/network/gamehost.cc 2019-04-28 14:24:30 +
@@ -531,7 +531,7 @@
hostuser.position = UserSettings::none();
hostuser.ready = true;
d->settings.users.push_back(hostuser);
- file_ = nullptr; // Initialize as 0 pointer - unfortunately needed in struct.
+ file_.reset(nullptr); // Initialize as 0 pointer - unfortunately needed in struct.
}
GameHost::~GameHost() {
@@ -546,7 +546,6 @@
d->net.reset();
d->promoter.reset();
delete d;
- delete file_;
}
const std::string& GameHost::get_local_playername() const {
@@ -1083,9 +1082,7 @@
// Read in the file
FileRead fr;
fr.open(*g_fs, mapfilename);
- if (file_)
- delete file_;
- file_ = new NetTransferFile()
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange into lp:widelands/build20
Oops, wrong target branch. Take this one: https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366618 -- https://code.launchpad.net/~widelands-dev/widelands/bug_1826669_MapDownloandChange/+merge/366616 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands.
Commit message:
Fix crash in NetClient when host changes their mind about a custom map in
mid-transfer.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1826669 in widelands: "R20-rc1 HeapUseAfterFree when changig Map during
Download"
https://bugs.launchpad.net/widelands/+bug/1826669
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366618
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands.
=== added file 'WL_RELEASE'
--- WL_RELEASE 1970-01-01 00:00:00 +
+++ WL_RELEASE 2019-04-28 14:26:47 +
@@ -0,0 +1,1 @@
+build-20-rc1
=== modified file 'appveyor.yml'
--- appveyor.yml 2019-04-25 15:49:31 +
+++ appveyor.yml 2019-04-28 14:26:47 +
@@ -38,16 +38,20 @@
- cmd: cd %APPVEYOR_BUILD_FOLDER%
- cmd: md build
- cmd: cd build
+<<< TREE
- cmd: echo %APPVEYOR_BUILD_VERSION%_%CONFIGURATION%_%PLATFORM% > %APPVEYOR_BUILD_FOLDER%\WL_RELEASE
- cmd: "IF \"%PLATFORM%\" == \"x86\" (cmake -G \"Ninja\" -DBoost_NO_BOOST_CMAKE=ON -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DOPTION_USE_GLBINDING=OFF -DOPTION_BUILD_WEBSITE_TOOLS=OFF -DOPTION_ASAN=OFF -DCMAKE_JOB_POOLS=\"linking=1\" -DCMAKE_JOB_POOL_LINK=linking %APPVEYOR_BUILD_FOLDER%) ELSE (cmake -G \"Ninja\" -DBoost_NO_BOOST_CMAKE=ON -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DOPTION_USE_GLBINDING=ON -DOPTION_BUILD_WEBSITE_TOOLS=OFF -DOPTION_ASAN=OFF -DCMAKE_JOB_POOLS=\"linking=1\" -DCMAKE_JOB_POOL_LINK=linking %APPVEYOR_BUILD_FOLDER%)"
+===
+ - cmd: "IF \"%PLATFORM%\" == \"x86\" (cmake -G \"Ninja\" -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DOPTION_USE_GLBINDING=OFF -DOPTION_BUILD_WEBSITE_TOOLS=OFF -DOPTION_ASAN=OFF -DCMAKE_JOB_POOLS=\"linking=1\" -DCMAKE_JOB_POOL_LINK=linking %APPVEYOR_BUILD_FOLDER%) ELSE (cmake -G \"Ninja\" -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DOPTION_USE_GLBINDING=ON -DOPTION_BUILD_WEBSITE_TOOLS=OFF -DOPTION_ASAN=OFF -DCMAKE_JOB_POOLS=\"linking=1\" -DCMAKE_JOB_POOL_LINK=linking %APPVEYOR_BUILD_FOLDER%)"
+>>> MERGE-SOURCE
- cmd: "cmake --build ."
on_success:
- cmd: strip -sv %APPVEYOR_BUILD_FOLDER%\build\src\widelands.exe
- - cmd: ISCC /q /o%APPVEYOR_BUILD_FOLDER% /fWidelands-%APPVEYOR_BUILD_VERSION%-%CONFIGURATION%-%PLATFORM% c:\projects\widelands\utils\win32\innosetup\Widelands.iss
- - appveyor PushArtifact %APPVEYOR_BUILD_FOLDER%\Widelands-%APPVEYOR_BUILD_VERSION%-%CONFIGURATION%-%PLATFORM%.exe
+ - cmd: ISCC /q /o%APPVEYOR_BUILD_FOLDER% /fWidelands-build20-rc1-%PLATFORM% c:\projects\widelands\utils\win32\innosetup\Widelands.iss
+ - appveyor PushArtifact %APPVEYOR_BUILD_FOLDER%\Widelands-build20-rc1-%PLATFORM%.exe
artifacts:
- - path: Widelands-$(APPVEYOR_BUILD_VERSION)-$(CONFIGURATION)-$(PLATFORM).exe
+ - path: Widelands-build20-rc1-$(PLATFORM).exe
name: Widelands Setup
platform:
=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc 2019-04-27 11:21:21 +
+++ src/network/gameclient.cc 2019-04-28 14:26:47 +
@@ -594,8 +594,7 @@
d->settings.mapfilename.c_str());
// New map was set, so we clean up the buffer of a previously requested file
- if (file_)
- delete file_;
+ file_.reset(nullptr);
break;
}
@@ -645,10 +644,7 @@
s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
d->net->send(s);
- if (file_)
- delete file_;
-
- file_ = new NetTransferFile();
+ file_.reset(new NetTransferFile());
file_->bytes = bytes;
file_->filename = path;
file_->md5sum = md5;
=== modified file 'src/network/gameclient.h'
--- src/network/gameclient.h 2019-03-15 15:11:57 +
+++ src/network/gameclient.h 2019-04-28 14:26:47 +
@@ -20,6 +20,8 @@
#ifndef WL_NETWORK_GAMECLIENT_H
#define WL_NETWORK_GAMECLIENT_H
+#include
+
#include "chat/chat.h"
#include "logic/game_controller.h"
#include "logic/game_settings.h"
@@ -123,7 +125,7 @@
bool sendreason = true,
bool showmsg = true);
- NetTransferFile* file_;
+ std::unique_ptr file_;
GameClientImpl* d;
bool internet_;
};
=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc 2019-03-15 15:11:57 +
+++ src/network/gamehost.cc 2019-04-28 14:26:47 +
@@ -531,7 +531,7 @@
hostuser.position = UserSettings::none();
hostuser.ready = true;
d->settings.users.push_back(hostuser);
- file_ = nullptr; // Initialize as 0 pointer - unfortunately needed in struct.
+ file_.reset(nullptr); // Initialize as 0 pointer - unfortunately needed in struct.
}
GameHost::~GameHost() {
@@ -546,7 +546,6 @@
d->net.reset();
d->promoter.reset();
delete d;
- delete file_;
}
const std::string& GameHost::get_local_playername() const {
@@ -1083,9 +1082,7 @@
// Read in the file
FileRead fr;
fr.open(*g_fs, mapfilename);
- if (file_)
- delete file_;
- file_ = new NetTransferFile()
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange into lp:widelands/build20
I have an alternative fix that will use unique_ptr. We are trying to get away from raw delete calls. Tested with 2 Ubuntu machines. https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366617 -- https://code.launchpad.net/~widelands-dev/widelands/bug_1826669_MapDownloandChange/+merge/366616 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange into lp:widelands/build20
Woohoo, forgot to tick the checkbox too. Now I finally have the correct target branch: https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619 -- https://code.launchpad.net/~widelands-dev/widelands/bug_1826669_MapDownloandChange/+merge/366616 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20.
Commit message:
Fix crash in NetClient when host changes their mind about a custom map in
mid-transfer.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1826669 in widelands: "R20-rc1 HeapUseAfterFree when changig Map during
Download"
https://bugs.launchpad.net/widelands/+bug/1826669
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20.
=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc 2019-02-23 11:00:49 +
+++ src/network/gameclient.cc 2019-04-28 14:30:33 +
@@ -586,8 +586,7 @@
d->settings.mapfilename.c_str());
// New map was set, so we clean up the buffer of a previously requested file
- if (file_)
- delete file_;
+ file_.reset(nullptr);
break;
}
@@ -637,10 +636,7 @@
s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
d->net->send(s);
- if (file_)
- delete file_;
-
- file_ = new NetTransferFile();
+ file_.reset(new NetTransferFile());
file_->bytes = bytes;
file_->filename = path;
file_->md5sum = md5;
=== modified file 'src/network/gameclient.h'
--- src/network/gameclient.h 2019-02-23 11:00:49 +
+++ src/network/gameclient.h 2019-04-28 14:30:33 +
@@ -20,6 +20,8 @@
#ifndef WL_NETWORK_GAMECLIENT_H
#define WL_NETWORK_GAMECLIENT_H
+#include
+
#include "chat/chat.h"
#include "logic/game_controller.h"
#include "logic/game_settings.h"
@@ -120,7 +122,7 @@
bool sendreason = true,
bool showmsg = true);
- NetTransferFile* file_;
+ std::unique_ptr file_;
GameClientImpl* d;
bool internet_;
};
=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc 2019-02-23 11:00:49 +
+++ src/network/gamehost.cc 2019-04-28 14:30:33 +
@@ -524,7 +524,7 @@
hostuser.position = UserSettings::none();
hostuser.ready = true;
d->settings.users.push_back(hostuser);
- file_ = nullptr; // Initialize as 0 pointer - unfortunately needed in struct.
+ file_.reset(nullptr); // Initialize as 0 pointer - unfortunately needed in struct.
}
GameHost::~GameHost() {
@@ -539,7 +539,6 @@
d->net.reset();
d->promoter.reset();
delete d;
- delete file_;
}
const std::string& GameHost::get_local_playername() const {
@@ -1076,9 +1075,7 @@
// Read in the file
FileRead fr;
fr.open(*g_fs, mapfilename);
- if (file_)
- delete file_;
- file_ = new NetTransferFile();
+ file_.reset(new NetTransferFile());
file_->filename = mapfilename;
uint32_t leftparts = file_->bytes = fr.get_size();
while (leftparts > 0) {
@@ -1097,10 +1094,7 @@
file_->md5sum = md5sum.get_checksum().str();
} else {
// reset previously offered map / saved game
- if (file_) {
- delete file_;
- file_ = nullptr;
- }
+ file_.reset(nullptr);
}
packet.reset();
=== modified file 'src/network/gamehost.h'
--- src/network/gamehost.h 2019-02-23 11:00:49 +
+++ src/network/gamehost.h 2019-04-28 14:30:33 +
@@ -20,6 +20,8 @@
#ifndef WL_NETWORK_GAMEHOST_H
#define WL_NETWORK_GAMEHOST_H
+#include
+
#include "logic/game_controller.h"
#include "logic/game_settings.h"
#include "logic/player_end_result.h"
@@ -156,7 +158,7 @@
const std::string& arg = "");
void reaper();
- NetTransferFile* file_;
+ std::unique_ptr file_;
GameHostImpl* d;
bool internet_;
bool forced_pause_;
=== modified file 'src/network/network.h'
--- src/network/network.h 2019-02-23 11:00:49 +
+++ src/network/network.h 2019-04-28 14:30:33 +
@@ -181,6 +181,11 @@
};
struct NetTransferFile {
+ NetTransferFile() : bytes(0), filename(""), md5sum("") {
+ }
+ ~NetTransferFile() {
+ parts.clear();
+ }
uint32_t bytes;
std::string filename;
std::string md5sum;
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bugfix-buildings-tooltips into lp:widelands
Ok adrresed code review. However .c_str() did not compile so I stood with .str() which works fine. Have tested this and added a translators hint. -- https://code.launchpad.net/~widelands-dev/widelands/bugfix-buildings-tooltips/+merge/366607 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bugfix-buildings-tooltips into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bugfix-buildings-tooltips into lp:widelands
Review: Approve I overlooked that you were using boost::format, so c_str() is not necessary. So, code LGTM now :) -- https://code.launchpad.net/~widelands-dev/widelands/bugfix-buildings-tooltips/+merge/366607 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bugfix-buildings-tooltips. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/per-level-soldier-anims into lp:widelands
That's because it still does need a unique key to identify the map contents. The pointer address is unique, so it can deal with that. We will need to do some thorough testing now. -- https://code.launchpad.net/~widelands-dev/widelands/per-level-soldier-anims/+merge/354929 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/per-level-soldier-anims into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20
Admitted, you fix is more elegant.
One nit inline.
Wil still compile and test this
Diff comments:
>
> === modified file 'src/network/network.h'
> --- src/network/network.h 2019-02-23 11:00:49 +
> +++ src/network/network.h 2019-04-28 14:30:33 +
> @@ -181,6 +181,11 @@
> };
>
> struct NetTransferFile {
> + NetTransferFile() : bytes(0), filename(""), md5sum("") {
> + }
> + ~NetTransferFile() {
> + parts.clear();
That extra clear(() does not make any difference
> + }
> uint32_t bytes;
> std::string filename;
> std::string md5sum;
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20
Review: Needs Fixing compile test Your soultion prevents the crahs but makes a difference: - The new Map is not announced at the GUI + The new Map appears in the logs - No Transfer of any Map happens any longer. from the code I see no diecrt difference. eventually ther is some == comparison for file_ which we must consider? lets check that code, what is done with that file_? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20
I found one: file_ = nullptr; and !file_ // this should be save. in gameclient.cc I used a Windows Server wit build20_rc1 as host so I could not test gamehost.cc this way. Now I am out of Ideas... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/overlapping_workareas into lp:widelands
Benedikt Straub has proposed merging
lp:~widelands-dev/widelands/overlapping_workareas into lp:widelands.
Commit message:
Indicate overlapping workareas when placing buildings
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/overlapping_workareas/+merge/366623
When placing a productionsite, only own conflicting productionsites and
constructionsites for productionsites will be indicated.
When placing a militarysite or port, own and enemy conflicting milsites and
ports and constructionsites for milsites and ports will be indicated.
Some restructuring and efficiency tweaks to workarea drawing code to improve
performance.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/overlapping_workareas into lp:widelands.
=== modified file 'src/graphic/color.cc'
--- src/graphic/color.cc 2019-02-23 11:00:49 +
+++ src/graphic/color.cc 2019-04-28 16:58:57 +
@@ -59,6 +59,13 @@
a = 255;
}
+RGBAColor::RGBAColor(uint32_t hex)
+ : r((hex & 0xff) >> 16),
+ g((hex & 0xff00) >> 8),
+ b((hex & 0xff)),
+ a((hex & 0xff00) >> 24) {
+}
+
std::string RGBAColor::hex_value() const {
return (boost::format("%02x%02x%02x%02x>") % int(r) % int(g) % int(b) % int(a)).str();
}
=== modified file 'src/graphic/color.h'
--- src/graphic/color.h 2019-02-23 11:00:49 +
+++ src/graphic/color.h 2019-04-28 16:58:57 +
@@ -49,6 +49,7 @@
struct RGBAColor {
RGBAColor(uint8_t R, uint8_t G, uint8_t B, uint8_t A);
+ RGBAColor(uint32_t);
RGBAColor(const RGBAColor& other) = default;
// Initializes the color to black.
=== modified file 'src/graphic/gl/workarea_program.cc'
--- src/graphic/gl/workarea_program.cc 2019-04-25 21:48:17 +
+++ src/graphic/gl/workarea_program.cc 2019-04-28 16:58:57 +
@@ -65,12 +65,21 @@
RGBAColor(0, 0, 127, kWorkareaTransparency),// Inner circle
};
static inline RGBAColor apply_color(RGBAColor c1, RGBAColor c2) {
+ if (c1.a == 0 && c2.a == 0) {
+ return RGBAColor((c1.r + c2.r) / 2, (c1.g + c2.g) / 2, (c1.b + c2.b) / 2, 0);
+ }
uint8_t r = (c1.r * c1.a + c2.r * c2.a) / (c1.a + c2.a);
uint8_t g = (c1.g * c1.a + c2.g * c2.a) / (c1.a + c2.a);
uint8_t b = (c1.b * c1.a + c2.b * c2.a) / (c1.a + c2.a);
uint8_t a = (c1.a + c2.a) / 2;
return RGBAColor(r, g, b, a);
}
+static inline RGBAColor apply_color_special(RGBAColor base, RGBAColor special) {
+ uint8_t r = (base.r * base.a + special.r * 255) / (base.a + 255);
+ uint8_t g = (base.g * base.a + special.g * 255) / (base.a + 255);
+ uint8_t b = (base.b * base.a + special.b * 255) / (base.a + 255);
+ return RGBAColor(r, g, b, special.a);
+}
void WorkareaProgram::add_vertex(const FieldsToDraw::Field& field, RGBAColor overlay) {
vertices_.emplace_back();
@@ -104,31 +113,43 @@
// Down triangle.
if (field.bln_index != FieldsToDraw::kInvalidIndex) {
RGBAColor color(0, 0, 0, 0);
- for (const std::map, uint8_t>& wa_map : workarea) {
-const auto it =
- wa_map.find(Widelands::TCoords<>(field.fcoords, Widelands::TriangleIndex::D));
-if (it != wa_map.end()) {
- color = apply_color(color, workarea_colors[it->second]);
+ for (const WorkareasEntry& wa_map : workarea) {
+for (const WorkareaPreviewData& data : wa_map) {
+ if (data.coords == Widelands::TCoords<>(field.fcoords, Widelands::TriangleIndex::D)) {
+ RGBAColor color_to_apply = workarea_colors[data.index];
+ if (data.use_special_coloring) {
+ color_to_apply = apply_color_special(color_to_apply, RGBAColor(data.special_coloring));
+ }
+ color = apply_color(color, color_to_apply);
+ }
}
}
- add_vertex(fields_to_draw.at(current_index), color);
- add_vertex(fields_to_draw.at(field.bln_index), color);
- add_vertex(fields_to_draw.at(field.brn_index), color);
+ if (color.a > 0) {
+add_vertex(fields_to_draw.at(current_index), color);
+add_vertex(fields_to_draw.at(field.bln_index), color);
+add_vertex(fields_to_draw.at(field.brn_index), color);
+ }
}
// Right triangle.
if (field.rn_index != FieldsToDraw::kInvalidIndex) {
RGBAColor color(0, 0, 0, 0);
- for (const std::map, uint8_t>& wa_map : workarea) {
-const auto it =
- wa_map.find(Widelands::TCoords<>(field.fcoords, Widelands::TriangleIndex::R));
-if (it != wa_map.end()) {
- color = apply_color(color, workarea_colors[it->second]);
+ for (const WorkareasEntry& wa_map : workarea) {
+for (const WorkareaPreviewData& data : wa_map) {
+ if (data.coords == Widelands::TCoords<>(field.fcoords, Widelands::TriangleIndex::R)) {
+ RGBAColor color_to_apply = workarea_colors[data.index];
+ if (data.use_special_coloring) {
+ color_to_apply = apply_color_special(color_to_apply, RGBAColor(data.special_coloring));
+ }
+ color = apply_color(color, color_to_apply);
+ }
}
}
- add_vertex(fields_to_draw.at(current_index
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20
When I tested I still had a file from the previous attempt with the crashy version. I had to delete that for the transfer to be initiated. Maybe that's it? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20
Yep, found and deleted it, will try again tomorrow. Playing against WorldSavior and the-X makes me tired. Thats another Bug for the Review in R21 then ... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/list-directories-in-cpp into lp:widelands
Continuous integration builds have changed state: Travis build 4826. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/525572046. Appveyor build 4607. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_list_directories_in_cpp-4607. -- https://code.launchpad.net/~widelands-dev/widelands/list-directories-in-cpp/+merge/366614 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/list-directories-in-cpp into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/per-level-soldier-anims into lp:widelands
Continuous integration builds have changed state: Travis build 4827. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/525599704. Appveyor build 4608. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_per_level_soldier_anims-4608. -- https://code.launchpad.net/~widelands-dev/widelands/per-level-soldier-anims/+merge/354929 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/per-level-soldier-anims into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bugfix-buildings-tooltips into lp:widelands
Continuous integration builds have changed state: Travis build 4829. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/525646461. Appveyor build 4610. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bugfix_buildings_tooltips-4610. -- https://code.launchpad.net/~widelands-dev/widelands/bugfix-buildings-tooltips/+merge/366607 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bugfix-buildings-tooltips. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/overlapping_workareas into lp:widelands
Continuous integration builds have changed state: Travis build 4830. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/525664696. Appveyor build 4611. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_overlapping_workareas-4611. -- https://code.launchpad.net/~widelands-dev/widelands/overlapping_workareas/+merge/366623 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/overlapping_workareas into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/overlapping_workareas into lp:widelands
I think we have some room here to get rid of code duplication.
Diff comments:
>
> === modified file 'src/graphic/gl/workarea_program.cc'
> --- src/graphic/gl/workarea_program.cc2019-04-25 21:48:17 +
> +++ src/graphic/gl/workarea_program.cc2019-04-28 16:58:57 +
> @@ -104,31 +113,43 @@
> // Down triangle.
> if (field.bln_index != FieldsToDraw::kInvalidIndex) {
> RGBAColor color(0, 0, 0, 0);
> - for (const std::map, uint8_t>&
> wa_map : workarea) {
> - const auto it =
> -
> wa_map.find(Widelands::TCoords<>(field.fcoords, Widelands::TriangleIndex::D));
> - if (it != wa_map.end()) {
> - color = apply_color(color,
> workarea_colors[it->second]);
> + for (const WorkareasEntry& wa_map : workarea) {
> + for (const WorkareaPreviewData& data : wa_map) {
> + if (data.coords ==
> Widelands::TCoords<>(field.fcoords, Widelands::TriangleIndex::D)) {
> + RGBAColor color_to_apply =
> workarea_colors[data.index];
> + if (data.use_special_coloring) {
> + color_to_apply =
> apply_color_special(color_to_apply, RGBAColor(data.special_coloring));
> + }
> + color = apply_color(color,
> color_to_apply);
> + }
> }
> }
> - add_vertex(fields_to_draw.at(current_index), color);
> - add_vertex(fields_to_draw.at(field.bln_index), color);
> - add_vertex(fields_to_draw.at(field.brn_index), color);
> + if (color.a > 0) {
> + add_vertex(fields_to_draw.at(current_index),
> color);
> + add_vertex(fields_to_draw.at(field.bln_index),
> color);
> + add_vertex(fields_to_draw.at(field.brn_index),
> color);
> + }
> }
>
> // Right triangle.
> if (field.rn_index != FieldsToDraw::kInvalidIndex) {
> RGBAColor color(0, 0, 0, 0);
> - for (const std::map, uint8_t>&
> wa_map : workarea) {
> - const auto it =
> -
> wa_map.find(Widelands::TCoords<>(field.fcoords, Widelands::TriangleIndex::R));
> - if (it != wa_map.end()) {
> - color = apply_color(color,
> workarea_colors[it->second]);
> + for (const WorkareasEntry& wa_map : workarea) {
You could pull out a common function here that takes the TriangleIndex as
parameters. It could be a lambda function, because it's not being used anywhere
else, like this:
auto do_something = [this](const Field& field, Widelands::TriangleIndex
triangle_index) { // do something];
> + for (const WorkareaPreviewData& data : wa_map) {
> + if (data.coords ==
> Widelands::TCoords<>(field.fcoords, Widelands::TriangleIndex::R)) {
> + RGBAColor color_to_apply =
> workarea_colors[data.index];
> + if (data.use_special_coloring) {
> + color_to_apply =
> apply_color_special(color_to_apply, RGBAColor(data.special_coloring));
> + }
> + color = apply_color(color,
> color_to_apply);
> + }
> }
> }
> - add_vertex(fields_to_draw.at(current_index), color);
> - add_vertex(fields_to_draw.at(field.brn_index), color);
> - add_vertex(fields_to_draw.at(field.rn_index), color);
> + if (color.a > 0) {
> + add_vertex(fields_to_draw.at(current_index),
> color);
> + add_vertex(fields_to_draw.at(field.brn_index),
> color);
> + add_vertex(fields_to_draw.at(field.rn_index),
> color);
> + }
> }
> }
>
>
> === modified file 'src/logic/map_objects/tribes/tribes.cc'
> --- src/logic/map_objects/tribes/tribes.cc2019-02-23 11:00:49 +
> +++ src/logic/map_objects/tribes/tribes.cc2019-04-28 16:58:57 +
> @@ -425,4 +425,13 @@
> }
> }
>
> +uint32_t Tribes::get_largest_workarea() const {
I think that we should calculate it only once during
