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,
_______________________________________________ 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