Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/chat-messages-overlap into lp:widelands
Review: Approve LGTM. -- https://code.launchpad.net/~widelands-dev/widelands/chat-messages-overlap/+merge/290834 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/chat-messages-overlap. ___ 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/chat-messages-overlap into lp:widelands
Review: Resubmit I decided to refactor this into a function. What do you think? -- https://code.launchpad.net/~widelands-dev/widelands/chat-messages-overlap/+merge/290834 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/chat-messages-overlap. ___ 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-1395322-tool3 into lp:widelands
Continuous integration builds have changed state: Travis build 951. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/120451312. Appveyor build 784. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1395322_tool3-784. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1395322-tool3/+merge/290829 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1395322-tool3 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/chat-messages-overlap into lp:widelands
Fine with me. Could you also fix the inconsistent indentation of that lambda? -- https://code.launchpad.net/~widelands-dev/widelands/chat-messages-overlap/+merge/290834 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/chat-messages-overlap. ___ 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/chat-messages-overlap into lp:widelands
The constructor would probably need prettifying as well. -- https://code.launchpad.net/~widelands-dev/widelands/chat-messages-overlap/+merge/290834 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/chat-messages-overlap. ___ 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/travis-clang-warnings into lp:widelands
Hello Gun: changes look good to me, Compiles on OSX without any new Issues. Will try to play this on trunk today. (Thanks for playing the Triangle :-) -- https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/travis-clang-warnings. ___ 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/travis-clang-warnings into lp:widelands
I am waiting on a comment from SirVer regarding 4. before merging ;) -- https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/travis-clang-warnings. ___ 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/chat-messages-overlap into lp:widelands
What would you suggest regarding the prettyfying? Indents are a problem with my IDE - we will run clang-format over the codebase before branching Build 19 to fix these issues. -- https://code.launchpad.net/~widelands-dev/widelands/chat-messages-overlap/+merge/290834 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/chat-messages-overlap. ___ 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/chat-messages-overlap into lp:widelands
I don't have any experience with automatic formatting tools besides auto-indent in my IDE. I was suggesting doing it manually for these few lines (the rest is OK), but it can be done later. I think you can go ahead and merge it as it is then. -- https://code.launchpad.net/~widelands-dev/widelands/chat-messages-overlap/+merge/290834 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/chat-messages-overlap. ___ 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/chat-messages-overlap into lp:widelands
OK, thanks! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/chat-messages-overlap/+merge/290834 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/chat-messages-overlap. ___ 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/chat-messages-overlap into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/chat-messages-overlap into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/chat-messages-overlap/+merge/290834 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/chat-messages-overlap. ___ 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-1538549-dismantle-enemy-buildings into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1538549-dismantle-enemy-buildings into
lp:widelands.
Commit message:
For the dismantle function, buildings now check which wares the tribe can use
and adjust the dismantle button / returned wares accordingly.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1538549 in widelands: "Dismantle enemy buildings"
https://bugs.launchpad.net/widelands/+bug/1538549
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1538549-dismantle-enemy-buildings/+merge/290895
See commit message.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1538549-dismantle-enemy-buildings into
lp:widelands.
=== modified file 'src/logic/map_objects/tribes/dismantlesite.cc'
--- src/logic/map_objects/tribes/dismantlesite.cc 2016-03-19 09:58:41 +
+++ src/logic/map_objects/tribes/dismantlesite.cc 2016-04-04 16:27:33 +
@@ -107,45 +107,37 @@
{
PartiallyFinishedBuilding::init(egbase);
- std::map wares;
- count_returned_wares(this, wares);
-
- std::map::const_iterator it = wares.begin();
- wares_.resize(wares.size());
-
- for (size_t i = 0; i < wares.size(); ++i, ++it) {
- WaresQueue & wq =
- *(wares_[i] = new WaresQueue(*this, it->first, it->second));
-
- wq.set_filled(it->second);
- work_steps_ += it->second;
+ for (const auto& ware: count_returned_wares(this)) {
+ WaresQueue* wq = new WaresQueue(*this, ware.first, ware.second);
+ wq->set_filled(ware.second);
+ wares_.push_back(wq);
+ work_steps_ += ware.second;
}
}
/*
===
-Count wich wares you get back if you dismantle the given building
+Count which wares you get back if you dismantle the given building
===
*/
-void DismantleSite::count_returned_wares
- (Building* building,
- std::map & res)
+const Buildcost DismantleSite::count_returned_wares(Building* building)
{
+ Buildcost result;
for (DescriptionIndex former_idx : building->get_former_buildings()) {
- const std::map * return_wares;
const BuildingDescr* former_descr = building->owner().tribe().get_building_descr(former_idx);
- if (former_idx != building->get_former_buildings().front()) {
- return_wares = & former_descr->returned_wares_enhanced();
- } else {
- return_wares = & former_descr->returned_wares();
- }
- assert(return_wares != nullptr);
+ const Buildcost& return_wares =
+former_idx != building->get_former_buildings().front() ?
+ former_descr->returned_wares_enhanced() :
+ former_descr->returned_wares();
- std::map::const_iterator i;
- for (i = return_wares->begin(); i != return_wares->end(); ++i) {
- res[i->first] += i->second;
+ for (const auto& ware : return_wares) {
+ // TODO(GunChleoc): Once we have trading, we might want to return all wares again.
+ if (building->owner().tribe().has_ware(ware.first)) {
+result[ware.first] += ware.second;
+ }
}
}
+ return result;
}
=== modified file 'src/logic/map_objects/tribes/dismantlesite.h'
--- src/logic/map_objects/tribes/dismantlesite.h 2016-01-31 15:31:00 +
+++ src/logic/map_objects/tribes/dismantlesite.h 2016-04-04 16:27:33 +
@@ -72,7 +72,7 @@
bool get_building_work(Game &, Worker &, bool success) override;
- static void count_returned_wares(Building* building, std::map & res);
+ static const Buildcost count_returned_wares(Building* building);
protected:
void update_statistics_string(std::string*) override;
=== modified file 'src/wui/buildingwindow.cc'
--- src/wui/buildingwindow.cc 2016-03-29 10:04:48 +
+++ src/wui/buildingwindow.cc 2016-04-04 16:27:33 +
@@ -272,21 +272,19 @@
}
if (capscache_ & Widelands::Building::PCap_Dismantle) {
- std::map wares;
- Widelands::DismantleSite::count_returned_wares(&building_, wares);
- UI::Button * dismantlebtn =
-new UI::Button
- (capsbuttons, "dismantle", 0, 0, 34, 34,
- g_gr->images().get("images/ui_basic/but4.png"),
- g_gr->images().get(pic_dismantle),
- std::string(_("Dismantle")) + "" + _("Returns:") + "" +
- waremap_to_richtext(owner.tribe(), wares));
- dismantlebtn->sigclicked.connect(boost::bind(&BuildingWindow::act_dismantle, boost::ref(*this)));
- capsbuttons->add
-(dismantlebtn,
- UI::Align::kHCenter);
-
- requires_destruction_separator = true;
+ const Widelands::Buildcost wares = Widelands::DismantleSite::count_returned_wares(&building_);
+ if (!wares.empty()) {
+UI::Button * dismantlebtn =
+ new UI::Button
+ (capsbuttons, "dismantle", 0, 0, 34, 34,
+ g_gr->images().get("images/ui_basic/but4.png"),
+ g_gr->images().get(pic_dismantle),
+ std::string(_("Dismantle")) + "" + _("Returns:") + "" +
+ waremap_to_richtext(owner.tribe(), wares));
+dismantlebtn->sigclicked.connect(boost::bind(&BuildingWindow::act_dismantle, boost::ref(*this)));
+capsbuttons->add(dismantlebtn, UI::Align::kHCenter);
+r
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/travis-clang-warnings into lp:widelands
Removing both 'std::move's is fine if all GCCs eat it. In fact the compiler is right and an explicit move might make a copy elision there illegal. However, I doubt it matters for performance much - if at all. -- https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/travis-clang-warnings. ___ 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-1538549-dismantle-enemy-buildings into lp:widelands
Review: Approve code review + testing LGTM. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1538549-dismantle-enemy-buildings/+merge/290895 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1538549-dismantle-enemy-buildings. ___ 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-1538549-dismantle-enemy-buildings into lp:widelands
Just lately I modified AI to sent away wares before upgrading, but I believe no conflict here... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1538549-dismantle-enemy-buildings/+merge/290895 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1538549-dismantle-enemy-buildings. ___ 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/travis-clang-warnings into lp:widelands
I got a crash in 0 widelands 0x00010d0cfa40 FullscreenMenuInternetLobby::fill_client_list(std::__1::vector > const*) + 2336 (vector:641) 1 widelands 0x00010d0cf0e2 FullscreenMenuInternetLobby::think() + 146 (internet_lobby.cc:182) but this is the same on trunk. @SirVer I will remove the std:move now and then merge with trunk again. -- https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/travis-clang-warnings. ___ 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/travis-clang-warnings into lp:widelands
Uhm, the fix for bug-1562332 was somehow reverted, I will add it in this branch again -- https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/travis-clang-warnings. ___ 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-1538549-dismantle-enemy-buildings into lp:widelands
Continuous integration builds have changed state: Travis build 961. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/120671195. Appveyor build 794. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1538549_dismantle_enemy_buildings-794. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1538549-dismantle-enemy-buildings/+merge/290895 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1538549-dismantle-enemy-buildings. ___ 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
