Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle into lp:widelands
Review: Approve test Agreed. now it looks consistent with the rest of the window. Thanks for fixing. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
Indentations will be fixed by bunnybot. I have set my IDE back to tabs from spaces, so it will be a bit less crazy in the future. The dismantle tooltip now only says "Dismantle" when there are no wares returned. I have tried out some alternatives and I think that looks best. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
Ok I will test this tonight and merge if ok.
in the diff here on Launchpad there are some differing indentations? are they
intentional?
Anyhow I have marked them in the diff comments
Diff comments:
>
> === modified file 'src/logic/map_objects/tribes/building.cc'
> --- src/logic/map_objects/tribes/building.cc 2018-11-19 21:38:03 +
> +++ src/logic/map_objects/tribes/building.cc 2018-11-23 08:20:28 +
> @@ -59,7 +59,9 @@
> const EditorGameBase& egbase)
> : MapObjectDescr(init_type, table.get_string("name"), init_descname,
> table),
> egbase_(egbase),
> - buildable_(false),
> + buildable_(table.has_key("buildcost")),
> + can_be_dismantled_(table.has_key("return_on_dismantle") ||
> table.has_key("return_on_dismantle_on_enhanced")),
> + destructible_(table.has_key("destructible") ?
> table.get_bool("destructible") : true),
different indentation. Intentional?
> size_(BaseImmovable::SMALL),
> mine_(false),
> port_(false),
>
> === modified file 'src/logic/map_objects/tribes/building.h'
> --- src/logic/map_objects/tribes/building.h 2018-09-25 06:32:35 +
> +++ src/logic/map_objects/tribes/building.h 2018-11-23 08:20:28 +
> @@ -73,6 +73,9 @@
> bool is_buildable() const {
> return buildable_;
> }
> +bool can_be_dismantled() const {
is this indentation intentional
> + return can_be_dismantled_;
> + }
> bool is_destructible() const {
> return destructible_;
> }
> @@ -172,8 +175,9 @@
> const EditorGameBase& egbase_;
>
> private:
> - bool buildable_; // the player can build this himself
> - bool destructible_; // the player can destruct this himself
> + const bool buildable_; // the player can build this himself
> +const bool can_be_dismantled_; // the player can dismantle this building
same here
> + const bool destructible_; // the player can destruct this himself
> Buildcost buildcost_;
> Buildcost return_dismantle_; // Returned wares on dismantle
> Buildcost enhance_cost_; // cost for enhancing
--
https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle.
___
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/empire04_unused_key_return_on_dismantle into lp:widelands
Review: Resubmit We already had to add some strings for other bug fixes. I think breaking the string freeze for smaller strings is OK, as long as we don't overdo it. I have addressed all the points in the code review now. We can't check for duplicate keys in Lua tables, because the backend (Eris) or maybe Lua itself already squashes them. So, I have removed all those checks. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
Review: Approve Perhaps it might be worth having it without the string . (showing nothing beneath the Headings) -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
Tested the branch. Works like it should do. no difference found in empire 04. ALthough the message for no wares returned looks unfamiliar and is to long for my taste. For me we should have the same headings as for wares returned. and a short message like "no wares" instead of the waremap to richtext. resulting in: Dismantle Returns: "no wares" anyhow as this is a matter of taste I will approve this. Question is do we want to have it for b20? Problem is that it will have at least one new string to translate. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
thanks Arty for your comments and solutions. I would be happy to test this
again after the fixes.
Nice spot with the resulting farm1 issue after this finally works.
Diff comments:
>
> === modified file 'data/campaigns/emp04.wmf/scripting/tribes/farm1.lua'
> --- data/campaigns/emp04.wmf/scripting/tribes/farm1.lua 2018-09-09
> 17:58:55 +
> +++ data/campaigns/emp04.wmf/scripting/tribes/farm1.lua 2018-11-21
> 09:48:10 +
> @@ -10,10 +10,6 @@
> enhancement = "empire_farm2",
>
> return_on_dismantle = {
> - planks = 0,
> - granite = 0,
> - marble = 0,
> - marble_column = 0
> },
correct. Thanks Arty. The table has to be removed completely, as the farm
shouldn't be dismantable at all.
>
> animations = {
--
https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle.
___
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/empire04_unused_key_return_on_dismantle into lp:widelands
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 appear
for such empty tables. This is easily fixed in ‘wui/buildingwindow.cc’ by
removing a ware check, i.e., removing lines 287 and 297 (and unindenting the
block inbetween). I already tested this, it works fine.
2) Having no items shown in the box when hovering over the dismantle button
looks good enough for me, but I can imagine that some might find this not nice.
Maybe adding a little text “nothing” or so might be good. And (at a later
point) maybe even a little icon.
3) See diff comments.
Diff comments:
>
> === modified file 'data/campaigns/emp04.wmf/scripting/tribes/farm1.lua'
> --- data/campaigns/emp04.wmf/scripting/tribes/farm1.lua 2018-09-09
> 17:58:55 +
> +++ data/campaigns/emp04.wmf/scripting/tribes/farm1.lua 2018-11-21
> 09:48:10 +
> @@ -10,10 +10,6 @@
> enhancement = "empire_farm2",
>
> return_on_dismantle = {
> - planks = 0,
> - granite = 0,
> - marble = 0,
> - marble_column = 0
> },
For farm1, return_on_dismantle should now be gone completely for the campaign
to work properly. (The farm click check relies on having no dismantle button,
and the text also says that the farms can't be dismantled.)
>
> animations = {
>
> === modified file 'src/logic/map_objects/buildcost.cc'
> --- src/logic/map_objects/buildcost.cc2018-04-07 16:59:00 +
> +++ src/logic/map_objects/buildcost.cc2018-11-21 09:48:10 +
> @@ -37,32 +35,37 @@
> Buildcost::Buildcost(std::unique_ptr table, const Tribes& tribes)
> : std::map() {
> for (const std::string& warename : table->keys()) {
> - int32_t value = INVALID_INDEX;
> - try {
> - DescriptionIndex const idx =
> tribes.safe_ware_index(warename);
> - if (count(idx)) {
> - throw GameDataError(
> -"A buildcost item of this ware type has
> already been defined: %s", warename.c_str());
> - }
> - value = table->get_int(warename);
> - const uint8_t ware_count = value;
> - if (ware_count != value) {
> - throw GameDataError("Ware count is out of range
> 1 .. 255");
> - }
> - insert(std::pair(idx,
> ware_count));
> - } catch (const WException& e) {
> - throw GameDataError("[buildcost] \"%s=%d\": %s",
> warename.c_str(), value, e.what());
> - }
> + // Read ware index
> + if (!tribes.ware_exists(warename)) {
> + throw GameDataError("Buildcost: Unknown ware: %s",
> warename.c_str());
> + }
> + DescriptionIndex const idx = tribes.safe_ware_index(warename);
> + if (count(idx) != 0) {
This condition is actually never true, because table->keys() returns a
std::set, so even if a key appears multiple times in a lua table, it only
occurs once in the set, so this loop also touches each key only once.
This is unrelated to this branch though, but we should probably fix this at
some point. Maybe this should be fixed more generally anyway. (Not even sure if
multiple occurrences of the same key make sense somewhere.) I guess I’ll make a
proper bug report after thinking about it a bit more. But if you know a simple
way to check this properly here, you can just add a fix here.
> + throw GameDataError(
> +"Buildcost: Ware '%s' is listed twice",
> warename.c_str());
> + }
> +
> + // Read value
> + int32_t value = table->get_int(warename);
> + if (value < 1) {
> + throw GameDataError("Buildcost: Ware count needs to be
> > 0 in \"%s=%d\".\nEmpty buildcost tables are allowed if you wish to have an
> amount of 0.", warename.c_str(), value);
> + } else if (value > 255) {
> + throw GameDataError("Buildcost: Ware count needs to be
> <= 255 0 in \"%s=%d\".", warename.c_str(), value);
1) The 0 after 255 should go away.
2) This throw actually crashes the whole application. The problem is the “<” in
the text which - I assume - causes a crash in the rich text handling (attempt
to interpret it as opening a tag) when trying to show the message box with the
error. (Didn’t check this thoroughly yet, but simply deleting the “<” from the
message avoids the crash. I’ll check this properly and make a bug report.)
For now, just replace the “<=” by “at most” or so.
> + }
> +
> + // Add
> + insert(std::pair(idx, value));
> }
>
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle into lp:widelands
Have tested it. Unfortunately it doesn't work. an empty table is not triggering the dismantle button. I just remembered myself why I had the forester dismantable with no wares. It was the only way to distinguish its building window from the building window of the farms which we need to catch for one objective. furthermore some indents seem to be wrong in the diff. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
Review: Resubmit Arty has convinced me, so I have changed the design. Needs retesting. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
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 though it makes sense - doesn't really add anything. The game would work fine without it. And regular buildings (and most special builidings in campaigns) have proper return on dismantle amounts anyway, so I wouldn't expect "false" bug reports about this. 2) Not having the condition allows for a little more flavour in missions. Like in emp04. Or maybe there'll be a mission where destroying would come with a fire hazard to neighboring buildings, possibly destroying them, too. (Not sure this is possibly atm, i.e., whether we can detect the difference between dismantling and destroying via mission scripts. Just a general idea.) 3) If the building is not far from a warehouse with a builder available, dismantling without return is even a little faster than destroying. Difference is minor though. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
I am ok with that. Unfortunately I overlooked some code issues in the toher branch which Arty found. we should make sure we will fix them with this branch at the latest. Perhaps it will be even recommendable to do it now in a fixing round -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
If dismantling doesn't return any wares, the player will choose to destroy the building anyway, because all that dismantling will do for the player is to take longer. So, I think we should change the text for build 21. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
Review: Needs Fixing Have tested this branch now and it is not working as expected (and stated in the dialogs) for the "special" forester which has been designed to be dismantable but delivering no wares. So we could either keep the text and ensure this in the logic or have this logic and change the text both we should do for b21. So I'll disapprove this with needs fixing. and approve the other branch to get rid of the error log message. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
Continuous integration builds have changed state: Travis build 4251. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/456892857. Appveyor build 4045. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_empire04_unused_key_return_on_dismantle-4045. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
could you please merge trunk so we will get Appveyor builds to test this. I would start with the complete thing (this branch) for testing and try the other only in case this is not delivering the correct result. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
Continuous integration builds have changed state: Travis build 4198. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/452676639. Appveyor build 3994. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_empire04_unused_key_return_on_dismantle-3994. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
I'll create another branch then that only loads the backend bits (r8907), without the UI - we can expect bug reports for the log message, so I'll want that clean for Build 20. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
For me this is a new feature and should therefore be postponed to B21, which would give me some time to test which i haven't currently and to properly rework the scenario to the new feature -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
Review: Approve diff, testing >From what I can say without knowing the scenario it works as intended, with >some building gaining and some building loosing the possibility/button to >dismantle them. Code is looking good to me as well. Unrelated, but noticed this when testing: The empire "outpost enhanced to barrier" gives more materials when dismantling than a directly constructed barrier. I think this is on purpose, though. -- https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle. ___ 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/empire04_unused_key_return_on_dismantle into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle into
lp:widelands.
Commit message:
Make dismantle button independent of buildable/enhanced. This fixes missing
Dismantle buttons in Empire scenario 4.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/empire04_unused_key_return_on_dismantle/+merge/358273
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/empire04_unused_key_return_on_dismantle into
lp:widelands.
=== modified file 'src/logic/map_objects/tribes/building.cc'
--- src/logic/map_objects/tribes/building.cc 2018-09-23 11:10:56 +
+++ src/logic/map_objects/tribes/building.cc 2018-11-03 16:33:17 +
@@ -60,6 +60,7 @@
: MapObjectDescr(init_type, table.get_string("name"), init_descname, table),
egbase_(egbase),
buildable_(false),
+ can_be_dismantled_(false),
size_(BaseImmovable::SMALL),
mine_(false),
port_(false),
@@ -133,17 +134,22 @@
}
}
+// We define a building as buildable if it has a "buildcost" table.
+// A buildable building must also define "return_on_dismantle".
+// However, we support "return_on_dismantle" without "buildable", because this is used by custom scenario buildings.
+if (table.has_key("return_on_dismantle")) {
+return_dismantle_ = Buildcost(table.get_table("return_on_dismantle"), egbase_.tribes());
+}
if (table.has_key("buildcost")) {
buildable_ = true;
- try {
- buildcost_ = Buildcost(table.get_table("buildcost"), egbase_.tribes());
- return_dismantle_ = Buildcost(table.get_table("return_on_dismantle"), egbase_.tribes());
- } catch (const WException& e) {
- throw wexception(
- "A buildable building must define \"buildcost\" and \"return_on_dismantle\": %s",
- e.what());
- }
+if (!table.has_key("return_on_dismantle")) {
+throw wexception(
+ "The building '%s' has a \"buildcost\" but no \"return_on_dismantle\"",
+ name().c_str());
+}
+ buildcost_ = Buildcost(table.get_table("buildcost"), egbase_.tribes());
}
+
if (table.has_key("enhancement_cost")) {
enhanced_building_ = true;
try {
@@ -151,11 +157,12 @@
return_enhanced_ =
Buildcost(table.get_table("return_on_dismantle_on_enhanced"), egbase_.tribes());
} catch (const WException& e) {
- throw wexception("An enhanced building must define \"enhancement_cost\""
+ throw wexception("The enhanced building '%s' must define \"enhancement_cost\""
"and \"return_on_dismantle_on_enhanced\": %s",
- e.what());
+ name().c_str(), e.what());
}
}
+can_be_dismantled_ = (return_dismantle_.total() > 0 || return_enhanced_.total() > 0);
needs_seafaring_ = table.has_key("needs_seafaring") ? table.get_bool("needs_seafaring") : false;
@@ -310,7 +317,7 @@
const BuildingDescr& tmp_descr = descr();
if (tmp_descr.is_destructible()) {
caps |= PCap_Bulldoze;
- if (tmp_descr.is_buildable() || tmp_descr.is_enhanced())
+ if (tmp_descr.can_be_dismantled())
caps |= PCap_Dismantle;
}
if (tmp_descr.enhancement() != INVALID_INDEX)
=== modified file 'src/logic/map_objects/tribes/building.h'
--- src/logic/map_objects/tribes/building.h 2018-09-25 06:32:35 +
+++ src/logic/map_objects/tribes/building.h 2018-11-03 16:33:17 +
@@ -73,6 +73,9 @@
bool is_buildable() const {
return buildable_;
}
+bool can_be_dismantled() const {
+ return can_be_dismantled_;
+ }
bool is_destructible() const {
return destructible_;
}
@@ -173,6 +176,7 @@
private:
bool buildable_; // the player can build this himself
+bool can_be_dismantled_; // the player can dismantle this building
bool destructible_; // the player can destruct this himself
Buildcost buildcost_;
Buildcost return_dismantle_; // Returned wares on dismantle
___
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
