Review: Needs Fixing
Diff comments:
> === modified file 'src/logic/map_objects/immovable.cc'
> --- src/logic/map_objects/immovable.cc 2016-11-03 07:20:57 +0000
> +++ src/logic/map_objects/immovable.cc 2016-11-03 07:36:43 +0000
> @@ -339,6 +339,7 @@
>
> Immovable::Immovable(const ImmovableDescr& imm_descr)
> : BaseImmovable(imm_descr),
> + antecedent_(nullptr),
I am not familiar with this word. Can you find a simpler name?
> anim_(0),
> animstart_(0),
> program_(nullptr),
>
> === modified file 'src/logic/map_objects/immovable.h'
> --- src/logic/map_objects/immovable.h 2016-11-03 07:20:57 +0000
> +++ src/logic/map_objects/immovable.h 2016-11-03 07:36:43 +0000
> @@ -201,7 +201,11 @@
> Immovable(const ImmovableDescr&);
> ~Immovable();
>
> - void set_owner(Player*);
> + /// If this immovable was created by a building, this can be set in
> order to display information
> + /// about it. If this is a player immovable, you will need to set the
> owner first.
> + void set_antecedent(const BuildingDescr& building);
> +
> + void set_owner(Player* player);
I think this function is overwritten in base classes. You'll have a bad time if
you do not mark it as virtual here. Ideally you want to remove the overwritten
functions too.
>
> Coords get_position() const {
> return position_;
>
> === modified file 'src/logic/map_objects/tribes/building.cc'
> --- src/logic/map_objects/tribes/building.cc 2016-11-03 07:20:57 +0000
> +++ src/logic/map_objects/tribes/building.cc 2016-11-03 07:36:43 +0000
> @@ -441,8 +442,12 @@
> const Coords pos = position_;
> PlayerImmovable::destroy(egbase);
> // We are deleted. Only use stack variables beyond this point
> - if (fire)
> - egbase.create_immovable(pos, "destroyed_building",
> MapObjectDescr::OwnerType::kTribe);
> + if (fire) {
> + Immovable& destroyed_building =
> + egbase.create_immovable(pos, "destroyed_building",
> MapObjectDescr::OwnerType::kTribe);
You'll probably end up with cleaner code if you get rid of 'set_owner' and
'set_antcedent' and instead add a 'create_immovable_with_precedessor' in
EditorGame and set up all the state in the constructor of Immovable and/or in
the EgBase function.
> + destroyed_building.set_owner(get_owner());
> + destroyed_building.set_antecedent(descr());
> + }
> }
>
> std::string Building::info_string(const InfoStringFormat& format) {
--
https://code.launchpad.net/~widelands-dev/widelands/bug-863185-census-on-destroyed-building/+merge/309818
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-863185-census-on-destroyed-building.
_______________________________________________
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
