← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/ferry into lp:widelands

 

The overlays look fine now, thank you.

I have pushed a commit with a code review. Check the "NOCOM" comments.

Some notes that are not in the review commit:

- Ferry yard needs an economy check, just like the Ox Farm etc. Otherwise, it will just keep producing en masse.

- switch is faster than if/else, so use that for distinguishing ware/worker cases if it keeps the code still readable

- And ASan is not happy. I built a ferry yard, then a waterway. Connected a construction site with a separate flag to the other end of the waterway and waited a bit. Only then did I connect the worker economy -> boom


TI(2857): destination appears to have become split from current location -> fail
=================================================================
==32501==ERROR: AddressSanitizer: heap-use-after-free on address 0x61100075fb38 at pc 0x55b7a7a1a6e7 bp 0x7ffcd58678f0 sp 0x7ffcd58678e0
READ of size 8 at 0x61100075fb38 thread T0
    #0 0x55b7a7a1a6e6 in __gnu_cxx::__normal_iterator<Widelands::Ferry**, std::vector<Widelands::Ferry*, std::allocator<Widelands::Ferry*> > >::__normal_iterator(Widelands::Ferry** const&) /usr/include/c++/7/bits/stl_iterator.h:780
    #1 0x55b7a7a127c5 in std::vector<Widelands::Ferry*, std::allocator<Widelands::Ferry*> >::begin() /usr/include/c++/7/bits/stl_vector.h:564
    #2 0x55b7a7a079d2 in Widelands::Fleet::cancel_ferry_request(Widelands::Game&, Widelands::Waterway*) ../src/economy/fleet.cc:533
    #3 0x55b7a7a5b870 in Widelands::Waterway::cleanup(Widelands::EditorGameBase&) ../src/economy/waterway.cc:227
    #4 0x55b7a7084ea7 in Widelands::MapObject::remove(Widelands::EditorGameBase&) ../src/logic/map_objects/map_object.cc:434
    #5 0x55b7a7081626 in Widelands::ObjectManager::cleanup(Widelands::EditorGameBase&) ../src/logic/map_objects/map_object.cc:162
    #6 0x55b7a6ccb2d7 in Widelands::EditorGameBase::cleanup_objects() ../src/logic/editor_game_base.h:170
    #7 0x55b7a6f6dd40 in Widelands::Game::run(UI::ProgressWindow*, Widelands::Game::StartGameType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ../src/logic/game.cc:554
    #8 0x55b7a6c6cef0 in WLApplication::new_game() ../src/wlapplication.cc:1320
    #9 0x55b7a6c6b29c in WLApplication::mainmenu_singleplayer() ../src/wlapplication.cc:1177
    #10 0x55b7a6c6a3d3 in WLApplication::mainmenu() ../src/wlapplication.cc:1082
    #11 0x55b7a6c61699 in WLApplication::run() ../src/wlapplication.cc:454
    #12 0x55b7a6c5dbee in main ../src/main.cc:44
    #13 0x7f0401bfbb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #14 0x55b7a6c5da69 in _start (/media/cuideigin/Linux/sources/widelands/ferry/widelands+0xd94a69)

-- 
https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ferry into lp:widelands.


References