Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1526903 into lp:widelands

2016-01-05 Thread TiborB
I believe that I can merge it

@bunnybot merge
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1526903/+merge/281410
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1526903.

___
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-1526903 into lp:widelands

2016-01-04 Thread GunChleoc
> But there is another issue with the number of players: Open editor, open "set
> players", set number of players to two but place only one player to the map.
> Save the map and after reload the map the "set player" window will always show
> two players. If you try to start a game with this map, a error occurs:
> 
> Fatal exception: Widelands could not start the game, because player 2 has no
> starting position.
> You can manually add a starting position with the Widelands Editor to fix this
> problem.
> 
> I will make another bug report for this issue.

Yes, this is unrelated.

> There is couple of sections we can discuss, or perhaps can be ommited for 
> saving in editor:

I think maybe we should discuss this in a new bug, so it won't get lost after 
this branch is merged?
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1526903/+merge/281410
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1526903.

___
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-1526903 into lp:widelands

2015-12-30 Thread kaputtnik
We should check the player directory... but i am unsure if it is needed for 
scritping. Maybe wl_zocker could tell us.

I will test this branch...
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1526903/+merge/281410
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1526903 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


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1526903 into lp:widelands

2015-12-30 Thread kaputtnik
Review: Approve

I couldn't trigger any of the errors described in the related bug reports 
anymore :-)

The file "allowed_building_types" is never been created. So for this it seems 
to be fixed and i give approve for testing.

But there is another issue with the number of players: Open editor, open "set 
players", set number of players to two but place only one player to the map. 
Save the map and after reload the map the "set player" window will always show 
two players. If you try to start a game with this map, a error occurs:

Fatal exception: Widelands could not start the game, because player 2 has no 
starting position.
You can manually add a starting position with the Widelands Editor to fix this 
problem.

I will make another bug report for this issue.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1526903/+merge/281410
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1526903.

___
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-1526903 into lp:widelands

2015-12-30 Thread TiborB
> Could you go through the map saver and have a look at other packets that we 
> don't need?

There is couple of sections we can discuss, or perhaps can be ommited for 
saving in editor:

log("Writing Flag Data ... ");
log("Writing Road Data ... ");
log("Writing Map Objects ... "); - this is probably needed in editor, or isn't 
it?
// DATA PACKETS 3 sections here
log("Writing Node Ownership Data ... ");
log("Writing Exploration Data ... ");
log("Writing Players Unseen Data ... ");
log("Writing Scripting Data ... ");
log("Writing Objective Data ... ");

What is your opinion? Or is here somebody willing to test what can be skipped?


I see kaputtnik approved this. So can I merge to trunk or will we go on with 
discussion what can be ommited as well?
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1526903/+merge/281410
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1526903.

___
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-1526903 into lp:widelands

2015-12-30 Thread TiborB
>Fatal exception: Widelands could not start the game, because player 2 has no 
>starting position.
You can manually add a starting position with the Widelands Editor to fix this 
problem.

For me this is "unfinished map" not a "bug".
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1526903/+merge/281410
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1526903.

___
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-1526903 into lp:widelands

2015-12-30 Thread GunChleoc
Could you go through the map saver and have a look at other packets that we 
don't need?
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1526903/+merge/281410
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1526903 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-1526903 into lp:widelands

2015-12-29 Thread TiborB
TiborB has proposed merging lp:~widelands-dev/widelands/bug-1526903 into 
lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1526514 in widelands: "player 2 in "" section [player_2] not found"
  https://bugs.launchpad.net/widelands/+bug/1526514
  Bug #1526903 in widelands: "Editor crashes when changing tribe"
  https://bugs.launchpad.net/widelands/+bug/1526903

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1526903/+merge/281410

This is very simple change - prohibiting save of allowed_building_types in 
editor (=creating particular file). Should not cause problems, but must be 
tested
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1526903 into lp:widelands.
=== modified file 'src/map_io/map_saver.cc'
--- src/map_io/map_saver.cc	2015-11-11 09:53:54 +
+++ src/map_io/map_saver.cc	2015-12-29 22:26:03 +
@@ -139,18 +139,22 @@
 	{MapAllowedWorkerTypesPacket p; p.write(m_fs, m_egbase, *m_mos);}
 	log("took %ums\n ", timer.ms_since_last_query());
 
- //  allowed building types
-	iterate_players_existing_const(plnum, nr_players, m_egbase, player) {
-		for (DescriptionIndex i = 0; i < m_egbase.tribes().nrbuildings(); ++i) {
-			if (!player->is_building_type_allowed(i)) {
-log("Writing Allowed Building Types Data ... ");
-MapAllowedBuildingTypesPacket p;
-p  .write(m_fs, m_egbase, *m_mos);
-log("took %ums\n ", timer.ms_since_last_query());
-goto end_find_a_forbidden_building_type_loop;
+//  allowed building types
+// Not saving this when in editor - it causes issues when changing tribe
+// after next load in editor
+	if (is_a(Game, _egbase)) {
+		iterate_players_existing_const(plnum, nr_players, m_egbase, player) {
+			for (DescriptionIndex i = 0; i < m_egbase.tribes().nrbuildings(); ++i) {
+if (!player->is_building_type_allowed(i)) {
+	log("Writing Allowed Building Types Data ... ");
+	MapAllowedBuildingTypesPacket p;
+	p  .write(m_fs, m_egbase, *m_mos);
+	log("took %ums\n ", timer.ms_since_last_query());
+	goto end_find_a_forbidden_building_type_loop;
+}
 			}
-		}
-	} end_find_a_forbidden_building_type_loop:;
+		} end_find_a_forbidden_building_type_loop:;
+	}
 
 	// !! NOTE
 	// This packet must be before any building or road packet. So do not

___
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-1526903 into lp:widelands

2015-12-29 Thread TiborB
The diff looks bit complex, but really only thing that was added is:
if (is_a(Game, _egbase)) {  }
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1526903/+merge/281410
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1526903 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