widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #11073
[Merge] lp:~widelands-dev/widelands/flatten_map_object_descr into lp:widelands
SirVer has proposed merging lp:~widelands-dev/widelands/flatten_map_object_descr into lp:widelands.
Commit message:
Flatten PlayerField::map_object_descr.
There once was a vague plan of moving immovables from nodes to triangles. The Player code was written to support this, but the plan was scratched, though the code never deleted. The array was indexed with TriangleIndex::[R|D|None] and this is the only use of TriangleIndex::None - which is a confusing concept I want to get rid off.
I kept the save/load code intact since ripping it out was more complicated than I wanted to invest right now.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/flatten_map_object_descr/+merge/329994
--
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/flatten_map_object_descr into lp:widelands.
=== modified file 'src/logic/editor_game_base.cc'
--- src/logic/editor_game_base.cc 2017-08-20 15:45:50 +0000
+++ src/logic/editor_game_base.cc 2017-08-31 10:18:08 +0000
@@ -180,7 +180,7 @@
iterate_players_existing_const(plnum, kMaxPlayers, *this, p) {
Player::Field& player_field = p->fields_[i];
if (1 < player_field.vision) {
- player_field.map_object_descr[TCoords<>::None] = descr;
+ player_field.map_object_descr = descr;
}
}
}
=== modified file 'src/logic/player.cc'
--- src/logic/player.cc 2017-08-19 22:22:20 +0000
+++ src/logic/player.cc 2017-08-31 10:18:08 +0000
@@ -942,8 +942,7 @@
field.border_bl = ((1 | br_vision) && (br_owner_number == field.owner) &&
((r_owner_number == field.owner) ^ (bl_owner_number == field.owner)));
- { // map_object_descr[TCoords::None]
-
+ {
const MapObjectDescr* map_object_descr;
field.constructionsite.becomes = nullptr;
if (const BaseImmovable* base_immovable = f.field->get_immovable()) {
@@ -963,7 +962,7 @@
}
} else
map_object_descr = nullptr;
- field.map_object_descr[TCoords<>::None] = map_object_descr;
+ field.map_object_descr = map_object_descr;
}
}
{ // discover the D triangle and the SW edge of the top right neighbour
=== modified file 'src/logic/player.h'
--- src/logic/player.h 2017-08-16 13:23:15 +0000
+++ src/logic/player.h 2017-08-31 10:18:08 +0000
@@ -209,6 +209,7 @@
roads(0),
owner(0),
time_node_last_unseen(0),
+ map_object_descr(nullptr),
border(0),
border_r(0),
border_br(0),
@@ -220,9 +221,6 @@
time_triangle_last_surveyed[0] = never();
time_triangle_last_surveyed[1] = never();
-
- // Initialized for debug purposes only.
- map_object_descr[0] = map_object_descr[1] = map_object_descr[2] = nullptr;
}
/// Military influence is exerted by buildings with the help of soldiers.
@@ -377,7 +375,7 @@
* Only valid when the player has seen this node (or maybe a nearby node
* if the immovable is big?). (Roads are not stored here.)
*/
- const MapObjectDescr* map_object_descr[3];
+ const MapObjectDescr* map_object_descr;
/// Information for constructionsite's animation.
/// only valid, if there is a constructionsite on this node
@@ -405,9 +403,7 @@
// time_triangle_last_surveyed[0] 0x040 0x20 0x040 0x20
// time_triangle_last_surveyed[1] 0x060 0x20 0x060 0x20
// time_node_last_unseen 0x080 0x20 0x080 0x20
- // map_object_descr[0] 0x0a0 0x20 0x0a0 0x40
- // map_object_descr[1] 0x0c0 0x20 0x0e0 0x40
- // map_object_descr[2] 0x0e0 0x20 0x120 0x40
+ // map_object_descr 0x0a0 0x20 0x0a0 0x40
// ConstructionsiteInformation
// border
// border_r
=== modified file 'src/map_io/map_players_view_packet.cc'
--- src/map_io/map_players_view_packet.cc 2017-06-24 08:47:46 +0000
+++ src/map_io/map_players_view_packet.cc 2017-08-31 10:18:08 +0000
@@ -351,7 +351,7 @@
map_object_descr = nullptr;
} else
map_object_descr = nullptr;
- f_player_field.map_object_descr[TCoords<>::None] = map_object_descr;
+ f_player_field.map_object_descr = map_object_descr;
}
{ // triangles
@@ -554,7 +554,7 @@
}
MapObjectData mod = read_unseen_immovable(
egbase, imm_kind, node_immovables_file, node_immovables_file_version);
- f_player_field.map_object_descr[TCoords<>::None] = mod.map_object_descr;
+ f_player_field.map_object_descr = mod.map_object_descr;
f_player_field.constructionsite = mod.csi;
// Read in whether this field had a border the last time it was seen
@@ -593,7 +593,7 @@
map_object_descr = nullptr;
} else
map_object_descr = nullptr;
- f_player_field.map_object_descr[TCoords<>::None] = map_object_descr;
+ f_player_field.map_object_descr = map_object_descr;
break;
}
@@ -603,7 +603,6 @@
// information about the triangle has not been saved. Fill in
// the information from the game state.
f_player_field.terrains.d = f.field->terrain_d();
- f_player_field.map_object_descr[TCoords<>::D] = nullptr;
} else if (f_everseen | bl_everseen | br_everseen) {
// The player has seen the D triangle but does not see it now.
// Load his information about the triangle from file.
@@ -621,16 +620,18 @@
triangle_immovable_kinds_file_version,
kCurrentPacketVersionImmovableKinds);
}
- MapObjectData mod = read_unseen_immovable(
+ // We read and ignore the immovable information on the D
+ // triangle. This was done because there were vague plans of
+ // suporting immovables on the triangles instead as on the
+ // nodes.
+ read_unseen_immovable(
egbase, im_kind, triangle_immovables_file, triangle_immovables_file_version);
- f_player_field.map_object_descr[TCoords<>::D] = mod.map_object_descr;
}
if (f_seen | br_seen | r_seen) {
// The player currently sees the R triangle. Therefore his
// information about the triangle has not been saved. Fill in
// the information from the game state.
f_player_field.terrains.r = f.field->terrain_r();
- f_player_field.map_object_descr[TCoords<>::R] = nullptr;
} else if (f_everseen | br_everseen | r_everseen) {
// The player has seen the R triangle but does not see it now.
// Load his information about the triangle from file.
@@ -648,9 +649,12 @@
triangle_immovable_kinds_file_version,
kCurrentPacketVersionImmovableKinds);
}
- MapObjectData mod = read_unseen_immovable(
+ // We read and ignore the immovable information on the D
+ // triangle. This was done because there were vague plans of
+ // suporting immovables on the triangles instead as on the
+ // nodes.
+ read_unseen_immovable(
egbase, im_kind, triangle_immovables_file, triangle_immovables_file_version);
- f_player_field.map_object_descr[TCoords<>::R] = mod.map_object_descr;
}
{ // edges
@@ -895,7 +899,7 @@
assert(f_player_field.owner < 0x20);
owners_file.unsigned_8(f_player_field.owner);
MapObjectData mod;
- mod.map_object_descr = f_player_field.map_object_descr[TCoords<>::None];
+ mod.map_object_descr = f_player_field.map_object_descr;
mod.csi = f_player_field.constructionsite;
write_unseen_immovable(&mod, node_immovable_kinds_file, node_immovables_file);
@@ -915,7 +919,7 @@
(!bl_seen & !br_seen & (f_everseen | bl_everseen | br_everseen)) {
terrains_file.unsigned_8(f_player_field.terrains.d);
MapObjectData mod;
- mod.map_object_descr = f_player_field.map_object_descr[TCoords<>::D];
+ mod.map_object_descr = nullptr;
write_unseen_immovable(
&mod, triangle_immovable_kinds_file, triangle_immovables_file);
}
@@ -925,7 +929,7 @@
(!br_seen & !r_seen & (f_everseen | br_everseen | r_everseen)) {
terrains_file.unsigned_8(f_player_field.terrains.r);
MapObjectData mod;
- mod.map_object_descr = f_player_field.map_object_descr[TCoords<>::R];
+ mod.map_object_descr = nullptr;
write_unseen_immovable(
&mod, triangle_immovable_kinds_file, triangle_immovables_file);
}
=== modified file 'src/wui/game_debug_ui.cc'
--- src/wui/game_debug_ui.cc 2017-08-19 22:22:20 +0000
+++ src/wui/game_debug_ui.cc 2017-08-31 10:18:08 +0000
@@ -310,10 +310,9 @@
break;
case 1: {
std::string animation_name = "(no animation)";
- if (player_field.map_object_descr[Widelands::TCoords<>::None]) {
+ if (player_field.map_object_descr) {
animation_name = "(seen an animation)";
}
-
str += (boost::format(" last seen at %u:\n"
" owner: %u\n"
" immovable animation:\n%s\n"
=== modified file 'src/wui/interactive_player.cc'
--- src/wui/interactive_player.cc 2017-08-29 10:48:24 +0000
+++ src/wui/interactive_player.cc 2017-08-31 10:18:08 +0000
@@ -125,74 +125,74 @@
const Widelands::Player::Field& player_field,
const float scale,
RenderTarget* dst) {
- if (const Widelands::MapObjectDescr* const map_object_descr =
- player_field.map_object_descr[Widelands::TCoords<>::None]) {
- 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);
- } else if (upcast(const Widelands::BuildingDescr, building, 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");
- }
+ if (player_field.map_object_descr == nullptr) {
+ return;
+ }
+ 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);
+ } 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());
+ } 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, field.owner->get_playercolor());
+ } else if (const uint32_t pic = player_field.map_object_descr->main_animation()) {
+ if (field.owner != nullptr) {
dst->blit_animation(
- field.rendertarget_pixel, scale, pic, 0, field.owner->get_playercolor());
- } else if (map_object_descr->type() == Widelands::MapObjectType::FLAG) {
- assert(field.owner != nullptr);
- dst->blit_animation(field.rendertarget_pixel, scale, field.owner->tribe().flag_animation(),
- 0, field.owner->get_playercolor());
- } else if (const uint32_t pic = map_object_descr->main_animation()) {
- if (field.owner != nullptr) {
- dst->blit_animation(
- field.rendertarget_pixel, scale, pic, 0, field.owner->get_playercolor());
- } else {
- dst->blit_animation(field.rendertarget_pixel, scale, pic, 0);
- }
+ field.rendertarget_pixel, scale, pic, 0, field.owner->get_playercolor());
+ } else {
+ dst->blit_animation(field.rendertarget_pixel, scale, pic, 0);
}
}
}
Follow ups
