Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1526903 into lp:widelands
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
> 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
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
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
> 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
>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
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
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
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