Merge lp:~widelands-dev/widelands/expedition_portspace_indicator into lp:widelands

Proposed by Benedikt Straub
Status: Merged
Merged at revision: 9093
Proposed branch: lp:~widelands-dev/widelands/expedition_portspace_indicator
Merge into: lp:widelands
Diff against target: 171 lines (+61/-6)
4 files modified
src/game_io/game_interactive_player_packet.cc (+24/-4)
src/wui/interactive_base.cc (+25/-0)
src/wui/interactive_base.h (+8/-0)
src/wui/interactive_player.cc (+4/-2)
To merge this branch: bzr merge lp:~widelands-dev/widelands/expedition_portspace_indicator
Reviewer Review Type Date Requested Status
GunChleoc Approve
Benedikt Straub Needs Resubmitting
Review via email: mp+366640@code.launchpad.net

Commit message

Show a port icon on the field where an expedition ship can build a port

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4835. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/525916562.
Appveyor build 4616. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_expedition_portspace_indicator-4616.

Revision history for this message
GunChleoc (gunchleoc) wrote :

We are trying to get rid of references to InteractiveBase from the logic library.

Would you like to try your hand at adding new actions to NoteShip and subscribing to those in InteractivePlayer?

After placing a port construction site, a big building plot icon is shown on top of the construction site.

review: Needs Fixing
9089. By Nordfriese

Use notifications instead

9090. By Nordfriese

Saveloading support

Revision history for this message
Benedikt Straub (nordfriese) wrote :

OK, now InteractiveBase receives a notification instead when a portspace is found. Since there are fairly many reasons why a port space can become unavailable again (ship sails on, port is constructed, ship is sunk…), the IBase takes care of cleaning up the set itself now when think()ing. This fixes the bug that the indicator won´t go away on portconstruction.
Also added saving support for the indicators so they are also shown for ships that already are in PortSpaceFound state when the game is loaded.

review: Needs Resubmitting
Revision history for this message
GunChleoc (gunchleoc) wrote :

I am still wondering if we can get more performance out of this thing.

* Ship sails on and ship is sunk could be taken care of by the ShipNote
* Port being built can be taken care of by buildcaps, just like we do for the building spaces overlays in general

Any other reasons?

+1 for the saving support :)

Revision history for this message
Benedikt Straub (nordfriese) wrote :

> Port being built can be taken care of by buildcaps
IMHO we should remove the portspaces either only in response to Notes or only autonomously. Mixing both feels messy.
To use notes, we´d need subscriptions (in addition to what we already have) to
– NoteExpeditionCanceled
– FieldPossession (another possible reason why a portspace might become unavailable is that some other player conquers that land, which is not even necessarily reflected in the buildcaps)
– FieldTerrainChanged (if the port space is removed by a terrain-changing script)
– A tree grows over the port space (or is this forbidden somehow?) → is there a notification for that?
– A script unsets a port space directly → is there a notification for that?

That´s quite a lot for a pretty small feature IMHO; and I don´t think the autonomous cleanup of displayed portspaces is performance-critical – I can´t imagine a player will have so many expeditions at a time that it really matters.

Revision history for this message
GunChleoc (gunchleoc) wrote :

You have convinced me.

I have done more testing - all working now :)

@bunnybot merge

Revision history for this message
GunChleoc (gunchleoc) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/game_io/game_interactive_player_packet.cc'
2--- src/game_io/game_interactive_player_packet.cc 2019-02-23 11:00:49 +0000
3+++ src/game_io/game_interactive_player_packet.cc 2019-05-03 09:15:36 +0000
4@@ -25,6 +25,8 @@
5 #include "logic/game_data_error.h"
6 #include "logic/map_objects/tribes/tribe_descr.h"
7 #include "logic/player.h"
8+#include "map_io/map_object_loader.h"
9+#include "map_io/map_object_saver.h"
10 #include "wui/interactive_player.h"
11 #include "wui/mapview.h"
12
13@@ -32,16 +34,16 @@
14
15 namespace {
16
17-constexpr uint16_t kCurrentPacketVersion = 4;
18+constexpr uint16_t kCurrentPacketVersion = 5;
19
20 } // namespace
21
22-void GameInteractivePlayerPacket::read(FileSystem& fs, Game& game, MapObjectLoader*) {
23+void GameInteractivePlayerPacket::read(FileSystem& fs, Game& game, MapObjectLoader* mol) {
24 try {
25 FileRead fr;
26 fr.open(fs, "binary/interactive_player");
27 uint16_t const packet_version = fr.unsigned_16();
28- if (packet_version == kCurrentPacketVersion) {
29+ if (packet_version <= kCurrentPacketVersion && packet_version >= 4) {
30 PlayerNumber player_number = fr.unsigned_8();
31 if (!(0 < player_number && player_number <= game.map().get_nrplayers())) {
32 throw GameDataError("Invalid player number: %i.", player_number);
33@@ -92,6 +94,17 @@
34 ibase->set_landmark(i, view);
35 }
36 }
37+
38+ if (packet_version == kCurrentPacketVersion) {
39+ size_t nr_port_spaces = fr.unsigned_32();
40+ for (size_t i = 0; i < nr_port_spaces; ++i) {
41+ uint32_t serial = fr.unsigned_32();
42+ int16_t x = fr.signed_16();
43+ int16_t y = fr.signed_16();
44+ ibase->get_expedition_port_spaces().emplace(&mol->get<Widelands::Ship>(serial),
45+ Widelands::Coords(x, y));
46+ }
47+ }
48 }
49 } else {
50 throw UnhandledVersionError(
51@@ -105,7 +118,7 @@
52 /*
53 * Write Function
54 */
55-void GameInteractivePlayerPacket::write(FileSystem& fs, Game& game, MapObjectSaver* const) {
56+void GameInteractivePlayerPacket::write(FileSystem& fs, Game& game, MapObjectSaver* const mos) {
57 FileWrite fw;
58
59 fw.unsigned_16(kCurrentPacketVersion);
60@@ -138,6 +151,13 @@
61 fw.float_32(landmark.view.viewpoint.y);
62 fw.float_32(landmark.view.zoom);
63 }
64+
65+ fw.unsigned_32(ibase->get_expedition_port_spaces().size());
66+ for (const auto& pair : ibase->get_expedition_port_spaces()) {
67+ fw.unsigned_32(mos->get_object_file_index(*pair.first));
68+ fw.signed_16(pair.second.x);
69+ fw.signed_16(pair.second.y);
70+ }
71 }
72
73 fw.write(fs, "binary/interactive_player");
74
75=== modified file 'src/wui/interactive_base.cc'
76--- src/wui/interactive_base.cc 2019-04-25 06:31:33 +0000
77+++ src/wui/interactive_base.cc 2019-05-03 09:15:36 +0000
78@@ -152,6 +152,12 @@
79 });
80 sound_subscriber_ = Notifications::subscribe<NoteSound>(
81 [this](const NoteSound& note) { play_sound_effect(note); });
82+ shipnotes_subscriber_ = Notifications::subscribe<Widelands::NoteShip>([this](const Widelands::NoteShip& note) {
83+ if (note.action == Widelands::NoteShip::Action::kWaitingForCommand &&
84+ note.ship->get_ship_state() == Widelands::Ship::ShipStates::kExpeditionPortspaceFound) {
85+ expedition_port_spaces_.emplace(note.ship, note.ship->exp_port_spaces().front());
86+ }
87+ });
88
89 toolbar_.set_layout_toplevel(true);
90 map_view_.changeview.connect([this] { mainview_move(); });
91@@ -304,6 +310,15 @@
92 void InteractiveBase::on_buildhelp_changed(bool /* value */) {
93 }
94
95+bool InteractiveBase::has_expedition_port_space(const Widelands::Coords& coords) const {
96+ for (const auto& pair : expedition_port_spaces_) {
97+ if (pair.second == coords) {
98+ return true;
99+ }
100+ }
101+ return false;
102+}
103+
104 // Show the given workareas at the given coords and returns the overlay job id associated
105 void InteractiveBase::show_workarea(const WorkareaInfo& workarea_info, Widelands::Coords coords) {
106 workarea_previews_[coords] = &workarea_info;
107@@ -457,6 +472,16 @@
108 }
109 egbase().think(); // Call game logic here. The game advances.
110
111+ // Cleanup found port spaces if the ship sailed on or was destroyed
112+ for (auto it = expedition_port_spaces_.begin(); it != expedition_port_spaces_.end(); ++it) {
113+ if (!egbase().objects().object_still_available(it->first) ||
114+ it->first->get_ship_state() != Widelands::Ship::ShipStates::kExpeditionPortspaceFound) {
115+ expedition_port_spaces_.erase(it);
116+ // If another port space also needs removing, we'll take care of it in the next frame
117+ return;
118+ }
119+ }
120+
121 UI::Panel::think();
122 }
123
124
125=== modified file 'src/wui/interactive_base.h'
126--- src/wui/interactive_base.h 2019-04-25 06:31:33 +0000
127+++ src/wui/interactive_base.h 2019-05-03 09:15:36 +0000
128@@ -86,6 +86,11 @@
129 void show_workarea(const WorkareaInfo& workarea_info, Widelands::Coords coords);
130 void hide_workarea(const Widelands::Coords& coords);
131
132+ bool has_expedition_port_space(const Widelands::Coords&) const;
133+ std::map<Widelands::Ship*, Widelands::Coords>& get_expedition_port_spaces() {
134+ return expedition_port_spaces_;
135+ }
136+
137 // point of view for drawing
138 virtual Widelands::Player* get_player() const = 0;
139
140@@ -291,11 +296,14 @@
141 // coordinate that the building that shows the work area is positioned.
142 std::map<Widelands::Coords, const WorkareaInfo*> workarea_previews_;
143
144+ std::map<Widelands::Ship*, Widelands::Coords> expedition_port_spaces_;
145+
146 RoadBuildingOverlays road_building_overlays_;
147
148 std::unique_ptr<Notifications::Subscriber<GraphicResolutionChanged>>
149 graphic_resolution_changed_subscriber_;
150 std::unique_ptr<Notifications::Subscriber<NoteSound>> sound_subscriber_;
151+ std::unique_ptr<Notifications::Subscriber<Widelands::NoteShip>> shipnotes_subscriber_;
152 Widelands::EditorGameBase& egbase_;
153 uint32_t display_flags_;
154 uint32_t lastframe_; // system time (milliseconds)
155
156=== modified file 'src/wui/interactive_player.cc'
157--- src/wui/interactive_player.cc 2019-04-25 06:31:33 +0000
158+++ src/wui/interactive_player.cc 2019-05-03 09:15:36 +0000
159@@ -339,8 +339,10 @@
160
161 if (f->vision > 0) {
162 // Draw build help.
163- if (buildhelp()) {
164- const auto* overlay = get_buildhelp_overlay(plr.get_buildcaps(f->fcoords));
165+ bool show_port_space = has_expedition_port_space(f->fcoords);
166+ if (show_port_space || buildhelp()) {
167+ const auto* overlay = get_buildhelp_overlay(show_port_space ?
168+ f->fcoords.field->maxcaps() : plr.get_buildcaps(f->fcoords));
169 if (overlay != nullptr) {
170 blit_field_overlay(dst, *f, overlay->pic, overlay->hotspot, scale);
171 }

Subscribers

People subscribed via source and target branches

to status/vote changes: