[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
Continuous integration builds have changed state: Travis build 3850. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/421503400. Appveyor build 3648. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1783878_editor_random_map_tribe-3648. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
Continuous integration builds have changed state: Travis build 3841. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/420890791. Appveyor build 3639. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1783878_editor_random_map_tribe-3639. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways. Travis build 3841. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/420890791. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
@ypopezios: Good idea, but not as easy as it sounds - when set to "random", the
tribe is already sneakily assigned in the background so that the user can
choose an initialization ("Headquarters") etc. So, removing that extra variable
has a lot of other implications in the code as well and is not trivial at all.
So, definitely not for Build 20 ;)
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe.
___
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-1783878_editor_random_map_tribe into lp:widelands
Review: Approve compile, review, test, Works for me, I cannot judge mutch abouthe the code. ypopezios: you are correct, but lets get rid of the bug first and do the improvement in some second step. Gun: will we need a resubmit before wer can merge this? @bonnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
Added inline comment.
Diff comments:
>
> === modified file 'src/ui_fsmenu/launch_spg.cc'
> --- src/ui_fsmenu/launch_spg.cc 2018-07-07 20:22:12 +
> +++ src/ui_fsmenu/launch_spg.cc 2018-08-26 11:35:09 +
> @@ -283,7 +283,14 @@
> Widelands::PlayerNumber const nrplayers = map.get_nrplayers();
> for (uint8_t i = 0; i < nrplayers; ++i) {
> settings_->set_player_name(i, map.get_scenario_player_name(i +
> 1));
> - settings_->set_player_tribe(i, map.get_scenario_player_tribe(i
> + 1));
> + const std::string playertribe = map.get_scenario_player_tribe(i
> + 1);
> + if (playertribe.empty()) {
> + // Set tribe selection to random
> + settings_->set_player_tribe(i, "", true);
I would clean-up the interface of set_player_tribe() through-out the codebase
to remove the flag for random_tribe and use the empty-string convention
instead, then check for the string's emptiness inside set_player_tribe() .
> + } else {
> + // Set tribe selection from map
> + settings_->set_player_tribe(i, playertribe);
> + }
> }
> }
>
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe.
___
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-1783878_editor_random_map_tribe into lp:widelands
Changes are done. Could you please retest to make sure that I haven't missed anything? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
Review: Needs Fixing I like the proposed change. However, when starting the editor and clicking the "Players" button I get an exception: widelands: ../src/logic/map.cc:411: const string& Widelands::Map::get_scenario_player_tribe(Widelands::PlayerNumber) const: Assertion `p <= get_nrplayers()' failed. Problem is the change in diff line 81, where p can be greater than the number of active players. Since the editor-user has the option to do so now anyway: Select the "random" tribe in line 42 of the diff? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
Review: Resubmit Actually, the players loaded into the editor were never used, so I kicked them out. I changed the semantics so that an empty tribe means "pick at random." -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into lp:widelands has been updated. Commit message changed to: Empty player tribes are interpreted as random tribe. Random Map Generator now assigns a random tribe to all players. - Fix crash when loading a map in the editor where there is a player with no tribe assigned: player loading removed, because it's unused in the editor anyway - Map Editors now can choose "Random" tribe in the player menu - If player tribe is empty in map, set random player in singleplayer game setup For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
Review: Approve
Code is looking okay and the editor no longer crashes. Two observations:
- When loading the old/broken maps, we still get the "Tribe not known" message.
Maybe only display a "note:" message and select a default tribe in that case?
Having the wrong tribe is in most cases probably preferable to not being able
to use the map at all. Unknown tribes in maps could also happen when
"backporting" maps from (newer) game versions where more/custom tribes are
available.
- When loading the "broken" map in the editor and saving again the map is still
broken. This one is actually a bit strange, shouldn't the change in
editorinteractive.cc avoid this as well?
Diff comments:
> === modified file 'src/editor/editorinteractive.cc'
> --- src/editor/editorinteractive.cc 2018-07-20 08:42:23 +
> +++ src/editor/editorinteractive.cc 2018-08-12 17:17:16 +
> @@ -180,7 +180,7 @@
> loader_ui.step(_("Creating players"));
> iterate_player_numbers(p, map->get_nrplayers()) {
> egbase().add_player(
> -p, 0, map->get_scenario_player_tribe(p),
> map->get_scenario_player_name(p));
> +p, 0, map->get_scenario_player_tribe(p).empty() ?
> Widelands::get_all_tribenames().front() : map->get_scenario_player_tribe(p),
> map->get_scenario_player_name(p));
Why no random selection here?
> }
>
> ml->load_map_complete(egbase(),
> Widelands::MapLoader::LoadType::kEditor);
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe.
___
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-1783878_editor_random_map_tribe into lp:widelands
Continuous integration builds have changed state: Travis build 3777. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/415165379. Appveyor build 3576. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1783878_editor_random_map_tribe-3576. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe 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-1783878_editor_random_map_tribe into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into
lp:widelands.
Commit message:
Random Map Generator now assigns a random tribe to all players. Fix crash when
loading a map in the editor where there is a player with no tribe assigned.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1783878 in widelands: "Editor: Saved random map does segfault on load if
no tribe is explicitly set"
https://bugs.launchpad.net/widelands/+bug/1783878
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352943
I decided not to bother with any UI changes at this point - we have more
important bugs to fix for Build 20.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into
lp:widelands.
=== modified file 'src/editor/editorinteractive.cc'
--- src/editor/editorinteractive.cc 2018-07-20 08:42:23 +
+++ src/editor/editorinteractive.cc 2018-08-12 17:17:16 +
@@ -180,7 +180,7 @@
loader_ui.step(_("Creating players"));
iterate_player_numbers(p, map->get_nrplayers()) {
egbase().add_player(
- p, 0, map->get_scenario_player_tribe(p), map->get_scenario_player_name(p));
+ p, 0, map->get_scenario_player_tribe(p).empty() ? Widelands::get_all_tribenames().front() : map->get_scenario_player_tribe(p), map->get_scenario_player_name(p));
}
ml->load_map_complete(egbase(), Widelands::MapLoader::LoadType::kEditor);
=== modified file 'src/editor/map_generator.cc'
--- src/editor/map_generator.cc 2018-07-21 07:53:42 +
+++ src/editor/map_generator.cc 2018-08-12 17:17:16 +
@@ -28,6 +28,7 @@
#include "logic/editor_game_base.h"
#include "logic/findnode.h"
#include "logic/map.h"
+#include "logic/map_objects/tribes/tribe_basic_info.h"
#include "logic/map_objects/world/map_gen.h"
#include "logic/map_objects/world/world.h"
#include "scripting/lua_interface.h"
@@ -661,7 +662,6 @@
// Care about players and place their start positions
map_.set_nrplayers(map_info_.numPlayers);
assert(map_info_.numPlayers >= 1);
- const std::string tribe = map_.get_scenario_player_tribe(1);
const std::string ai = map_.get_scenario_player_ai(1);
FindNodeSize functor(FindNodeSize::sizeBig);
Coords playerstart(Coords::null());
@@ -714,6 +714,7 @@
for (PlayerNumber n = 1; n <= map_info_.numPlayers; ++n) {
// Set scenario information - needed even if it's not a scenario
map_.set_scenario_player_name(n, _("Random Player"));
+ const std::string tribe = get_all_tribenames()[rng.rand() % get_all_tribenames().size()];
map_.set_scenario_player_tribe(n, tribe);
map_.set_scenario_player_ai(n, ai);
map_.set_scenario_player_closeable(n, false);
___
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-1783878_editor_random_map_tribe into lp:widelands
Thanks GunChleoc. I'll delete this branch then. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
I think it makes most sense if I'm the one who takes this one on - the game setup screens are in transition code-wise, so it's hard for somebody who doesn't know that code to deal with it. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
My suggestion has been based solely on general programming principles, not on reading the code. I'm not familiar with the relevant files. I had a look and they are a mess (as most of the codebase is). If someone points me to specific places in the code, maybe I could do it. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
ypopecios, can you please make the needed changes? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
Random tribe is a separate variable in the game setup screens. I think ypopezios has the right idea to keep the implementation simple. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
I would not create a special extra tribe. I would instead use a nullptr as a default value and have it represent the random tribe by convention and only where it makes sense. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
As ever: The idea might be good, but implementing is quite difficult.
I think a special "Random Tribe" needs only the name, descname and an icon,
nothing more. So i have tried to add a "Random Tribe" through lua, but it fails
because the code for tribe initializing depends on so much other stuff. In
result we may need exceptions in many places in form of
if (tribe != "random_tribe"){
// Do stuff for normal tribes
}
Maybe there could be another way of implementing the idea?
For build20 this isn't worth the work, imho. And i am feeling not able to do so.
So lets stick with the bug fix proposed here and reschedule a "Random tribe"
for another day?
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe.
___
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-1783878_editor_random_map_tribe into lp:widelands
Sounds good - we could have that both for the random map generator and the player menu. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
Since one could also choose "Random tribe" in the Game setup menu, regardless of the maps setting, i am thinking about this: Maybe we can have a special tribe called "Random Tribe". So a map designer can choose "Random tribe" beside the others in the editor player menu for a normal map. "Random Tribe" could then be set as default for a tribe in the random map generator. If a map has a "Random Tribe", this can also be the preset for the game setup menu. I think this would be the cleaner solution. What do you think? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
Review: Approve Looks good to me - the maps will be more interesting anyway than if we were assigning Barbarians to everybody. Not tested. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe. ___ 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-1783878_editor_random_map_tribe into lp:widelands
After sleeping a night i am not sure if this is the correct approach to fix the bug ... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe 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-1783878_editor_random_map_tribe into lp:widelands
Continuous integration builds have changed state: Travis build 3744. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/410949945. Appveyor build 3544. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1783878_editor_random_map_tribe-3544. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe 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-1783878_editor_random_map_tribe into lp:widelands
kaputtnik has proposed merging
lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into
lp:widelands.
Commit message:
Set random tribes when using the random map generator
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1783878 in widelands: "Editor: Saved random map does segfault on load if
no tribe is explicitly set"
https://bugs.launchpad.net/widelands/+bug/1783878
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038
Fixes a bug in random map generator -> explicitly set a tribe to each player.
The tribe is set randomly.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into
lp:widelands.
=== modified file 'src/editor/map_generator.cc'
--- src/editor/map_generator.cc 2018-07-21 07:53:42 +
+++ src/editor/map_generator.cc 2018-08-01 18:31:33 +
@@ -28,6 +28,7 @@
#include "logic/editor_game_base.h"
#include "logic/findnode.h"
#include "logic/map.h"
+#include "logic/map_objects/tribes/tribe_basic_info.h"
#include "logic/map_objects/world/map_gen.h"
#include "logic/map_objects/world/world.h"
#include "scripting/lua_interface.h"
@@ -661,7 +662,6 @@
// Care about players and place their start positions
map_.set_nrplayers(map_info_.numPlayers);
assert(map_info_.numPlayers >= 1);
- const std::string tribe = map_.get_scenario_player_tribe(1);
const std::string ai = map_.get_scenario_player_ai(1);
FindNodeSize functor(FindNodeSize::sizeBig);
Coords playerstart(Coords::null());
@@ -714,6 +714,7 @@
for (PlayerNumber n = 1; n <= map_info_.numPlayers; ++n) {
// Set scenario information - needed even if it's not a scenario
map_.set_scenario_player_name(n, _("Random Player"));
+ const std::string tribe = get_all_tribenames()[rng.rand() % get_all_tribenames().size()];
map_.set_scenario_player_tribe(n, tribe);
map_.set_scenario_player_ai(n, ai);
map_.set_scenario_player_closeable(n, false);
___
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
