Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view into lp:widelands

2018-04-28 Thread Notabilis 
Indeed, spectators are working. Guess I mixed something up.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1623375-multiplayer-starting-view/+merge/343287
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view.

___
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

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view into lp:widelands

2018-04-27 Thread noreply 
The proposal to merge 
lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view into 
lp:widelands has been updated.

Status: Needs review => Merged

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1623375-multiplayer-starting-view/+merge/343287
-- 
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view.

___
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view into lp:widelands

2018-04-26 Thread GunChleoc 
Spectators already work for new games. I have added your fix.

Thanks for the review :)

@bunnybot merge

Diff comments:

> 
> === modified file 'src/wui/interactive_gamebase.cc'
> --- src/wui/interactive_gamebase.cc   2018-04-07 16:59:00 +
> +++ src/wui/interactive_gamebase.cc   2018-04-16 07:21:13 +
> @@ -148,6 +148,27 @@
>   hide_minimap();
>  }
>  
> +void InteractiveGameBase::start() {
> + InteractiveBase::start();

I really don't know what's best practice here, so I'll go with "less code is 
better" and remove it.

> + // Multiplayer games don't save the view position, so we go to the 
> starting position instead
> + if (is_multiplayer()) {
> + Widelands::PlayerNumber pln = player_number();
> + const Widelands::PlayerNumber max = 
> game().map().get_nrplayers();
> + if (pln == 0) {
> + // Spectator, use the view of the first viable player
> + for (pln = 1; pln <= max; ++pln) {
> + if (game().get_player(pln)) {
> + break;
> + }
> + }
> + }
> + // Adding a check, just in case there was no viable player 
> found for spectator
> + if (game().get_player(pln)) {
> + 
> map_view()->scroll_to_field(game().map().get_starting_pos(pln), 
> MapView::Transition::Jump);
> + }
> + }
> +}
> +
>  void InteractiveGameBase::on_buildhelp_changed(const bool value) {
>   toggle_buildhelp_->set_perm_pressed(value);
>  }


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1623375-multiplayer-starting-view/+merge/343287
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view.

___
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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view into lp:widelands

2018-04-24 Thread Notabilis 
Review: Approve

Testing worked fine, one comment in the diff.

As a suggestion: What about moving spectators to the first player position on 
new games, too?

Diff comments:

> 
> === modified file 'src/wui/interactive_gamebase.cc'
> --- src/wui/interactive_gamebase.cc   2018-04-07 16:59:00 +
> +++ src/wui/interactive_gamebase.cc   2018-04-16 07:21:13 +
> @@ -148,6 +148,27 @@
>   hide_minimap();
>  }
>  
> +void InteractiveGameBase::start() {
> + InteractiveBase::start();

This call is new (as in: hasn't been done in start() before), right? Seems to 
just call the empty Panel:start().
Is there a reason that this is called? The previous empty override was making 
explicit that it isn't.

> + // Multiplayer games don't save the view position, so we go to the 
> starting position instead
> + if (is_multiplayer()) {
> + Widelands::PlayerNumber pln = player_number();
> + const Widelands::PlayerNumber max = 
> game().map().get_nrplayers();
> + if (pln == 0) {
> + // Spectator, use the view of the first viable player
> + for (pln = 1; pln <= max; ++pln) {
> + if (game().get_player(pln)) {
> + break;
> + }
> + }
> + }
> + // Adding a check, just in case there was no viable player 
> found for spectator
> + if (game().get_player(pln)) {
> + 
> map_view()->scroll_to_field(game().map().get_starting_pos(pln), 
> MapView::Transition::Jump);
> + }
> + }
> +}
> +
>  void InteractiveGameBase::on_buildhelp_changed(const bool value) {
>   toggle_buildhelp_->set_perm_pressed(value);
>  }


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1623375-multiplayer-starting-view/+merge/343287
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view.

___
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

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view into lp:widelands

2018-04-16 Thread bunnybot 
Continuous integration builds have changed state:

Travis build 3382. State: passed. Details: 
https://travis-ci.org/widelands/widelands/builds/367035337.
Appveyor build 3188. State: success. Details: 
https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1623375_multiplayer_starting_view-3188.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1623375-multiplayer-starting-view/+merge/343287
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view into 
lp:widelands.

___
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

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view into lp:widelands

2018-04-16 Thread GunChleoc 
GunChleoc has proposed merging 
lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view into 
lp:widelands.

Commit message:
In multiplayer games, scroll to starting position when a game is loaded.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1623375 in widelands: "Map viewpoint and location markers not read in 
multiplayer mode"
  https://bugs.launchpad.net/widelands/+bug/1623375

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1623375-multiplayer-starting-view/+merge/343287

Gets rid of starting loaded multiplayer games with the map view at position 
0,0, which is often unexplored.
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view into 
lp:widelands.
=== modified file 'src/logic/game.cc'
--- src/logic/game.cc	2018-04-07 16:59:00 +
+++ src/logic/game.cc	2018-04-16 07:18:48 +
@@ -452,6 +452,8 @@
 		}
 
 		if (get_ipl()) {
+			// Scroll map to startng position for new games.
+			// Loaded games are handled in GameInteractivePlayerPacket for single player, and in InteractiveGameBase::start() for multiplayer.
 			get_ipl()->map_view()->scroll_to_field(
 			   map().get_starting_pos(get_ipl()->player_number()), MapView::Transition::Jump);
 		}

=== modified file 'src/wui/interactive_gamebase.cc'
--- src/wui/interactive_gamebase.cc	2018-04-07 16:59:00 +
+++ src/wui/interactive_gamebase.cc	2018-04-16 07:18:48 +
@@ -148,6 +148,27 @@
 	hide_minimap();
 }
 
+void InteractiveGameBase::start() {
+	InteractiveBase::start();
+	// Multiplayer games don't save the view position, so we go to the starting position instead
+	if (is_multiplayer()) {
+		Widelands::PlayerNumber pln = player_number();
+		const Widelands::PlayerNumber max = game().map().get_nrplayers();
+		if (pln == 0) {
+			// Spectator, use the view of the first viable player
+			for (pln = 1; pln <= max; ++pln) {
+if (game().get_player(pln)) {
+	break;
+}
+			}
+		}
+		// Adding a check, just in case there was no viable player found for spectator
+		if (game().get_player(pln)) {
+			map_view()->scroll_to_field(game().map().get_starting_pos(pln), MapView::Transition::Jump);
+		}
+	}
+}
+
 void InteractiveGameBase::on_buildhelp_changed(const bool value) {
 	toggle_buildhelp_->set_perm_pressed(value);
 }

=== modified file 'src/wui/interactive_gamebase.h'
--- src/wui/interactive_gamebase.h	2018-04-07 16:59:00 +
+++ src/wui/interactive_gamebase.h	2018-04-16 07:18:48 +
@@ -92,8 +92,7 @@
 
 	void show_game_summary();
 	void postload() override;
-	void start() override {
-	}
+	void start() override;
 
 protected:
 	void draw_overlay(RenderTarget&) override;

___
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