Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refactor_gamehost into lp:widelands
Internat game trunk as host this branch joining, I get an assert about some missing image, too. No idea where this cames from. Maybe today I can throw some more time at the Problem. -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gamehost/+merge/370320 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/refactor_gamehost. ___ 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/refactor_gamehost into lp:widelands
Review: Needs Fixing Looks like I must check this with a debugger, get his wheh joining a netwok game in trunk Assertion failed: (clients_.at(id).state_ == Client::State::kConnected), function try_receive, file ../src/network/nethostproxy.cc, line 76. -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gamehost/+merge/370320 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/refactor_gamehost. ___ 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/compiler-warnings-201908-2 into lp:widelands
Review: Approve local tests ok -- https://code.launchpad.net/~widelands-dev/widelands/compiler-warnings-201908-2/+merge/372111 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/compiler-warnings-201908-2. ___ 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/compiler-warnings-201908-2 into lp:widelands
Mhh,one test in travis fails test/maps/plain.wmf/scripting/test_ui.lua ... Running Widelands ... FAIL But only for GCC_VERSION="4.9" BUILD_TYPE="Debug" ? will run the test loocally, too -- https://code.launchpad.net/~widelands-dev/widelands/compiler-warnings-201908-2/+merge/372111 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/compiler-warnings-201908-2. ___ 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/compiler-warnings-201908-2 into lp:widelands
Review: Approve Locally commpiles without issues. Start upto Lobby was fine. -- https://code.launchpad.net/~widelands-dev/widelands/compiler-warnings-201908-2/+merge/372111 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/compiler-warnings-201908-2. ___ 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/compiler-warnings-201908-2 into lp:widelands
Review: Approve * Replaced: - widelands_ball_of_mud + widelands_options -> fine * Codestyle: using this for a member function makes no sense. * Smuggeled in some USE_XDG handling :-) Will compile and start this, not doing much testing. Please: review my refactor_gamehost before merging it becomes to much pain. -- https://code.launchpad.net/~widelands-dev/widelands/compiler-warnings-201908-2/+merge/372111 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/compiler-warnings-201908-2. ___ 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/fix-cmakelists-codecheck into lp:widelands
Mh, compile runs without any issues. But how can I detect that utils/build_deps.py was executed? So I dont know how to approve this. -- https://code.launchpad.net/~widelands-dev/widelands/fix-cmakelists-codecheck/+merge/372114 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/fix-cmakelists-codecheck 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/compiler-warnings-201908 into lp:widelands
Review: Approve Compiled withouth any noteworthy complaints @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/compiler-warnings-201908/+merge/372109 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/compiler-warnings-201908. ___ 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/compiler-warnings-201908 into lp:widelands
Review: Approve code review Nothing to really check here. I will it compile it anyway -- https://code.launchpad.net/~widelands-dev/widelands/compiler-warnings-201908/+merge/372109 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/compiler-warnings-201908. ___ 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/new-shipping into lp:widelands
I now got Assertion failed: (ship.get_nritems() == 0), function ship_arrived, file ../src/economy/portdock.cc, line 383. after ./widelands --loadgame=~/Downloads/ShipCrash5.wgf Somwhat after 1:56... from https://www.magentacloud.de/share/tu4ayusx.k Do frisians have very special wares? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Review: Needs Fixing OK I have a ShipCrash4.wgf at https://www.magentacloud.de/share/tu4ayusx.k I start this with $ ./widelands --loadgame=~/Downloads/ShipCrash4.wgf and wait about 10 Minutes for Assertion failed: (detour >= base_length), function check_push_destination, file ../src/economy/fleet.cc, line 888. Abort trap: 6 Please consider that I have soldiers that need transport and have special wishes from Laybrinths and Dungeons. Sorrry for all thee detours :-) -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Now I was kiled here: Assertion failed: (detour >= base_length), function check_push_destination, file ../src/economy/fleet.cc, line 887. 5 widelands 0x0001102aedf9 Widelands::Fleet::check_push_destination(Widelands::Game&, Widelands::Ship&, Widelands::PortDock const&, Widelands::PortDock&, unsigned int) + 1497 (fleet.cc:887) 6 widelands 0x0001102ad69e Widelands::Fleet::push_next_destinations(Widelands::Game&, Widelands::Ship&, Widelands::PortDock const&) + 3294 (fleet.cc:843) 7 widelands 0x0001102dd544 Widelands::PortDock::ship_arrived(Widelands::Game&, Widelands::Ship&) + 2260 (portdock.cc:378) 8 widelands 0x00010fbb1cbe Widelands::Ship::ship_update_transport(Widelands::Game&, Widelands::Bob::State&) + 1166 (ship.cc:316) 9 widelands 0x00010fbb08d9 Widelands::Ship::ship_update(Widelands::Game&, Widelands::Bob::State&) + 1033 (ship.cc:265) 10 widelands 0x00010f8ea790 Widelands::Bob::do_act(Widelands::Game&) + 704 (bob.cc:196) 11 widelands 0x00010f8ea486 Widelands::Bob::act(Widelands::Game&, unsigned int) + 1030 (bob.cc:181) 12 widelands 0x00010f98d88a Widelands::CmdAct::execute(Widelands::Game&) + 570 (map_object.cc:110) 13 widelands 0x0001103d42f9 Widelands::CmdQueue::run_queue(int, unsigned int&) + 1497 (cmd_queue.cc:124) 14 widelands 0x00010e3c9b4d Widelands::Game::think() + 637 (game.cc:602) 15 widelands 0x00010f3a53c8 InteractiveBase::think() + 488 (interactive_base.cc:516) 16 widelands 0x00010f4ae6bc InteractivePlayer::think() + 572 (interactive_player.cc:232) 17 widelands 0x00010ea040e7 UI::Panel::do_think() + 151 (panel.cc:488) 18 widelands 0x00010ea0230c UI::Panel::do_run() + 2332 (panel.cc:189) ... Will try to reprodcue this ... -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
OK, I can load the Savegame again, lets continue hunting ... -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Next one please check: https://www.magentacloud.de/share/tu4ayusx.k
ShioCrash2.wgf
Crashes here:
Assertion failed: (old_index < nr_dests), function reorder_destinations,
file ../src/logic/map_objects/tribes/ship.cc, line 902.
With a Debugger I found...
Via the ReplayWriter game.cleanup_for_load(); is called
This in turn calls
void MapObject::remove(EditorGameBase& egbase)
void PortDock::cleanup(EditorGameBase& egbase) // why doing such a fuzz for a
cleanup?
void Ship::pop_destination(Game& game, PortDock& pd)
void Ship::reorder_destinations(Game& game)
I assume there is some issue with the == Operator from OPtr
optr(pair.first);
if (old_destinations[old_index].first == optr) {
break; // does not happen
}
This code does not really seem robust, but I still must find out what this OPtr
thing is.
Maybe when loading Objects ar kind of cloned by the loading so the code fails?
--
https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/new-shipping.
___
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/new-shipping into lp:widelands
OK, found an obvious Bug: * Two ships find the same PortSpace * Tell both of them to build a port. 4: Colonization error: unloading wares to constructionsite of atlanteans_port (owner 4) failed. Wares unloaded to the site: 3, max capacity: 3, remaining to unload: 13 No free capacity to unload another ware Assertion failed: (wq.get_max_fill() > cur), function ship_update_idle, file ../src/logic/map_objects/tribes/ship.cc, line 653. The same might happen if you tell a ship to uild a port that is already (partially) equipped from the Land side Do you want a save game for tis? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Hello benedikt, I am back and will try more with a debugger attached, any news from your siede? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Mhh, it took about 15-30 minutes to trigger the asserts. I will be "on call duty" this week, so I have even less time. I tried one of the new features of trunk : order Soldiers to be sent to the port immediateley once it was build. As of some Towers "across the Sea", I was able to plan buldings next to the Port while it was build. And I connected some Ports being build to some exiting economy. I will try some AI-only setups (Well he AI fails to grasp the evil tricks needed for this Map..) I will give this some 4 hours (on 10x Speed or such). Next I will setup a debugger and give you details from "inside the code". What OS/CPU/Debugger do you use? Maybe we should recruit some tester from the tournmanent? -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Review: Needs Fixing Debugging can be FUN!, well not for you Benedikt, sorry: 5: Ullr: exploration - breaking for free sea, dir=1 TrainingSite::drop_stalled_soldiers: Kicking somebody out. Assertion failed: (detour >= base_length), function check_push_destination, file ../src/economy/fleet.cc, line 887. Abort trap: 6 Application Specific Information: Assertion failed: (old_index < nr_dests), function reorder_destinations, file ../src/logic/map_objects/tribes/ship.cc, line 902. I added two more Savegames +/- around the asserts from above https://www.magentacloud.de/share/tu4ayusx.k The _may_ be related to shipping soldiers. I "forgot" some Soldiers on the Mainland and after sending them awy I got theses assertions. Maybe adding some coordinates and the exact values to the assert may help? Ill go back to fine tuning my map ... -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Review: Needs Fixing OK here comes thee next (me conquering the Imperial Isaland East of mine) 5: (126x 23) expedition spot scoring, colony_scan_area == 21 5: Ullr at 2x 23: PORTSPACE found, we valued it: 0 5: Ullr at 2x 23: explore uphold, visited first time 5: Ullr: continue island circumvention, dir=1 Cmd_EnemyFlagAction::execute player(3): flag->owner(2) number=4 Assertion failed: (old_index < nr_dests), function reorder_destinations, file ../src/logic/map_objects/tribes/ship.cc, line 902. Abort trap: 6 Loos like this logic needs more Braintwisting :-) But now I really _will_ stop... good night -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Look like a) The attached game on partially reproduces the Issue (just slow as hell) b) A Millitarysite near a Port was under heavy Attack, you ca find two barbarian Inhabitat Atlanter Buidlign, ooks I lost them just during the crash. I set Autosae to Minutes ant will continue ... later. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
OK, now its getting interesting. My "Unreal Water" Map got totally slow and stopped with 1: attacking site at 56x 40, score 5, with 2 soldiers, attacking 2 times, after 102 seconds Cmd_EnemyFlagAction::execute player(1): flag->owner(3) number=2 Cmd_EnemyFlagAction::execute player(3): flag->owner(2) number=3 TI(55348): destination disappeared or economy mismatch -> fail Assertion failed: (dest), function load_wares, file ../src/economy/portdock.cc, line 395. Abort trap: 6 Lets see, if I can get some savegame ... Check here; https://www.magentacloud.de/share/tu4ayusx.k UnrealAssert.wgf -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Just started the Debugger and wondered where this this OPtr thing comes from. But whatever you did looks like you fixed it. Will go on laying my Map now until I got whatever was possible. AND thanks! -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
I added the complete output to: https://www.magentacloud.de/share/tu4ayusx.k#$/ Crash.txt Will try a debugger now -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Sorry still got it SaveHandler::save_game() took 2536ms Reloading the game from replay = ==99248==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000c71568 at pc 0x0001102296de bp 0x7ffee2224ef0 sp 0x7ffee2224ee8 #2 0x110214b23 in Widelands::PortDock::cleanup(Widelands::EditorGameBase&) portdock.cc:200 #3 0x10f8c9822 in Widelands::MapObject::remove(Widelands::EditorGameBase&) map_object.cc:465 #4 0x10fc7d67c in Widelands::Warehouse::cleanup(Widelands::EditorGameBase&) warehouse.cc:638 #5 0x10f8c9822 in Widelands::MapObject::remove(Widelands::EditorGameBase&) map_object.cc:465 #6 0x10f8c9436 in Widelands::ObjectManager::cleanup(Widelands::EditorGameBase&) map_object.cc:167 #7 0x10db4f8f4 in Widelands::EditorGameBase::cleanup_objects() editor_game_base.h:170 #8 0x10e2d1b08 in Widelands::EditorGameBase::cleanup_for_load() editor_game_base.cc:510 #9 0x10e304d5f in Widelands::Game::cleanup_for_load() game.cc:615 #10 0x10e3eee66 in Widelands::ReplayWriter::ReplayWriter(Widelands::Game&, std::__1::basic_string, std::__1::alloca freed by thread T0 here: ... std::__1::__tree_node, void*>*, long>) set:647 #6 0x11021740f in Widelands::PortDock::ship_coming(Widelands::Ship&, bool) portdock.cc:336 #7 0x10fafac3b in Widelands::Ship::pop_destination(Widelands::Game&, Widelands::PortDock&) ship.cc:777 No idea what this replay writer does, no time for debugging right now -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Mhh, I have: Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -- works Apple LLVM version 10.0.1 (clang-1001.0.46.4) And I get still get this crash at bzr9140[new-shipping] directy when loading that file. We had such subtule differences between compilers before. ==1286==ERROR: AddressSanitizer: heap-use-after-free on address 0x604001876498 at pc 0x00010f0ed78e bp 0x7ffee3233ef0 sp 0x7ffee3233ee8 #2 0x10f0d88cc in Widelands::PortDock::cleanup(Widelands::EditorGameBase&) portdock.cc:200 Hmm, trying a Debugger, next. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Whatever you did, looks like it fixed the regression tests, too. I have about no time this weeek. But this deserves some playtesting, mmh. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Loading the Savegame Unreal.wgf from https://www.magentacloud.de/share/tu4ayusx.k provokes this ASAN error in ==1418==ERROR: AddressSanitizer: heap-use-after-free on address 0x6040025b5898 at pc 0x00010b77478e ... #2 0x10b75f8cc in Widelands::PortDock::cleanup(Widelands::EditorGameBase&) portdock.cc:200 please confirm you can reproduce it and can fix the regression tests. new code looks fine. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
> be carefull not to use (local) timings ...
For some rendering code you might use an approach
long ts = current_time() + kMaxRenderingMillis;
do {
refine_textures(...)
} while (ts < current_time());
With widelands we must not do such things, got the idea?
--
https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/new-shipping.
___
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/new-shipping into lp:widelands
Review: Needs Fixing regression tests We have a lot of regressiontests about seafaring (for a reason :-) ./regression_test.py -b ./widelands FAILs test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_worker_in_transit.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_first_port_with_worker_in_portdock.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_first_port_with_ware_in_portdock.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_second_port_with_ware_in_portdock.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_farm_with_ware_and_worker_in_transit.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_ware_in_transit.lua ... test/maps/ship_transportation.wmf/scripting/test_rip_second_port_with_worker_in_portdock.lua ... One hard crash from ASAN with: ==14492==ERROR: AddressSanitizer: heap-use-after-free on address 0x6040020e6798 at pc 0x000107e3088e bp 0x7ffeea4f2790 sp 0x7ffeea4f2788 READ of size 8 at 0x6040020e6798 thread T0 #0 0x107e3088d in std::__1::__tree_end_node*>* std::__1::__tree_next_iter*>*, std::__1::__tree_node_base*>(std::__1::__tree_node_base*) __tree:176 #1 0x107e1c53a in std::__1::__tree_const_iterator*, long>::operator++() __tree:920 #2 0x107e1bc2c in Widelands::PortDock::cleanup(Widelands::EditorGameBase&) portdock.cc:200 #3 0x1074fcef2 in Widelands::MapObject::remove(Widelands::EditorGameBase&) map_object.cc:465 #4 0x1078b2d7c in Widelands::Warehouse::cleanup(Widelands::EditorGameBase&) warehouse.cc:638 ... previously allocated by thread T0 here: ... #4 0x107e3340f in std::__1::unique_ptr, std::__1::__tree_node_destructor > > > std::__1::__tree, std::__1::allocator >::__construct_node(Widelands::Ship*&&) __tree:2201 #5 0x107e32bfc in std::__1::pair*, long>, bool> std::__1::__tree, std::__1::allocator >::__emplace_unique_key_args(Widelands::Ship* const&, Widelands::Ship*&&) __tree:2147 #6 0x107e327ab in std::__1::__tree, std::__1::allocator >::__insert_unique(Widelands::Ship*&&) __tree:1276 #7 0x107e1eb3f in std::__1::set, std::__1::allocator >::insert(Widelands::Ship*&&) set:635 #8 0x107e1e3af in Widelands::PortDock::ship_coming(Widelands::Ship&, bool) portdock.cc:330 #9 0x10772d84b in Widelands::Ship::push_destination(Widelands::Game&, Widelands::PortDock&) ship.cc:763 #10 0x107df0509 in Widelands::Fleet::push_next_destinations(Widelands::Game&, Widelands::Ship&, Widelands::PortDock const&) fleet.cc:872 #11 0x107e1fa50 in Widelands::PortDock::ship_arrived(Widelands::Game&, Widelands::Ship&) portdock.cc:374 #12 0x10772239d in Widelands::Ship::ship_update_transport(Widelands::Game&, Widelands::Bob::State&) ship.cc:315 #13 0x107720fb8 in Widelands::Ship::ship_update(Widelands::Game&, Widelands::Bob::State&) ship.cc:265 #14 0x107458bff in Widelands::Bob::do_act(Widelands::Game&) bob.cc:194 Caused by FAIL: test/maps/ship_transportation.wmf/scripting/test_rip_portdock_with_worker_and_ware_in_transit.lua Please tell me how I may help you -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-shipping. ___ 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/new-shipping into lp:widelands
Did a first review, the code is ok, but some more comments
would help me to grasp me the semantics of some stuctures.
and please refactor out some of the bigger loops.
(https://de.wikipedia.org/wiki/Refactoring)
Please check my inline comments.
Do you have an idea how big the perfomance impact may be for a Map with a lot
of Ports.
e.g. two AI Players on the Nile or my new Map
https://www.widelands.org/maps/unreal-waterflow/
You are solving a kind of
https://de.wikipedia.org/wiki/Problem_des_Handlungsreisenden
or https://en.wikipedia.org/wiki/Travelling_salesman_problem
this has potential for exponential (or NP-Hard) complexity. So maybe we should
put in some limits so that we dod not stop the game dunring these calculations.
e.g.
* limit the number of ports on a route to 4/8/16?
* stop some calcualtions after a giiven mount of steps?
* be carefull not to use (local) timings as this depends on the CPU and may
rsult in desyncs
Next I will run the regression tests.
Maybe its worth to design a big Map with a lot of Ports (128) to test this?
Diff comments:
> === modified file 'src/economy/fleet.cc'
> --- src/economy/fleet.cc 2019-04-24 15:11:23 +
> +++ src/economy/fleet.cc 2019-08-10 17:23:31 +
> @@ -699,6 +700,46 @@
> if (route_length == kRouteNotCalculated) {
> route_length = s->calculate_sea_route(game, *p);
> }
> + if (fallback) {
> + // This ship is employed transporting wares,
> lower its priority drastically
Please make this a function of its own add some more coments
> + const uint32_t real_length = route_length;
> + uint32_t malus = 1;
> + uint32_t index = 0;
As wee need an index anyway a "classic" loop would avoid inconsistencies when
we add continue statements later?
> + PortDock* iterator = nullptr;
> + uint32_t shortest_detour =
> std::numeric_limits::max();
> + uint32_t best_index;
> + for (const auto& pair : s->destinations_) {
> + PortDock* pd = pair.first.get(game);
> + Path path;
> + uint32_t detour;
> + if (iterator == p) {
> + detour = 0;
> + } else if (iterator) {
> + get_path(*iterator, *p, path);
> + detour = path.get_nsteps();
> + } else {
> + s->calculate_sea_route(game,
> *p, );
> + detour = path.get_nsteps();
> + }
> + if (p != pd) {
> + get_path(*p, *pd, path);
> + detour += path.get_nsteps();
> + }
> + if (detour < shortest_detour) {
> + shortest_detour = detour;
> + best_index = index;
> + }
> + malus += pair.second;
> + iterator = pd;
> + ++index;
> + }
> + route_length += shortest_detour * best_index;
> + if (route_length + shortest_detour >
> real_length * malus) {
> + // Unreasonably long detour
> + continue;
> + }
> + route_length *= malus;
> + }
>
> if (route_length < shortest_dist) {
> shortest_dist = route_length;
> @@ -744,107 +785,93 @@
> }
>
> if (closest_port) {
> - s->set_destination(closest_port);
> + s->push_destination(game, *closest_port);
> s->send_signal(game, "wakeup");
> }
> }
> }
>
> /**
> - * For the given three consecutive ports, decide if their path is favourable
> or not.
> - * \return true if the path from start to finish >= the path from middle to
> finish
> - */
> -bool Fleet::is_path_favourable(const PortDock& start,
> - const PortDock& middle,
> - const PortDock& finish) {
> - if ( != ) {
> - Path path_start_to_finish;
> - Path
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/new-shipping into lp:widelands
Thanks for picking this up, as I am just teesting some new Mpa with a lot of Ports, I can combine two things here. I dont have that much time the next weeks, though. -- https://code.launchpad.net/~widelands-dev/widelands/new-shipping/+merge/371155 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/new-shipping 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/toolbar-dropdown-scripting-review-only into lp:widelands
Gun: now this got merged, what kind of accident was this? -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-scripting-review-only/+merge/368228 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only 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/toolbar-dropdown-menus into lp:widelands
Review: Approve comile, regression test Fine onn OSX now, too (release build) > Ran 45 tests in 1128.847s > OK @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only. ___ 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/toolbar-dropdown-menus into lp:widelands
Review: Approve compile, regression test Ahh, now you confused me :-) Just tried on Linux > Linux iXubuntu 5.0.0-23-generic #24-Ubuntu SMP Mon Jul 29 15:36:44 UTC 2019 > x86_64 x86_64 x86_64 GNU/Linux and suprise: al Regreession tests pass on this branch and trunk with release and debug OK Ill go back to OSX and try again -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only. ___ 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/toolbar-dropdown-menus into lp:widelands
Running tests locally I get (on OSX): FAIL: test_buttons_property: [string "scripting/ui.lua"]:26: 'nil' not expected! FAIL: test_name: [string "scripting/ui.lua"]:113: attempt to index a nil value (field 'b') stack traceback: [string "scripting/ui.lua"]:113: in upvalue 'func' [string "scripting/lunit.lua"]:425: in function <[string "scripting/lunit.lua"]:425> [C]: in global 'xpcall' [string "scripting/lunit.lua"]:425: in upvalue 'call' [string "scripting/lunit.lua"]:450: in local 'run' [string "scripting/lunit.lua"]:476: in global 'lunit_run_testcase' [string "scripting/lunit.lua"]:382: in method 'run' [string "scripting/init.lua"]:70: in main chunk FAIL: test_click: [string "scripting/ui.lua"]:116: attempt to index a nil value (field 'b') stack traceback: [string "scripting/ui.lua"]:116: in upvalue 'func' [string "scripting/lunit.lua"]:425: in function <[string "scripting/lunit.lua"]:425> [C]: in global 'xpcall' [string "scripting/lunit.lua"]:425: in upvalue 'call' [string "scripting/lunit.lua"]:450: in local 'run' [string "scripting/lunit.lua"]:476: in global 'lunit_run_testcase' [string "scripting/lunit.lua"]:382: in method 'run' [string "scripting/init.lua"]:70: in main chunk FAIL: test/maps/ship_transportation.wmf/scripting/test_rip_ship_before_picking_up_transporting_ware.lua ... Fatal exception: [../src/io/filesystem/layered_filesystem.cc:216] LayeredFileSystem: unable to create sub filesystem for existing directory: /var/folders/bx/q32c1w1965g5clyfbfdykcb4gp/T/widelands_regression_testfmUxCa/save/0_before_removing_ship.wgf I have tried the following path(s): /var/folders/bx/q32c1w1965g5clyfbfdykcb4gp/T/widelands_regression_testfmUxCa//var/folders/bx/q32c1w1965g5clyfbfdykcb4gp/T/widelands_regression_testfmUxCa/save/0_before_removing_ship.wgf /Users/klaus/develop/widelands-repo/toolbar-dropdown-menus//var/folders/bx/q32c1w1965 FATAL ERROR - game crashed. Attempting emergency save. lastserial: 0 Caught exception (of type '10WException') in outermost handler! The exception said: [../src/io/filesystem/layered_filesystem.cc:216] LayeredFileSystem: unable to create sub filesystem for existing directory: /var/folders/bx/q32c1w1965g5clyfbfdykcb4gp/T/widelands_regression_testfmUxCa/save/0_before_removing_ship.wgf I have tried the following path(s): /var/folders/bx/q32c1w1965g5clyfbfdykcb4gp/T/widelands_regression_testfmUxCa//var/folders/bx/q32c1w1965g5clyfbfdykcb4gp/T/widelands_regression_testfmUxCa/save/0_before_removing_ship.wgf /Users/klaus/develop/widelands-repo/toolbar-dropdown-menus//var/folders/bx/q32c1w1965 This should not happen. Please file a bug report on version bzr9123[toolbar-dropdown-menus](Release). and remember to specify your operating system. -- Ran 45 tests in 978.544s FAILED (failures=2) Gun: As you are "busy indexing nil values" you may have an idea what happend here ;-) WIll it help if I: a) Dig into the test and find what failed? b) run these tests on linux? I will no play a normala, single player game for a while and then try again with the debug build -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only. ___ 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/toolbar-dropdown-menus into lp:widelands
Review: Needs Fixing Mhh, some travis builds just time out, The Realse Builds are all broekn with a testcase. https://travis-ci.org/widelands/widelands/jobs/569957376 1383 Assertions checked. 405 Tests passed, 3 failed! Trying to run: test/maps/lua_testsuite.wmf/scripting/test_lua_in_game.lua: done this would indicate some asserts containing vital code which is now mssing in the release build? I will try to do a release build but as I am on OSX this should be ok. So we have an assertion with vital code that is still executed on OSX, odd. So I will try a realse buid oon linux, too. That will take a little longer. -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only. ___ 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/toolbar-dropdown-menus into lp:widelands
Review: Approve II creeated some Map now using this brnach and had no issues whatsover. The tools wok ass designed, alas the User-Interaction is sometimes not clear. Maybe we can play the nw mpa with this version and/or trunk. -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only. ___ 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/toolbar-dropdown-menus into lp:widelands
Hmm, PAUSE as Menu daas not work for me (single Player Network game) -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only. ___ 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/toolbar-dropdown-menus into lp:widelands
I always assumed all the windows in the editor are exclsuive. But we can open all of them a the same time. Well this works but just confused me. I assumed the different windows could affect each other in a bad way, bt was not able to produce any of this. I woill check out again and give int another try. -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only. ___ 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/arrow-keys-map-movement into lp:widelands
Review: Approve @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/arrow-keys-map-movement/+merge/370687 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/arrow-keys-map-movement. ___ 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/arrow-keys-map-movement into lp:widelands
Review: Approve compile testplay Works as designed -- https://code.launchpad.net/~widelands-dev/widelands/arrow-keys-map-movement/+merge/370687 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/arrow-keys-map-movement. ___ 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/arrow-keys-map-movement into lp:widelands
Code looks straing forward, wonnder that this was not implemented this way before. Will now copile and testplay this a litte. Gun: please take a look at my refactoring branch. -- https://code.launchpad.net/~widelands-dev/widelands/arrow-keys-map-movement/+merge/370687 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/arrow-keys-map-movement 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/refactor_gamehost into lp:widelands
Klaus Halfmann has proposed merging
lp:~widelands-dev/widelands/refactor_gamehost into lp:widelands.
Commit message:
Refactoring of gamehost.cc for better readability.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/refactor_gamehost/+merge/370320
Refactoring of gamehost.cc for better readability,
should be completly compatible with trunk.
Please contact me, so we can do some testing vs. trunk.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/refactor_gamehost into lp:widelands.
=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc 2019-05-26 11:39:41 +
+++ src/network/gamehost.cc 2019-07-18 14:48:27 +
@@ -903,7 +903,8 @@
}
/**
- * If the host sends a chat message with formation /kick
+ * If the host sends a chat message with formation /kick .
+ *
* This function will handle this command and try to kick the user.
*/
void GameHost::kick_user(uint32_t client, const std::string& reason) {
@@ -980,18 +981,22 @@
return false;
// if there is one client that is currently receiving a file, we can not launch.
+
+ std::vector = d->settings.users;
+
for (std::vector::iterator j = d->clients.begin(); j != d->clients.end(); ++j) {
- if (j->usernum == -1) {
+ int usernum = j->usernum;
+ if (usernum == -1) {
return false;
}
- if (!d->settings.users[j->usernum].ready) {
+ if (users[j->usernum].ready) {
return false;
}
}
// all players must be connected to a controller (human/ai) or be closed.
// but not all should be closed!
- bool one_not_closed = false;
+ bool one_not_closed = false; // TODO(k.halfmann): check this logic
for (PlayerSettings& setting : d->settings.players) {
if (setting.state != PlayerSettings::State::kClosed)
one_not_closed = true;
@@ -1011,14 +1016,15 @@
std::vector::size_type oldplayers = d->settings.players.size();
- SendPacket packet;
-
// Care about the host
if (static_cast(maxplayers) <= d->settings.playernum &&
d->settings.playernum != UserSettings::none()) {
set_player_number(UserSettings::none());
}
+ SendPacket packet;
+
+ // Drop players not matching map any longer
while (oldplayers > maxplayers) {
--oldplayers;
for (uint16_t i = 1; i < d->settings.users.size(); ++i)
@@ -1033,16 +1039,13 @@
d->clients.at(j).playernum = UserSettings::none();
// Broadcast change
-packet.reset();
-packet.unsigned_8(NETCMD_SETTING_USER);
-packet.unsigned_32(i);
-write_setting_user(packet, i);
-broadcast(packet);
+broadcast_setting_user(packet, d->clients.at(j).usernum);
}
}
d->settings.players.resize(maxplayers);
+ // Open slots for new players found on the map.
while (oldplayers < maxplayers) {
PlayerSettings& player = d->settings.players.at(oldplayers);
player.state = PlayerSettings::State::kOpen;
@@ -1110,9 +1113,12 @@
}
}
+// TODO(k.halfmann): refactor this
void GameHost::set_player_state(uint8_t const number,
PlayerSettings::State const state,
bool const host) {
+
+ // ignore player numbers out of range
if (number >= d->settings.players.size())
return;
@@ -1122,9 +1128,9 @@
return;
if (player.state == PlayerSettings::State::kHuman) {
- // 0 is host and has no client
- if (d->settings.users.at(0).position == number) {
- d->settings.users.at(0).position = UserSettings::none();
+ // kHostPlayerNum has no client
+ if (d->settings.users.at(kHostPlayerNum).position == number) {
+ d->settings.users.at(kHostPlayerNum).position = UserSettings::none();
d->settings.playernum = UserSettings::none();
}
for (uint8_t i = 1; i < d->settings.users.size(); ++i) {
@@ -1184,11 +1190,7 @@
// Broadcast change to player
SendPacket packet;
- packet.reset();
- packet.unsigned_8(NETCMD_SETTING_PLAYER);
- packet.unsigned_8(number);
- write_setting_player(packet, number);
- broadcast(packet);
+ broadcast_setting_player(packet, number);
// Let clients know whether their slot has changed
packet.reset();
@@ -1205,6 +1207,7 @@
PlayerSettings& player = d->settings.players.at(number);
+ // TODDO(k.halfmann): check this logic, will tribe "survive" een when random is selected?
if (player.tribe == tribe && player.random_tribe == random_tribe)
return;
@@ -1225,11 +1228,8 @@
// broadcast changes
SendPacket packet;
- packet.unsigned_8(NETCMD_SETTING_PLAYER);
- packet.unsigned_8(number);
- write_setting_player(packet, number);
- broadcast(packet);
- return;
+ broadcast_setting_player(packet, number);
+ return; // TODO(k.halfmann): check this logic
}
}
log("Player %u attempted to change to tribe %s; not a valid tribe\n",
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/toolbar-dropdown-menus into lp:widelands
Review: Needs Fixing Can we merge trunk? the savegameerro does not happen there? -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only. ___ 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/toolbar-dropdown-menus into lp:widelands
I played Turorail 2 for while and then saved it. When trying to load I get: lastserial: 677 Game data error buildingdata: building 524547: not found I will try this on trunk now, too -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only. ___ 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/toolbar-dropdown-menus into lp:widelands
Review: Needs Fixing play 1st tutorial Not nice, you open the tool popup toheeter with the tool-size window. The zoom Icons look a bit rough? Via the Editor-Main-Menu one can open all windows at the same time, this cries for all kinds of Errors. Tutorial did not wait until I actually used te "Show Buiding Spaces" menu Tutoral crashes with assert AND ASAN: ... Clicking tab 'small' Pressing button 'barbarians_lumberjacks_hut' Clicking button 'barbarians_lumberjacks_hut' Fatal exception: [../src/graphic/image_io.h:35] Image not found: images/wui/menus/menu_stock.png FATAL ERROR - game crashed. Attempting emergency save. Game: Writing Preload Data ... AddressSanitizer:DEADLYSIGNAL = ==18043==ERROR: AddressSanitizer: SEGV on unknown address 0x (pc 0x00010d04de4b bp 0x7ffee53a0910 sp 0x7ffee53a0460 T0) ==18043==The signal is caused by a READ memory access. ==18043==Hint: address points to the zero page. #0 0x10d04de4a in Widelands::GamePreloadPacket::write(FileSystem&, Widelands::Game&, Widelands::MapObjectSaver*) game_preload_packet.cc:114 #1 0x10d052b08 in Widelands::GameSaver::save() game_saver.cc:51 Looks like the emergency save stumbeld over already corrupt data? It think there is a lot to fix, aynthing more I can reasonably test? -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar-dropdown-scripting-review-only. ___ 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/toolbar-dropdown-menus into lp:widelands
Looks ike playing te tutorial is the best test? What are the names of Tutorial 1 & 4 ? -- https://code.launchpad.net/~widelands-dev/widelands/toolbar-dropdown-menus/+merge/368230 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/toolbar-dropdown-menus 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/fix-dropdowns into lp:widelands
Review: Needs Fixing testplay No I got a crash :( kaputtnik is right, we must play some network games on trunk, RSN. FATAL ERROR - game crashed. Attempting emergency save. ... InternetGaming: logout(SERVER_CRASHED) [NetClient] Closing network socket connected to 2a03:4000:32:524:48cb:feff:feae:b0c6:7395. Warning: Economy still has requests left on destruction Warning: Economy still has flags left on destruction Warning: Economy still has warehouses left on destruction WareList: 125 items of 30 left. WareList: 2 items of 33 left. ... WareList: 1 items of 102 left. Warning: Economy still has requests left on destruction Warning: Economy still has flags left on destruction Warning: Economy still has warehouses left on destruction WareList: 127 items of 0 left. ... WareList: 12 items of 102 left. ObjectManager: ouch! remaining objects lastserial: 2499 [NetRelayConnection] Closing network socket connected to 2a03:4000:32:524:48cb:feff:feae:b0c6:7397. InternetGaming: logout(SERVER_CRASHED) libc++abi.dylib: terminating with uncaught exception of type WException: [../src/logic/playercommand.cc:1070] Unreachable code was reached. Abort trap: 6 ObjectManager: ouch! remaining objects lastserial: 2499 [NetRelayConnection] Closing network socket connected to 2a03:4000:32:524:48cb:feff:feae:b0c6:7397. InternetGaming: logout(SERVER_CRASHED) I dont think this is related to tis particular branch, though. Will try this on trunk again, later -- https://code.launchpad.net/~widelands-dev/widelands/fix-dropdowns/+merge/368223 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix-dropdowns. ___ 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/fix-dropdowns into lp:widelands
Compiling again, got: src/wui/constructionsitewindow.cc:33:19: warning: unused variable 'pic_max_fill_indicator' static const char pic_max_fill_indicator[] = "images/wui/buildings/max_fill_indicator.png"; ^ src/wui/constructionsitewindow.cc:34:19: warning: unused variable 'pic_priority_low' static const char pic_priority_low[] = "images/wui/buildings/low_priority_button.png"; ^ src/wui/constructionsitewindow.cc:35:19: warning: unused variable 'pic_priority_normal' static const char pic_priority_normal[] = "images/wui/buildings/normal_priority_button.png"; ^ src/wui/constructionsitewindow.cc:36:19: warning: unused variable 'pic_priority_high' static const char pic_priority_high[] = "images/wui/buildings/high_priority_button.png"; Not sure where got that from? After merging with trunk I dont see the network Issue any longer, well. Did we just break savegame ccompatibility (yes with the elk_moose) OK, will start a new game for testing -- https://code.launchpad.net/~widelands-dev/widelands/fix-dropdowns/+merge/368223 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/fix-dropdowns 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/fix-dropdowns into lp:widelands
Played a bit and tested the MapEditor. So far all fine. The new WorkAreaIndicator (when selecting a building) is a pain, at least on my OSX-Computer. It is so slow, that selecting the correct building becomes quit difficult. Do we have an optin to disable this? Do we have a bug for this? This branch per se is fine, except for the Networking Issue. Will wait for other opinions for now. -- https://code.launchpad.net/~widelands-dev/widelands/fix-dropdowns/+merge/368223 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/fix-dropdowns 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/fix-dropdowns into lp:widelands
Found a unrealted, minor Bug: when leaving the "Set Player Positions" Window, the Map still shows only Big buidling Plots. Not sure if we _must_ fix this. Will go away with the next map commands. When trying to open an Online Game I get: InternetGaming: Connecting to the metaserver. [NetClient]: Trying to connect to 2a03:4000:32:524:48cb:feff:feae:b0c6:7395 ... failed. Could not resolve network name: resolve: Host not found (authoritative) > dig widelands.org widelands.org. 2431IN 2a03:4000:32:524:48cb:feff:feae:b0c6 > dig widelands.org A widelands.org. 3312IN A 85.235.66.69 ./widelands --metaserver=2a03:4000:32:524:48cb:feff:feae:b0c6 > OK ./widelands --metaserver=85.235.66.69 > OK How did this branch break the Internet (IPv6) connectivitiy? Can anybody confirm. I will continue testing with the hardcoded IPv6 Adress -- https://code.launchpad.net/~widelands-dev/widelands/fix-dropdowns/+merge/368223 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/fix-dropdowns 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/fix-dropdowns into lp:widelands
Most travis builds green, two fail as of test_inputqueues.lua Compiled. To test: * in MapEditor ** Widht/Height dropdowns (new Map) ** Widht/Height dropdowns (resize Map) ** Number of Players dropdown * Filter in news overview * Which examples of minimzed UniqueWindows can you give me? * How can I find the hotkeys and test them? * Improved SyntaxErrorImpl - no idea how to test this :-) Can I break som RichText in an easy way? * Sure we want to log every (Lua Button) keypress? Nice refactoring of boring code :-) Will no test the Lua part -> next Branch. -- https://code.launchpad.net/~widelands-dev/widelands/fix-dropdowns/+merge/368223 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/fix-dropdowns 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/elk_moose into lp:widelands
Travis: 1 x curl: (6) Could not resolve host: deb.debian.org Error: An exception occurred within a child process: DownloadError: Failed to download resource "isl" 3 x Loading savegame: inputqueues ... No output has been received in the last 10m0s -> Adressed elsewhere All other Builds are fine @bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/elk_moose/+merge/369201 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/elk_moose. ___ 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/elk_moose into lp:widelands
Review: Approve testing
Ahh, the json file is created in thee same dir as the wmf file, e.g.
{
"name": "CrossriverA",
"author": "Hasi50",
"description": "A nice River flows through this land and water can be found
there. Metal in the hills nearby, but you are not alone",
"hint": "",
"width": 64,
"height": 64,
"nr_players": 2,
"needs_widelands_version_after": 18,
"world_name": "",
"minimap": "/Users/klaus/.widelands/maps/My_Maps/CrossriverA1.wmf.png"
}
A map saved with the new version
{
"name": "27 zu 3 Inseln",
"author": "Björn alias the-x",
"description": "Starting with 27 Islands falling together when you find Ways
to pass through the rocks to 3 huge Island. Are you unstoppable to manage all
3 Islands under your control? Tipps: Start to find your dream Island real fast
and you wont find Iron on your main Island.",
"hint": "",
"width": 144,
"height": 144,
"nr_players": 4,
"needs_widelands_version_after": 20,
"world_name": "",
"minimap": "/Users/klaus/.widelands/maps/My_Maps/27 zu 3 Inselnx.wmf.png"
}
Trunk gives me:
Exception: map objects: [../src/logic/map_objects/world/critter.cc:323] loading
critter:
UnhandledVersionError: This game was saved using an older version of Widelands
and cannot be loaded anymore, or it's a new version that can't be handled yet.
Packet Name: Critter
Saved Version: 2
Current Version: 1.
Fine
--
https://code.launchpad.net/~widelands-dev/widelands/elk_moose/+merge/369201
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/elk_moose.
___
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/elk_moose into lp:widelands
Review: Approve review, compile, short test OTOH the code is OK for me, anything more I can / must do? -- https://code.launchpad.net/~widelands-dev/widelands/elk_moose/+merge/369201 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/elk_moose. ___ 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/elk_moose into lp:widelands
I openend some buitin and a new Map, how can I see this new value in a map? I am missing some info about wl_map_info / wl_map_object_info -- https://code.launchpad.net/~widelands-dev/widelands/elk_moose/+merge/369201 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/elk_moose 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/elk_moose into lp:widelands
I expect some followup change on the website then.
Widelands will always read older Maps.
Did another review with a look into the complete code.
(We need some Widelands historian for all this legacy :-)
A bit more documentation would help.
Diff comments:
>
> === modified file 'src/map_io/widelands_map_loader.cc'
> --- src/map_io/widelands_map_loader.cc2019-05-26 03:14:41 +
> +++ src/map_io/widelands_map_loader.cc2019-06-22 11:22:28 +
> @@ -198,7 +202,7 @@
> log("Reading Map Version Data ... ");
> {
> MapVersionPacket p;
> - p.read(*fs_, egbase, is_game, *mol_);
> + p.read(*fs_, egbase, is_game, old_world_name_.empty());
Please add a command what this old_world_name_ is (rep. was)
> }
> log("took %ums\n ", timer.ms_since_last_query());
>
--
https://code.launchpad.net/~widelands-dev/widelands/elk_moose/+merge/369201
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/elk_moose 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/elk_moose into lp:widelands
Some questions inline Code otherwise LGTM.
Will commpile this and the read that code again.
Diff comments:
>
> === modified file 'src/logic/map.cc'
> --- src/logic/map.cc 2019-05-16 09:15:03 +
> +++ src/logic/map.cc 2019-06-22 11:22:28 +
> @@ -714,6 +714,22 @@
> pathfieldmgr_->set_size(field_size);
> }
>
> +int Map::needs_widelands_version_after() const {
> + return map_version_.needs_widelands_version_after;
> +}
> +
> +void Map::calculate_needs_widelands_version_after(bool is_post_one_world) {
* Please add a acommnt what this "is_post_one_world" means
> + if (map_version_.needs_widelands_version_after == 0) {
> + if (nrplayers_ > 8) {
> + // We introduced support for 16 players after Build 19
> + map_version_.needs_widelands_version_after = 19;
> + } else if (is_post_one_world) {
> + // We merged the worlds in the engine after Build 18
> + map_version_.needs_widelands_version_after = 18;
> + }
> + }
> +}
> +
> /*
> * The scenario get/set functions
> */
>
> === modified file 'src/map_io/map_version_packet.cc'
> --- src/map_io/map_version_packet.cc 2019-02-23 11:00:49 +
> +++ src/map_io/map_version_packet.cc 2019-06-22 11:22:28 +
> @@ -30,11 +30,17 @@
> namespace Widelands {
>
> constexpr uint16_t kCurrentPacketVersion = 1;
> +// Map compatibility information for the website
> +constexpr int kCurrentNeedsWidelandsVersionAfter = 20;
Mhh, when do we have to increase that number? When we get the ferries?
>
> void MapVersionPacket::read(FileSystem& fs,
> EditorGameBase& egbase,
> -bool const skip,
> -MapObjectLoader&) {
> +bool const skip, bool is_post_one_world) {
> +
> + pre_read(fs, egbase.mutable_map(), skip, is_post_one_world);
> +}
> +
> +void MapVersionPacket::pre_read(FileSystem& fs, Map* map, bool skip, bool
> is_post_one_world) {
> if (skip)
> return;
>
--
https://code.launchpad.net/~widelands-dev/widelands/elk_moose/+merge/369201
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/elk_moose 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/ferry into lp:widelands
Now, how do we get progress here? Will try to play this again on Cavisson -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/failed_skipped_10s into lp:widelands
Running regression tests locally: ./regression_test.py -b ./widelands -- https://code.launchpad.net/~widelands-dev/widelands/failed_skipped_10s/+merge/368014 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/failed_skipped_10s. ___ 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/refactor_gameclient into lp:widelands
Now why can we nort merge this one? -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/refactor_gameclient. ___ 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/ferry into lp:widelands
Review: Needs Fixing testplay I played Calvission again fo about 4 hours gametime. This was fine for the most parts, I found some of the expected Issued: e.g. Buildings not attachted to roads dont get workers. I found one Issue where ferries are not built. But now I hit an seertion that should be reproducible, please check: https://www.magentacloud.de/share/tu4ayusx.k Unferry.wgf If we really want this feature, we should get it in _now_ and fix such bugs with seperate bugs. Otherwsie this Brnach will diverge more and more. Lets get this decided soon. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Here is the SaveGame: https://www.magentacloud.de/share/tu4ayusx.k Ferries2.wgf (Multiplayer Game) -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Review: Needs Fixing Played Calvission a #1 with a Barbarian at #2 , Imperial at #3 and Frisisan at #4 Got an assert aftree trying to build two woodcutters: Forcing flag at (70, 164) Message: adding warehouse for player 1 at (69, 163) Forcing flag at (73, 163) Forcing flag at (66, 165) Forcing flag at (65, 169) Forcing flag at (157, 27) Message: adding warehouse for player 2 at (157, 26) Forcing flag at (168, 141) Message: adding warehouse for player 3 at (168, 140) Forcing flag at (56, 45) Message: adding warehouse for player 4 at (56, 44) InternetGaming: Received a client list update with 18 items. InternetGaming: Received a game list update with 1 items. Assertion failed: (wh), function get_next_step, file ../src/economy/transfer.cc, line 186. Abort trap: 6 4 widelands 0x0001049b4f48 Widelands::Transfer::get_next_step(Widelands::PlayerImmovable*, bool&) + 3320 (transfer.cc:186) 5 widelands 0x0001049bc711 Widelands::WareInstance::update(Widelands::Game&) + 1841 (ware_instance.cc:330) 6 widelands 0x0001048f73f4 Widelands::Flag::add_ware(Widelands::EditorGameBase&, Widelands::WareInstance&) + 1524 (flag.cc:462) 7 widelands 0x000102f0cbfd Widelands::Worker::dropoff_update(Widelands::Game&, Widelands::Bob::State&) + 2381 (worker.cc:2123) 8 widelands 0x000102abc710 Widelands::Bob::do_act(Widelands::Game&) + 704 (bob.cc:195) Do you need a savegame? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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-1827182-sort-client-list into lp:widelands
Some regression tais fail, but I dont think they are related: ./regression_test.py -b ./widelands FAIL: test/maps/ship_transportation.wmf/scripting/test_rip_ship_before_picking_up_transporting_ware.lua FAIL: test/maps/ship_transportation.wmf/scripting/test_rip_portdock_with_worker_and_ware_in_transit.lua FAIL: test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_worker_in_transit.lua Ran 44 tests in 1493.063s FAILED (failures=3) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1827182-sort-client-list/+merge/366843 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1827182-sort-client-list. ___ 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-1827182-sort-client-list into lp:widelands
Done -- https://code.launchpad.net/~widelands-dev/widelands/bug-1827182-sort-client-list/+merge/366843 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1827182-sort-client-list. ___ 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-1827182-sort-client-list into lp:widelands
I think this is a better fix:
bool has_focus() const {
return (get_can_focus() && parent_->focus_ == this);
}
That can_ / has_focus handling is a bit different then I had expected, well
Works find for me now
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827182-sort-client-list/+merge/366843
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1827182-sort-client-list.
___
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-1827182-sort-client-list into lp:widelands
Mhh, that assert looks valid fro me. Question is why this particular inputfield does not have the corresponding flag set? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1827182-sort-client-list/+merge/366843 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1827182-sort-client-list. ___ 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-1827182-sort-client-list into lp:widelands
Thats the way how Asserts work ... only in debug bilds. I dont have that much time today, Ill try to throw it at the debugger. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1827182-sort-client-list/+merge/366843 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1827182-sort-client-list. ___ 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/refactor_gameclient into lp:widelands
Review: Resubmit Found that I lost a '!' while refactoring. Now a normal network game works as itendend. I assume that disconnect/double freee has always been there, I will create a followup bug for that one. When debugging I triggered a racecondition, I doubt we ever will see that i the wild. Gun: and all, please take anotheer look -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/refactor_gameclient. ___ 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/refactor_gameclient into lp:widelands
Using unique_ptr is unneeded, this is only a helper class neede to as long as some progress dialog is open, so using normal scope is ok. (We should actually cleanup that usage, some other time) The Problem is the owenership of UI::ProgressWindow* loader The disconnect code try to close the last 'modal' window. In case of the loader this is incorrect. Not sure how to cleanup tis messs by now -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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/refactor_gameclient into lp:widelands
Still get a heap-use-after-free related to std::unique_ptr loader_ui(new UI::ProgressWindow()); But this is maybe just a problem as the game crashed with disconnect(CLIENT_CRASHED, ) A have a deja vue around GameClientImpl.modal which is released twice. -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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/refactor_gameclient into lp:widelands
Found a first bug, still need some debugging -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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/refactor_gameclient into lp:widelands
* Used parent instead of This. * moved all Variables into Impl About that -2 magic: I got lost in finding the sematics of user/player etc. I added a TODO comment with our best guess by now. I am at the End of my vacation (sigh) so lets get this in before it starts lingering around for too long. -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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/tribe-economy-settings into lp:widelands
Review: Disapprove short review I go along with Benedikt. An average player will have a _lot_ of trouble to get the economy going and will have no clue what the problems are. We would have to rethink all the Tutorials and Scenarios. Some novice players may be trapped in on of the tribes special pitfalls even faster then now (e.g. trying to build a silk infrastructure without silk, winery without Marble etc...) If you want to create a challenge for experienced players, lets have a startcondition like "Limited Start Resources" or such -- https://code.launchpad.net/~widelands-dev/widelands/tribe-economy-settings/+merge/366878 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/tribe-economy-settings. ___ 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/ferry into lp:widelands
Loading may savegame I now get: Assertion failed: (wh), function get_next_step, file ../src/economy/transfer.cc, line 188. Do I have to restart that Map now? -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
I now get a compile Error at:
../src/economy/transfer.cc:166:72: error: non-pointer operand type
'Widelands::MapObjectType' incompatible with nullptr
if (!curflag.get_roadbase(nextflag, request_->get_type() ==
wwWORKER ? MapObjectType::ROAD : nullptr)) {
^ ~~~
../src/economy/transfer.cc:184:78: error: non-pointer operand type
'Widelands::MapObjectType' incompatible with nullptr
if (!nextflag.get_roadbase(nextnextflag,
request_->get_type() == wwWORKER ? MapObjectType::ROAD : nullptr)) {
--
https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/ferry.
___
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/ferry into lp:widelands
Review: Approve compile, review, testplay The ccode is basiccally OK, I did not read all of it, sorry. I noticed some odd Behavior that carrriees tried to use a wterway to reach theire destination, but got stuck on the waterway. This way neither wares no carriers where transported in the end. I think we cann still merge this one (nice work Benedikt!) and create Bugtickets later. I got a saveagme and some movie showing the Problem. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
Plaaying Calvission for a while now: In case a location is reachabel by road and by waterway, but there is no ferry, wares may get stuck waiting at the flag for the waterway, mmh. But well, works as designed. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/ferry into lp:widelands
CLang gives me some comppiler warnings:
src/economy/economy.cc:854:45: warning: loop variable 'r' has type
'const std::pair &'
(aka 'const pair &')
but is initialized with type
'std::__1::__map_iterator,
std::__1::__tree_node, void *> *, long> >::value_type'
(aka 'pair')
resulting in a copy [-Wrange-loop-analysis]
for (const std::pair& r : open_requests) {
^
../src/economy/economy.cc:854:8: note: use non-reference type
'std::pair' (aka 'pair') to keep the copy or type 'const
std::__1::__map_iterator,
std::__1::__tree_node, void *> *, long> >::value_type &'
(aka 'const pair &') to prevent copying
for (const std::pair& r : open_requests)
I think you need more const inside the declaration of r?
/src/wui/economy_options_window.cc:20:
../src/wui/economy_options_window.h:56:25: warning: private field 'type_' is
not used
[-Wunused-private-field] Widelands::WareWorker type_;
You should see them in the automated builds, too
--
https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/ferry.
___
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/ferry into lp:widelands
OK this is a little Monster and in depends on some workarea change? Please direkt me to that branch or was this mereged by now? Im checking this out now. and (try to) do some code review. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Mhhh, We could iprove the RoadType via 4 bitflags and some inline fucntions: 1000 ^ Road ^ Bridge ^ Waterway ^ Busy kRoad = 1 kNormalRoad = 1 kBusyRoad = 9 kBridge = 2 kBridgeNormal = 2 kBridgeBusy = 10 kWaterway = 4 kNormalWaterway = 4 kBusyWaterway = 12 kBusy = 8 This woud avoid a lot of if(this && that || soemthingElse) code? Once this is in I would address SirVers TODO and extract some RoadDirection enum -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Now, can ths be merged or do we want another review? -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/refactor_gameclient into lp:widelands
Hello Gun: * Adressed some comments * Get the tribe and check whether it exists before you push it -> I have no idea (yet) what this code does :-) * Playercommand is not for current player , not sure if this can normally happen lets wait If I ever see this, then we can make it an assert. * The This / this confusion is because I moved some fucntion into Impl, but they need the main class. I could use "outer" or some other name to avoid it. * send_player_command(Widelands::PlayerCommand* pc) making this a unique_ptr: * Will affect a lot of code * Some aspects of this code are not clear to me (e.g. Commnands a queued for networking) -> lets adress this in a seperate branch, I already regret I touched it :-) * I would leave "GameClientImpl* d" as is: * it is used very often * it is used in the local scope only -> no risc to loose control * commonly used Pattern in Widelands * d->settings.usernum == -2,-1, n seems to be used to define the "hello" handshake what about "kPreHello" and "kAfterHello" constants? Maybe we should move the two remaining variables to GameClientImpl, too? Please check the logic of the main switch statement, I changed some control flow for better readbility. I intend to do a similar refactoring with gamehost, too. -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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/refactor_gameclient into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/refactor_gameclient into lp:widelands has been updated. Description changed to: * Refactor big switch statement in gameclient.cc * Refactor ::run() to improve readability * use send_player_command with Ptr instead of Reference to clarify ownership of PlayerCommand. * Added more comments Added some TODOs and empty comments where I did not know the details. There should be no functional change at all. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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/refactor_gameclient into lp:widelands
Klaus Halfmann has proposed merging
lp:~widelands-dev/widelands/refactor_gameclient into lp:widelands.
Commit message:
Refactor gameclient.h/.cc for better readability
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743
* Refactor big switch statement in gameclient.cc
* Refactor ::run() to improve readability
* use send_play_camd with Ptr instead of reference to clarify ownership of cmd.
* Added more comments
Added some TODOs and empty comments where I dodnt know the details.
There should be no functional change at all.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/refactor_gameclient into lp:widelands.
=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc 2019-04-10 16:39:31 +
+++ src/ai/defaultai.cc 2019-05-01 07:38:30 +
@@ -6544,7 +6544,7 @@
const uint16_t new_target = std::max(default_target * multiplier / 10, 3);
assert(new_target > 1);
- game().send_player_command(*new Widelands::CmdSetWareTargetQuantity(
+ game().send_player_command(new Widelands::CmdSetWareTargetQuantity(
gametime, player_number(), observer->economy.serial(), id, new_target));
}
}
=== modified file 'src/logic/game.cc'
--- src/logic/game.cc 2019-04-26 16:52:39 +
+++ src/logic/game.cc 2019-05-01 07:38:30 +
@@ -712,7 +712,7 @@
* It takes the appropriate action, i.e. either add to the cmd_queue or send
* across the network.
*/
-void Game::send_player_command(PlayerCommand& pc) {
+void Game::send_player_command(PlayerCommand* pc) {
ctrl_->send_player_command(pc);
}
@@ -735,55 +735,55 @@
// we might want to make these inlines:
void Game::send_player_bulldoze(PlayerImmovable& pi, bool const recurse) {
- send_player_command(*new CmdBulldoze(get_gametime(), pi.owner().player_number(), pi, recurse));
+ send_player_command(new CmdBulldoze(get_gametime(), pi.owner().player_number(), pi, recurse));
}
void Game::send_player_dismantle(PlayerImmovable& pi) {
- send_player_command(*new CmdDismantleBuilding(get_gametime(), pi.owner().player_number(), pi));
+ send_player_command(new CmdDismantleBuilding(get_gametime(), pi.owner().player_number(), pi));
}
void Game::send_player_build(int32_t const pid, const Coords& coords, DescriptionIndex const id) {
assert(tribes().building_exists(id));
- send_player_command(*new CmdBuild(get_gametime(), pid, coords, id));
+ send_player_command(new CmdBuild(get_gametime(), pid, coords, id));
}
void Game::send_player_build_flag(int32_t const pid, const Coords& coords) {
- send_player_command(*new CmdBuildFlag(get_gametime(), pid, coords));
+ send_player_command(new CmdBuildFlag(get_gametime(), pid, coords));
}
void Game::send_player_build_road(int32_t pid, Path& path) {
- send_player_command(*new CmdBuildRoad(get_gametime(), pid, path));
+ send_player_command(new CmdBuildRoad(get_gametime(), pid, path));
}
void Game::send_player_flagaction(Flag& flag) {
- send_player_command(*new CmdFlagAction(get_gametime(), flag.owner().player_number(), flag));
+ send_player_command(new CmdFlagAction(get_gametime(), flag.owner().player_number(), flag));
}
void Game::send_player_start_stop_building(Building& building) {
send_player_command(
- *new CmdStartStopBuilding(get_gametime(), building.owner().player_number(), building));
+ new CmdStartStopBuilding(get_gametime(), building.owner().player_number(), building));
}
void Game::send_player_militarysite_set_soldier_preference(Building& building,
SoldierPreference my_preference) {
- send_player_command(*new CmdMilitarySiteSetSoldierPreference(
+ send_player_command(new CmdMilitarySiteSetSoldierPreference(
get_gametime(), building.owner().player_number(), building, my_preference));
}
void Game::send_player_start_or_cancel_expedition(Building& building) {
send_player_command(
- *new CmdStartOrCancelExpedition(get_gametime(), building.owner().player_number(), building));
+ new CmdStartOrCancelExpedition(get_gametime(), building.owner().player_number(), building));
}
void Game::send_player_enhance_building(Building& building, DescriptionIndex const id) {
assert(building.owner().tribe().has_building(id));
send_player_command(
- *new CmdEnhanceBuilding(get_gametime(), building.owner().player_number(), building, id));
+ new CmdEnhanceBuilding(get_gametime(), building.owner().player_number(), building, id));
}
void Game::send_player_evict_worker(Worker& worker) {
- send_player_command(*new CmdEvictWorker(get_gametime(), worker.owner().player_number(), worker));
+ send_player_command(new CmdEvictWorker(get_gametime(), worker.owner().player_number(), worker));
}
void Game::send_player_set_ware_priority(PlayerImmovable& imm,
@@ -791,14 +791,14
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refactor_gameclient into lp:widelands
Regressiontest are ok: Ran 44 tests in 1377.106s Will try to do a network game, too. -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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-1826669-mp-map-b20 into lp:widelands/build20
Review: Approve compile / test OK, tested this again, works as intended. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ 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-1826669-mp-map-b20 into lp:widelands/build20
Yep, found and deleted it, will try again tomorrow. Playing against WorldSavior and the-X makes me tired. Thats another Bug for the Review in R21 then ... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ 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-1826669-mp-map-b20 into lp:widelands/build20
I found one: file_ = nullptr; and !file_ // this should be save. in gameclient.cc I used a Windows Server wit build20_rc1 as host so I could not test gamehost.cc this way. Now I am out of Ideas... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ 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-1826669-mp-map-b20 into lp:widelands/build20
Review: Needs Fixing compile test Your soultion prevents the crahs but makes a difference: - The new Map is not announced at the GUI + The new Map appears in the logs - No Transfer of any Map happens any longer. from the code I see no diecrt difference. eventually ther is some == comparison for file_ which we must consider? lets check that code, what is done with that file_? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ 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-1826669-mp-map-b20 into lp:widelands/build20
Admitted, you fix is more elegant.
One nit inline.
Wil still compile and test this
Diff comments:
>
> === modified file 'src/network/network.h'
> --- src/network/network.h 2019-02-23 11:00:49 +
> +++ src/network/network.h 2019-04-28 14:30:33 +
> @@ -181,6 +181,11 @@
> };
>
> struct NetTransferFile {
> + NetTransferFile() : bytes(0), filename(""), md5sum("") {
> + }
> + ~NetTransferFile() {
> + parts.clear();
That extra clear(() does not make any difference
> + }
> uint32_t bytes;
> std::string filename;
> std::string md5sum;
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20.
___
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_1826669_MapDownloandChange into lp:widelands/build20
This will merge to buld20. I will try to do a bigger refactoring for trunk, but we should merge it there, too. -- https://code.launchpad.net/~widelands-dev/widelands/bug_1826669_MapDownloandChange/+merge/366616 Your team Widelands Developers is subscribed to branch lp:widelands/build20. ___ 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_1826669_MapDownloandChange into lp:widelands/build20
Klaus Halfmann has proposed merging
lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange into
lp:widelands/build20.
Commit message:
Fix for #1826669
Requested reviews:
Toni Förster (stonerl): remote network test
Related bugs:
Bug #1826669 in widelands: "R20-rc1 HeapUseAfterFree when changig Map during
Download"
https://bugs.launchpad.net/widelands/+bug/1826669
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug_1826669_MapDownloandChange/+merge/366616
Avoid double delete of _file in gameclient.cc
--
Your team Widelands Developers is subscribed to branch lp:widelands/build20.
=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc 2019-02-23 11:00:49 +
+++ src/network/gameclient.cc 2019-04-28 11:30:30 +
@@ -586,8 +586,10 @@
d->settings.mapfilename.c_str());
// New map was set, so we clean up the buffer of a previously requested file
- if (file_)
+if (file_) {
delete file_;
+file_ = NULL;
+}
break;
}
@@ -637,8 +639,9 @@
s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
d->net->send(s);
- if (file_)
+ if (file_) {
delete file_;
+ }
file_ = new NetTransferFile();
file_->bytes = bytes;
=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc 2019-02-23 11:00:49 +
+++ src/network/gamehost.cc 2019-04-28 11:30:30 +
@@ -539,7 +539,9 @@
d->net.reset();
d->promoter.reset();
delete d;
+d = NULL;
delete file_;
+file_ = NULL;
}
const std::string& GameHost::get_local_playername() const {
=== modified file 'src/network/network.h'
--- src/network/network.h 2019-02-23 11:00:49 +
+++ src/network/network.h 2019-04-28 11:30:30 +
@@ -185,6 +185,14 @@
std::string filename;
std::string md5sum;
std::vector parts;
+
+NetTransferFile() {
+// Allow Debugging
+}
+
+~NetTransferFile() {
+// Allow Debugging
+}
};
class Deserializer {
___
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/cleanup-soundhandler into lp:widelands
Review: Approve reiew, compile, testplay
I played some internet game with this brnahc and it was ok, felt a bit slow,
though.
Completed the review, code is fine, please check some nits:
* some functions should be inlined.
* Once this is in r21 I would like to do a performace review
* Mabye we could make more of this run in a seperate thread / cpu?
This can go in in r21 anytime now.
Diff comments:
>
> === modified file 'src/sound/sound_handler.cc'
> --- src/sound/sound_handler.cc2019-02-23 11:00:49 +
> +++ src/sound/sound_handler.cc2019-04-09 04:52:18 +
> @@ -199,476 +176,461 @@
> fx_lock_ = nullptr;
> }
>
> - songs_.clear();
> - fxs_.clear();
> -
> Mix_Quit();
> SDL_QuitSubSystem(SDL_INIT_AUDIO);
> }
>
> -/** Read the main config file, load background music and systemwide sound fx
> - *
> +/// Prints an error and disables and shuts down the sound system.
> +void SoundHandler::initialization_error(const char* const msg, bool
> quit_sdl) {
> + log("WARNING: Failed to initialize sound system: %s\n", msg);
> + SoundHandler::disable_backend();
> + if (quit_sdl) {
> + SDL_QuitSubSystem(SDL_INIT_AUDIO);
> + }
> + return;
> +}
> +
> +/**
> + * Load the sound options from g_options. If an option is not available, use
> the defaults set by the constructor.
> */
> void SoundHandler::read_config() {
> - Section& s = g_options.pull_section("global");
> -
> - if (nosound_) {
> - set_disable_music(true);
> - set_disable_fx(true);
> - } else {
> - set_disable_music(s.get_bool("disable_music", false));
> - set_disable_fx(s.get_bool("disable_fx", false));
> - music_volume_ = s.get_int("music_volume", kDefaultMusicVolume);
> - fx_volume_ = s.get_int("fx_volume", kDefaultFxVolume);
> - }
> -
> - random_order_ = s.get_bool("sound_random_order", true);
> -
> - register_song("music", "intro");
> - register_song("music", "menu");
> - register_song("music", "ingame");
> -}
> -
> -/** Load systemwide sound fx into memory.
> - * \note This loads only systemwide fx. Worker/building fx will be loaded by
> - * their respective conf-file parsers
> - */
> -void SoundHandler::load_system_sounds() {
> - load_fx_if_needed("sound", "click", "click");
> - load_fx_if_needed("sound", "create_construction_site",
> "create_construction_site");
> - load_fx_if_needed("sound", "message", "message");
> - load_fx_if_needed("sound/military", "under_attack",
> "military/under_attack");
> - load_fx_if_needed("sound/military", "site_occupied",
> "military/site_occupied");
> - load_fx_if_needed("sound", "lobby_chat", "lobby_chat");
> - load_fx_if_needed("sound", "lobby_freshmen", "lobby_freshmen");
> -}
> -
> -/**
> - * Returns 'true' if the playing of sounds is disabled due to sound driver
> problems.
> - */
> -bool SoundHandler::is_backend_disabled() const {
> - return is_backend_disabled_;
> -}
> -
> -/** Load a sound effect. One sound effect can consist of several audio files
> + // TODO(GunChleoc): Compatibility code to avoid getting bug reports
> about unread sections. Remove after Build 21.
> + if (g_options.get_section("sound") == nullptr) {
> + Section& global = g_options.pull_section("global");
> +
> + for (auto& option : sound_options_) {
> + switch (option.first) {
> + case SoundType::kMusic:
> + option.second.volume =
> global.get_int("music_volume", option.second.volume);
> + option.second.enabled =
> !global.get_bool("disable_music", !option.second.enabled);
> + break;
> + case SoundType::kChat:
> + option.second.volume =
> global.get_int("fx_volume", option.second.volume);
> + option.second.enabled =
> global.get_bool("sound_at_message", option.second.enabled);
> + break;
> + default:
> + option.second.volume =
> global.get_int("fx_volume", option.second.volume);
> + option.second.enabled =
> !global.get_bool("disable_fx", !option.second.enabled);
> + break;
> + }
> + }
> + save_config();
> + }
> +
> + // This is the code that we want to keep
> + Section& sound = g_options.pull_section("sound");
> + for (auto& option : sound_options_) {
> + option.second.volume = sound.get_int(("volume_" +
> option.second.name).c_str(), option.second.volume);
> + option.second.enabled = sound.get_bool(("enable_" +
> option.second.name).c_str(), option.second.enabled);
> + }
> +}
> +
> +/// Save the current sound options to g_options
> +void
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/cleanup-soundhandler into lp:widelands
Review, found so far:
* restructered the sound directory wihht more subdirectories
* Help now shows --nosoundStarts the game with sound disabled.
* Using FXset one could have different soundscapes for testing or scenarions,
interesting
* I assuem the random must be a _local_ random, as sounds are per player.
* More coommetns please, see inline comments
For the wishlist: a helpscreen explainng the sounds :-)
Checekd upto "TODO(GunChleoc): Compatibility code to avoid getting bug reports
about unread sections. Remove after Build 21." sorry can nott do more tonight
Diff comments:
>
> === modified file 'src/economy/road.h'
> --- src/economy/road.h2019-02-27 17:19:00 +
> +++ src/economy/road.h2019-04-09 04:52:18 +
> @@ -131,8 +131,7 @@
> void cleanup(EditorGameBase&) override;
>
> void draw(uint32_t gametime,
> - TextToDraw draw_text,
> - const Vector2f& point_on_dst,
> + TextToDraw draw_text, const Vector2f&, const Coords&,
mmh, i like those named varibales better, Now I must guess what "const Coords&"
may be used for?
> float scale,
> RenderTarget* dst) override;
>
>
> === modified file 'src/editor/editorinteractive.cc'
> --- src/editor/editorinteractive.cc 2019-02-27 17:19:00 +
> +++ src/editor/editorinteractive.cc 2019-04-09 04:52:18 +
> @@ -405,6 +405,10 @@
> is_painting_ = false;
> }
>
> +bool EditorInteractive::player_hears_field(const Widelands::Coords&) const {
Did I tell you, that I like comments?
> + return true;
> +}
> +
> void EditorInteractive::on_buildhelp_changed(const bool value) {
> toggle_buildhelp_->set_perm_pressed(value);
> }
--
https://code.launchpad.net/~widelands-dev/widelands/cleanup-soundhandler/+merge/365001
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/cleanup-soundhandler 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/cleanup-soundhandler into lp:widelands
I played the first 3 Tutorial on this branhc and everyhting was fine. When hanging around in the config I get > Songset: Loaded song "music/menu_00.ogg" again and again, is this intended, or should this happen only once I will try to do a code review. But this seems to be a _bigger_ task :-) -- https://code.launchpad.net/~widelands-dev/widelands/cleanup-soundhandler/+merge/365001 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/cleanup-soundhandler 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/vector-float-warnings into lp:widelands
OOps, you are right and its a compiler error:
> ../src/wui/mapview.h:83:13: error: expected unqualified-id before 'const'
> bool near(const View& other) const {
^
But I have no idea what the problem ist, did we hit some reserved indentifier
for WIN/GCC?
--
https://code.launchpad.net/~widelands-dev/widelands/vector-float-warnings/+merge/365003
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/vector-float-warnings.
___
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:~nordfriese/widelands/workareas into lp:widelands
Is this for R20? -- https://code.launchpad.net/~nordfriese/widelands/workareas/+merge/364266 Your team Widelands Developers is requested to review the proposed merge of lp:~nordfriese/widelands/workareas 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/vector-float-warnings into lp:widelands
Review: Approve compile, test, review compiled, warnings are gone. Jumped around a bit in the WideWorldMap, all fine. Anyrthing else we must check? -- https://code.launchpad.net/~widelands-dev/widelands/vector-float-warnings/+merge/365003 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/vector-float-warnings. ___ 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
