widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #12017
[Merge] lp:~widelands-dev/widelands/bug-1738054-random-battle-animations into lp:widelands
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1738054-random-battle-animations into lp:widelands.
Commit message:
Added missing call to get_rand_anim when a soldier is dying. Also, removed some code duplication and programming by exception.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1738054 in widelands: "Random soldier battle animations don´t work"
https://bugs.launchpad.net/widelands/+bug/1738054
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1738054-random-battle-animations/+merge/335296
--
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1738054-random-battle-animations into lp:widelands.
=== modified file 'src/logic/game_data_error.h'
--- src/logic/game_data_error.h 2017-01-25 18:55:59 +0000
+++ src/logic/game_data_error.h 2017-12-17 17:44:30 +0000
@@ -62,6 +62,6 @@
UnhandledVersionError() {
}
};
-}
+} // namespace Widelands
#endif // end of include guard: WL_LOGIC_GAME_DATA_ERROR_H
=== modified file 'src/logic/map_objects/immovable.cc'
--- src/logic/map_objects/immovable.cc 2017-11-10 09:31:46 +0000
+++ src/logic/map_objects/immovable.cc 2017-12-17 17:44:30 +0000
@@ -575,10 +575,10 @@
char const* const animname = fr.c_string();
try {
imm.anim_ = imm.descr().get_animation(animname);
- } catch (const MapObjectDescr::AnimationNonexistent&) {
+ } catch (const GameDataError& e) {
imm.anim_ = imm.descr().main_animation();
- log("Warning: (%s) Animation \"%s\" not found, using animation %s).\n",
- imm.descr().name().c_str(), animname, imm.descr().get_animation_name(imm.anim_).c_str());
+ log("Warning: Immovable: %s, using animation %s instead.\n",
+ e.what(), imm.descr().get_animation_name(imm.anim_).c_str());
}
imm.animstart_ = fr.signed_32();
if (packet_version >= 4) {
=== modified file 'src/logic/map_objects/map_object.cc'
--- src/logic/map_objects/map_object.cc 2017-11-13 08:03:31 +0000
+++ src/logic/map_objects/map_object.cc 2017-12-17 17:44:30 +0000
@@ -37,6 +37,7 @@
#include "io/filewrite.h"
#include "logic/cmd_queue.h"
#include "logic/game.h"
+#include "logic/game_data_error.h"
#include "logic/player.h"
#include "logic/queue_cmd_ids.h"
#include "map_io/map_object_loader.h"
@@ -276,14 +277,29 @@
const std::string anim_name = prefix + std::string("_") + dirstrings[dir - 1];
try {
anims->set_animation(dir, get_animation(anim_name));
- } catch (const MapObjectDescr::AnimationNonexistent&) {
- throw GameDataError("MO: no directional animation '%s'", anim_name.c_str());
+ } catch (const GameDataError& e) {
+ throw GameDataError("MO: Missing directional animation: %s", e.what());
}
}
}
+uint32_t MapObjectDescr::get_animation(char const* const anim) const {
+ std::map<std::string, uint32_t>::const_iterator it = anims_.find(anim);
+ if (it == anims_.end()) {
+ throw GameDataError("Unknown animation: %s for %s", anim, name().c_str());
+ }
+ return it->second;
+}
+
+uint32_t MapObjectDescr::get_animation(const std::string& animname) const {
+ return get_animation(animname.c_str());
+}
+
+uint32_t MapObjectDescr::main_animation() const {
+ return !anims_.empty() ? anims_.begin()->second : 0;
+}
+
std::string MapObjectDescr::get_animation_name(uint32_t const anim) const {
-
for (const auto& temp_anim : anims_) {
if (temp_anim.second == anim) {
return temp_anim.first;
=== modified file 'src/logic/map_objects/map_object.h'
--- src/logic/map_objects/map_object.h 2017-11-27 08:22:14 +0000
+++ src/logic/map_objects/map_object.h 2017-12-17 17:44:30 +0000
@@ -120,24 +120,10 @@
return type_;
}
- struct AnimationNonexistent {};
- uint32_t get_animation(char const* const anim) const {
- std::map<std::string, uint32_t>::const_iterator it = anims_.find(anim);
- if (it == anims_.end())
- throw AnimationNonexistent();
- return it->second;
- }
- uint32_t get_animation(const std::string& animname) const {
- return get_animation(animname.c_str());
- }
-
- uint32_t main_animation() const {
- return !anims_.empty() ? anims_.begin()->second : 0;
- }
-
+ uint32_t get_animation(char const* const anim) const;
+ uint32_t get_animation(const std::string& animname) const;
+ uint32_t main_animation() const ;
std::string get_animation_name(uint32_t) const; ///< needed for save, debug
- bool has_attribute(uint32_t) const;
- static uint32_t get_attribute_id(const std::string& name, bool add_if_not_exists = false);
bool is_animation_known(const std::string& name) const;
void add_animation(const std::string& name, uint32_t anim);
@@ -158,6 +144,9 @@
/// Returns the image fileneme for the menu image if the MapObject has one, is empty otherwise
const std::string& icon_filename() const;
+ bool has_attribute(uint32_t) const;
+ static uint32_t get_attribute_id(const std::string& name, bool add_if_not_exists = false);
+
protected:
// Add all the special attributes to the attribute list. Only the 'allowed_special'
// attributes are allowed to appear - i.e. resi are fine for immovables.
=== modified file 'src/logic/map_objects/tribes/building.cc'
--- src/logic/map_objects/tribes/building.cc 2017-11-30 10:59:06 +0000
+++ src/logic/map_objects/tribes/building.cc 2017-12-17 17:44:30 +0000
@@ -197,6 +197,11 @@
hints_.set_trainingsites_max_percent(percent);
}
+uint32_t BuildingDescr::get_unoccupied_animation() const {
+ return get_animation(is_animation_known("unoccupied") ? "unoccupied" : "idle");
+}
+
+
/**
* Normal buildings don't conquer anything, so this returns 0 by default.
*
@@ -357,10 +362,7 @@
}
// Start the animation
- if (descr().is_animation_known("unoccupied"))
- start_animation(egbase, descr().get_animation("unoccupied"));
- else
- start_animation(egbase, descr().get_animation("idle"));
+ start_animation(egbase, descr().get_unoccupied_animation());
leave_time_ = egbase.get_gametime();
return true;
=== modified file 'src/logic/map_objects/tribes/building.h'
--- src/logic/map_objects/tribes/building.h 2017-11-30 10:59:06 +0000
+++ src/logic/map_objects/tribes/building.h 2017-12-17 17:44:30 +0000
@@ -167,6 +167,8 @@
const BuildingHints& hints() const;
void set_hints_trainingsites_max_percent(int percent);
+ uint32_t get_unoccupied_animation() const;
+
protected:
virtual Building& create_object() const = 0;
Building& create_constructionsite() const;
=== modified file 'src/logic/map_objects/tribes/constructionsite.cc'
--- src/logic/map_objects/tribes/constructionsite.cc 2017-06-23 16:16:28 +0000
+++ src/logic/map_objects/tribes/constructionsite.cc 2017-12-17 17:44:30 +0000
@@ -39,6 +39,38 @@
namespace Widelands {
+void ConstructionsiteInformation::draw(const Vector2f& point_on_dst,
+ float scale,
+ const RGBColor& player_color,
+ RenderTarget* dst) const {
+ // Draw the construction site marker
+ const uint32_t anim_idx = becomes->is_animation_known("build") ?
+ becomes->get_animation("build") :
+ becomes->get_unoccupied_animation();
+
+ const Animation& anim = g_gr->animations().get_animation(anim_idx);
+ const size_t nr_frames = anim.nr_frames();
+ const uint32_t cur_frame = totaltime ? completedtime * nr_frames / totaltime : 0;
+ uint32_t anim_time = cur_frame * FRAME_LENGTH;
+
+ if (cur_frame) { // not the first pic
+ // Draw the complete prev pic , so we won't run into trouble if images have different sizes
+ dst->blit_animation(point_on_dst, scale, anim_idx, anim_time - FRAME_LENGTH, player_color);
+ } else if (was) {
+ // Is the first picture but there was another building here before,
+ // get its most fitting picture and draw it instead.
+ dst->blit_animation(
+ point_on_dst, scale, was->get_unoccupied_animation(), anim_time - FRAME_LENGTH, player_color);
+ }
+ // Now blit a segment of the current construction phase from the bottom.
+ int percent = 100 * completedtime * nr_frames;
+ if (totaltime) {
+ percent /= totaltime;
+ }
+ percent -= 100 * cur_frame;
+ dst->blit_animation(point_on_dst, scale, anim_idx, anim_time, player_color, percent);
+}
+
/**
* The contents of 'table' are documented in
* /data/tribes/buildings/partially_finished/constructionsite/init.lua
@@ -323,42 +355,7 @@
info_.completedtime += CONSTRUCTIONSITE_STEP_TIME + gametime - work_steptime_;
}
- uint32_t anim_idx;
- try {
- anim_idx = building().get_animation("build");
- } catch (MapObjectDescr::AnimationNonexistent&) {
- try {
- anim_idx = building().get_animation("unoccupied");
- } catch (MapObjectDescr::AnimationNonexistent) {
- anim_idx = building().get_animation("idle");
- }
- }
- const Animation& anim = g_gr->animations().get_animation(anim_idx);
- const size_t nr_frames = anim.nr_frames();
- const uint32_t cur_frame =
- info_.totaltime ? info_.completedtime * nr_frames / info_.totaltime : 0;
- tanim = cur_frame * FRAME_LENGTH;
-
- if (cur_frame) { // not the first pic
- // Draw the complete prev pic , so we won't run into trouble if images have different sizes
- dst->blit_animation(point_on_dst, scale, anim_idx, tanim - FRAME_LENGTH, player_color);
- } else if (!old_buildings_.empty()) {
- DescriptionIndex prev_idx = old_buildings_.back();
- const BuildingDescr* prev_building = owner().tribe().get_building_descr(prev_idx);
- // Is the first picture but there was another building here before,
- // get its most fitting picture and draw it instead.
- const uint32_t prev_building_anim_idx = prev_building->get_animation(
- prev_building->is_animation_known("unoccupied") ? "unoccupied" : "idle");
- dst->blit_animation(
- point_on_dst, scale, prev_building_anim_idx, tanim - FRAME_LENGTH, player_color);
- }
- // Now blit a segment of the current construction phase from the bottom.
- int percent = 100 * info_.completedtime * nr_frames;
- if (info_.totaltime) {
- percent /= info_.totaltime;
- }
- percent -= 100 * cur_frame;
- dst->blit_animation(point_on_dst, scale, anim_idx, tanim, player_color, percent);
+ info_.draw(point_on_dst, scale, player_color, dst);
// Draw help strings
draw_info(draw_text, point_on_dst, scale, dst);
=== modified file 'src/logic/map_objects/tribes/constructionsite.h'
--- src/logic/map_objects/tribes/constructionsite.h 2017-06-24 08:47:46 +0000
+++ src/logic/map_objects/tribes/constructionsite.h 2017-12-17 17:44:30 +0000
@@ -36,6 +36,13 @@
struct ConstructionsiteInformation {
ConstructionsiteInformation() : becomes(nullptr), was(nullptr), totaltime(0), completedtime(0) {
}
+
+ /// Draw the partly finished constructionsite
+ void draw(const Vector2f& point_on_dst,
+ float scale,
+ const RGBColor& player_color,
+ RenderTarget* dst) const;
+
const BuildingDescr*
becomes; // Also works as a marker telling whether there is a construction site.
const BuildingDescr* was; // only valid if "becomes" is an enhanced building.
=== modified file 'src/logic/map_objects/tribes/dismantlesite.cc'
--- src/logic/map_objects/tribes/dismantlesite.cc 2017-08-18 17:32:16 +0000
+++ src/logic/map_objects/tribes/dismantlesite.cc 2017-12-17 17:44:30 +0000
@@ -229,10 +229,8 @@
dst->blit_animation(point_on_dst, scale, anim_, tanim, player_color);
// Blit bottom part of the animation according to dismantle progress
- const uint32_t anim_idx =
- building_->get_animation(building_->is_animation_known("unoccupied") ? "unoccupied" : "idle");
dst->blit_animation(
- point_on_dst, scale, anim_idx, tanim, player_color, 100 - ((get_built_per64k() * 100) >> 16));
+ point_on_dst, scale, building_->get_unoccupied_animation(), tanim, player_color, 100 - ((get_built_per64k() * 100) >> 16));
// Draw help strings
draw_info(draw_text, point_on_dst, scale, dst);
=== modified file 'src/logic/map_objects/tribes/soldier.cc'
--- src/logic/map_objects/tribes/soldier.cc 2017-08-20 17:45:42 +0000
+++ src/logic/map_objects/tribes/soldier.cc 2017-12-17 17:44:30 +0000
@@ -1403,8 +1403,8 @@
// Dead soldier is not owned by a location
set_location(nullptr);
- start_task_idle(
- game, descr().get_animation(combat_walking_ == CD_COMBAT_W ? "die_w" : "die_e"), 1000);
+ const uint32_t anim = descr().get_rand_anim(game, combat_walking_ == CD_COMBAT_W ? "die_w" : "die_e");
+ start_task_idle(game, anim, 1000);
}
void Soldier::die_update(Game& game, State& state) {
=== modified file 'src/map_io/map_buildingdata_packet.cc'
--- src/map_io/map_buildingdata_packet.cc 2017-09-22 20:47:13 +0000
+++ src/map_io/map_buildingdata_packet.cc 2017-12-17 17:44:30 +0000
@@ -36,6 +36,7 @@
#include "io/filewrite.h"
#include "logic/editor_game_base.h"
#include "logic/game.h"
+#include "logic/game_data_error.h"
#include "logic/map.h"
#include "logic/map_objects/tribes/constructionsite.h"
#include "logic/map_objects/tribes/dismantlesite.h"
@@ -93,12 +94,11 @@
char const* const animation_name = fr.c_string();
try {
building.anim_ = building.descr().get_animation(animation_name);
- } catch (const MapObjectDescr::AnimationNonexistent&) {
- log("WARNING: %s %s does not have animation \"%s\"; "
- "using animation \"idle\" instead\n",
- building.owner().tribe().name().c_str(),
- building.descr().descname().c_str(), animation_name);
+ } catch (const GameDataError& e) {
building.anim_ = building.descr().get_animation("idle");
+ log("Warning: Tribe %s building: %s, using animation %s instead.\n",
+ building.owner().tribe().name().c_str(),
+ e.what(), building.descr().get_animation_name(building.anim_).c_str());
}
} else {
building.anim_ = 0;
=== modified file 'src/wui/interactive_player.cc'
--- src/wui/interactive_player.cc 2017-10-24 05:38:53 +0000
+++ src/wui/interactive_player.cc 2017-12-17 17:44:30 +0000
@@ -130,57 +130,12 @@
}
if (player_field.constructionsite.becomes) {
assert(field.owner != nullptr);
- const Widelands::ConstructionsiteInformation& csinf = player_field.constructionsite;
- // draw the partly finished constructionsite
- uint32_t anim_idx;
- try {
- anim_idx = csinf.becomes->get_animation("build");
- } catch (Widelands::MapObjectDescr::AnimationNonexistent&) {
- try {
- anim_idx = csinf.becomes->get_animation("unoccupied");
- } catch (Widelands::MapObjectDescr::AnimationNonexistent) {
- anim_idx = csinf.becomes->get_animation("idle");
- }
- }
- const Animation& anim = g_gr->animations().get_animation(anim_idx);
- const size_t nr_frames = anim.nr_frames();
- uint32_t cur_frame = csinf.totaltime ? csinf.completedtime * nr_frames / csinf.totaltime : 0;
- uint32_t tanim = cur_frame * FRAME_LENGTH;
-
- uint32_t percent = 100 * csinf.completedtime * nr_frames;
- if (csinf.totaltime) {
- percent /= csinf.totaltime;
- }
- percent -= 100 * cur_frame;
-
- if (cur_frame) { // not the first frame
- // Draw the prev frame
- dst->blit_animation(field.rendertarget_pixel, scale, anim_idx, tanim - FRAME_LENGTH,
- field.owner->get_playercolor());
- } else if (csinf.was) {
- // Is the first frame, but there was another building here before,
- // get its last build picture and draw it instead.
- uint32_t a;
- try {
- a = csinf.was->get_animation("unoccupied");
- } catch (Widelands::MapObjectDescr::AnimationNonexistent&) {
- a = csinf.was->get_animation("idle");
- }
- dst->blit_animation(field.rendertarget_pixel, scale, a, tanim - FRAME_LENGTH,
- field.owner->get_playercolor());
- }
- dst->blit_animation(
- field.rendertarget_pixel, scale, anim_idx, tanim, field.owner->get_playercolor(), percent);
+ player_field.constructionsite.draw(field.rendertarget_pixel, scale, field.owner->get_playercolor(), dst);
+
} else if (upcast(const Widelands::BuildingDescr, building, player_field.map_object_descr)) {
assert(field.owner != nullptr);
// this is a building therefore we either draw unoccupied or idle animation
- uint32_t pic;
- try {
- pic = building->get_animation("unoccupied");
- } catch (Widelands::MapObjectDescr::AnimationNonexistent&) {
- pic = building->get_animation("idle");
- }
- dst->blit_animation(field.rendertarget_pixel, scale, pic, 0, field.owner->get_playercolor());
+ dst->blit_animation(field.rendertarget_pixel, scale, building->get_unoccupied_animation(), 0, field.owner->get_playercolor());
} else if (player_field.map_object_descr->type() == Widelands::MapObjectType::FLAG) {
assert(field.owner != nullptr);
dst->blit_animation(field.rendertarget_pixel, scale, field.owner->tribe().flag_animation(), 0,
Follow ups
