The diff looks okay to me and a short test game hasn't shown any optical glitches. But... ;-)
Three minor remarks code-wise:
- Is there a reason that Bob::calc_drawpos() (and derived methods) are still returning floats? Since it returns the pixel position on the screen I guess it is always an integer anyway.
- UI::correct_for_align() and UI::center_vertically() are somehow related from what they are doing but are offering quite different interfaces. Since I found an older TODO concerning the Align-enum I guess this might be already scheduled to change at some time.
- correct_for_align(kCenter) is called but correct_for_align(kLeft) is omitted. This might be a problem with right-to-left languages (or strange corner cases I could not come up with right now). Could also already be scheduled for change.
Another problem is a crash I encountered in my test game. It happened randomly and I couldn't reproduce it, so it might be also in trunk. But since it is graphic/blitting related I post the backtrace here for now.
Cmd_EnemyFlagAction::execute player(1): flag->owner(2) number=1
Cmd_EnemyFlagAction::execute player(1): flag->owner(2) number=1
Cmd_EnemyFlagAction::execute player(1): flag->owner(2) number=1
Forcing flag at (45, 31)
Thread 1 "widelands" received signal SIGSEGV, Segmentation fault.
0x0000555555de1828 in std::equal_to<unsigned int>::operator() (this=0x555556ad91b8 <Gl::State::instance()::binder+56>, __x=@0x7fffffff9100: 27123, __y=<error reading variable>)
at /usr/include/c++/6/bits/stl_function.h:356
356 { return __x == __y; }
(gdb) bt
#0 0x0000555555de1828 in std::equal_to<unsigned int>::operator() (this=0x555556ad91b8 <Gl::State::instance()::binder+56>, __x=@0x7fffffff9100: 27123, __y=<error reading variable>)
at /usr/include/c++/6/bits/stl_function.h:356
#1 0x00005555563b0eab in std::__detail::_Equal_helper<unsigned int, std::pair<unsigned int const, unsigned int>, std::__detail::_Select1st, std::equal_to<unsigned int>, unsigned long, false>::_S_equals (__eq=..., __extract=..., __k=@0x7fffffff9100: 27123, __n=0x608f5b40) at /usr/include/c++/6/bits/hashtable_policy.h:1331
#2 0x00005555563b0b0a in std::__detail::_Hashtable_base<unsigned int, std::pair<unsigned int const, unsigned int>, std::__detail::_Select1st, std::equal_to<unsigned int>, std::hash<unsigned int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Hashtable_traits<false, false, true> >::_M_equals (
this=0x555556ad91b8 <Gl::State::instance()::binder+56>, __k=@0x7fffffff9100: 27123, __c=27123, __n=0x608f5b40) at /usr/include/c++/6/bits/hashtable_policy.h:1702
#3 0x00005555563b0687 in std::_Hashtable<unsigned int, std::pair<unsigned int const, unsigned int>, std::allocator<std::pair<unsigned int const, unsigned int> >, std::__detail::_Select1st, std::equal_to<unsigned int>, std::hash<unsigned int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node (this=0x555556ad91b8 <Gl::State::instance()::binder+56>, __n=27123, __k=@0x7fffffff9100: 27123, __code=27123)
at /usr/include/c++/6/bits/hashtable.h:1420
#4 0x00005555563b0072 in std::_Hashtable<unsigned int, std::pair<unsigned int const, unsigned int>, std::allocator<std::pair<unsigned int const, unsigned int> >, std::__detail::_Select1st, std::equal_to<unsigned int>, std::hash<unsigned int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_node (this=0x555556ad91b8 <Gl::State::instance()::binder+56>, __bkt=27123, __key=@0x7fffffff9100: 27123, __c=27123)
at /usr/include/c++/6/bits/hashtable.h:634
#5 0x00005555563af71e in std::__detail::_Map_base<unsigned int, std::pair<unsigned int const, unsigned int>, std::allocator<std::pair<unsigned int const, unsigned int> >, std::__detail::_Select1st, std::equal_to<unsigned int>, std::hash<unsigned int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true>, true>::operator[] (this=0x555556ad91b8 <Gl::State::instance()::binder+56>, __k=@0x7fffffff9100: 27123) at /usr/include/c++/6/bits/hashtable_policy.h:591
#6 0x00005555563aefe5 in std::unordered_map<unsigned int, unsigned int, std::hash<unsigned int>, std::equal_to<unsigned int>, std::allocator<std::pair<unsigned int const, unsigned int> > >::operator[] (this=0x555556ad91b8 <Gl::State::instance()::binder+56>, __k=@0x7fffffff9100: 27123) at /usr/include/c++/6/bits/unordered_map.h:904
#7 0x00005555563ae47e in Gl::State::do_bind (this=0x555556ad9180 <Gl::State::instance()::binder>, target=33985, texture=27123)
at /widelands/rendertarget_ints/src/graphic/gl/utils.cc:196
#8 0x00005555563ae3b6 in Gl::State::bind (this=0x555556ad9180 <Gl::State::instance()::binder>, target=33985, texture=27123)
at /widelands/rendertarget_ints/src/graphic/gl/utils.cc:180
#9 0x000055555636751d in BlitProgram::draw (this=0x555556ad8e60 <BlitProgram::instance()::blit_program>, arguments=std::vector of length 416, capacity 512 = {...})
at /widelands/rendertarget_ints/src/graphic/gl/blit_program.cc:166
#10 0x0000555556222689 in RenderQueue::draw_items (this=0x555556ad6a60 <RenderQueue::instance()::render_queue>, items=std::vector of length 455, capacity 1024 = {...})
at /widelands/rendertarget_ints/src/graphic/render_queue.cc:226
#11 0x0000555556222585 in RenderQueue::draw (this=0x555556ad6a60 <RenderQueue::instance()::render_queue>, screen_width=1920, screen_height=1080)
at /widelands/rendertarget_ints/src/graphic/render_queue.cc:213
#12 0x0000555555e7f71a in Graphic::refresh (this=0x555556b44630) at /widelands/rendertarget_ints/src/graphic/graphic.cc:206
#13 0x000055555607e130 in UI::Panel::do_run (this=0x55555f50c240) at /widelands/rendertarget_ints/src/ui_basic/panel.cc:197
#14 0x0000555555dda8a8 in UI::Panel::run<UI::Panel::Returncodes> (this=0x55555f50c240) at ../src/ui_basic/panel.h:96
#15 0x0000555555eea120 in Widelands::Game::run (this=0x7fffffff99b0, loader_ui=0x7fffffff9810, start_game_type=Widelands::Game::NewNonScenario, script_to_run="", replay=false,
prefix_for_replays="single_player") at /widelands/rendertarget_ints/src/logic/game.cc:525
#16 0x0000555555dd4dae in WLApplication::new_game (this=0x555556b03a90) at /widelands/rendertarget_ints/src/wlapplication.cc:1243
#17 0x0000555555dd4368 in WLApplication::mainmenu_singleplayer (this=0x555556b03a90) at /widelands/rendertarget_ints/src/wlapplication.cc:1103
#18 0x0000555555dd3d5f in WLApplication::mainmenu (this=0x555556b03a90) at /widelands/rendertarget_ints/src/wlapplication.cc:1008
#19 0x0000555555dd0758 in WLApplication::run (this=0x555556b03a90) at /widelands/rendertarget_ints/src/wlapplication.cc:445
#20 0x0000555555dced0d in main (argc=1, argv=0x7fffffffd918) at /widelands/rendertarget_ints/src/main.cc:49
The diff looks okay to me and a short test game hasn't shown any optical glitches. But... ;-)
Three minor remarks code-wise: for_align( ) and UI::center_ vertically( ) are somehow related from what they are doing but are offering quite different interfaces. Since I found an older TODO concerning the Align-enum I guess this might be already scheduled to change at some time. for_align( kCenter) is called but correct_ for_align( kLeft) is omitted. This might be a problem with right-to-left languages (or strange corner cases I could not come up with right now). Could also already be scheduled for change.
- Is there a reason that Bob::calc_drawpos() (and derived methods) are still returning floats? Since it returns the pixel position on the screen I guess it is always an integer anyway.
- UI::correct_
- correct_
Another problem is a crash I encountered in my test game. It happened randomly and I couldn't reproduce it, so it might be also in trunk. But since it is graphic/blitting related I post the backtrace here for now.
Cmd_EnemyFlagAc tion::execute player(1): flag->owner(2) number=1 tion::execute player(1): flag->owner(2) number=1 tion::execute player(1): flag->owner(2) number=1
Cmd_EnemyFlagAc
Cmd_EnemyFlagAc
Forcing flag at (45, 31)
Thread 1 "widelands" received signal SIGSEGV, Segmentation fault. to<unsigned int>::operator() (this=0x555556a d91b8 <Gl::State: :instance( )::binder+ 56>, __x=@0x7fffffff 9100: 27123, __y=<error reading variable>) c++/6/bits/ stl_function. h:356 to<unsigned int>::operator() (this=0x555556a d91b8 <Gl::State: :instance( )::binder+ 56>, __x=@0x7fffffff 9100: 27123, __y=<error reading variable>) c++/6/bits/ stl_function. h:356 detail: :_Equal_ helper< unsigned int, std::pair<unsigned int const, unsigned int>, std::__ detail: :_Select1st, std::equal_ to<unsigned int>, unsigned long, false>::_S_equals (__eq=..., __extract=..., __k=@0x7fffffff 9100: 27123, __n=0x608f5b40) at /usr/include/ c++/6/bits/ hashtable_ policy. h:1331 detail: :_Hashtable_ base<unsigned int, std::pair<unsigned int const, unsigned int>, std::__ detail: :_Select1st, std::equal_ to<unsigned int>, std::hash<unsigned int>, std::__ detail: :_Mod_range_ hashing, std::__ detail: :_Default_ ranged_ hash, std::__ detail: :_Hashtable_ traits< false, false, true> >::_M_equals ( 0x555556ad91b8 <Gl::State: :instance( )::binder+ 56>, __k=@0x7fffffff 9100: 27123, __c=27123, __n=0x608f5b40) at /usr/include/ c++/6/bits/ hashtable_ policy. h:1702 <unsigned int, std::pair<unsigned int const, unsigned int>, std::allocator< std::pair< unsigned int const, unsigned int> >, std::__ detail: :_Select1st, std::equal_ to<unsigned int>, std::hash<unsigned int>, std::__ detail: :_Mod_range_ hashing, std::__ detail: :_Default_ ranged_ hash, std::__ detail: :_Prime_ rehash_ policy, std::__ detail: :_Hashtable_ traits< false, false, true> >::_M_find_ before_ node (this=0x555556a d91b8 <Gl::State: :instance( )::binder+ 56>, __n=27123, __k=@0x7fffffff 9100: 27123, __code=27123) c++/6/bits/ hashtable. h:1420 <unsigned int, std::pair<unsigned int const, unsigned int>, std::allocator< std::pair< unsigned int const, unsigned int> >, std::__ detail: :_Select1st, std::equal_ to<unsigned int>, std::hash<unsigned int>, std::__ detail: :_Mod_range_ hashing, std::__ detail: :_Default_ ranged_ hash, std::__ detail: :_Prime_ rehash_ policy, std::__ detail: :_Hashtable_ traits< false, false, true> >::_M_find_node (this=0x555556a d91b8 <Gl::State: :instance( )::binder+ 56>, __bkt=27123, __key=@ 0x7fffffff9100: 27123, __c=27123) c++/6/bits/ hashtable. h:634 detail: :_Map_base< unsigned int, std::pair<unsigned int const, unsigned int>, std::allocator< std::pair< unsigned int const, unsigned int> >, std::__ detail: :_Select1st, std::equal_ to<unsigned int>, std::hash<unsigned int>, std::__ detail: :_Mod_range_ hashing, std::__ detail: :_Default_ ranged_ hash, std::__ detail: :_Prime_ rehash_ policy, std::__ detail: :_Hashtable_ traits< false, false, true>, true>::operator[] (this=0x555556a d91b8 <Gl::State: :instance( )::binder+ 56>, __k=@0x7fffffff 9100: 27123) at /usr/include/ c++/6/bits/ hashtable_ policy. h:591 map<unsigned int, unsigned int, std::hash<unsigned int>, std::equal_ to<unsigned int>, std::allocator< std::pair< unsigned int const, unsigned int> > >::operator[] (this=0x555556a d91b8 <Gl::State: :instance( )::binder+ 56>, __k=@0x7fffffff 9100: 27123) at /usr/include/ c++/6/bits/ unordered_ map.h:904 d9180 <Gl::State: :instance( )::binder> , target=33985, texture=27123) rendertarget_ ints/src/ graphic/ gl/utils. cc:196 d9180 <Gl::State: :instance( )::binder> , target=33985, texture=27123) rendertarget_ ints/src/ graphic/ gl/utils. cc:180 d8e60 <BlitProgram: :instance( )::blit_ program> , arguments= std::vector of length 416, capacity 512 = {...}) rendertarget_ ints/src/ graphic/ gl/blit_ program. cc:166 :draw_items (this=0x555556a d6a60 <RenderQueue: :instance( )::render_ queue>, items=std::vector of length 455, capacity 1024 = {...}) rendertarget_ ints/src/ graphic/ render_ queue.cc: 226 d6a60 <RenderQueue: :instance( )::render_ queue>, screen_width=1920, screen_height=1080) rendertarget_ ints/src/ graphic/ render_ queue.cc: 213 44630) at /widelands/ rendertarget_ ints/src/ graphic/ graphic. cc:206 0c240) at /widelands/ rendertarget_ ints/src/ ui_basic/ panel.cc: 197 :run<UI: :Panel: :Returncodes> (this=0x55555f5 0c240) at ../src/ ui_basic/ panel.h: 96 :Game:: run (this=0x7ffffff f99b0, loader_ ui=0x7fffffff98 10, start_game_ type=Widelands: :Game:: NewNonScenario, script_to_run="", replay=false, for_replays= "single_ player" ) at /widelands/ rendertarget_ ints/src/ logic/game. cc:525 :new_game (this=0x555556b 03a90) at /widelands/ rendertarget_ ints/src/ wlapplication. cc:1243 :mainmenu_ singleplayer (this=0x555556b 03a90) at /widelands/ rendertarget_ ints/src/ wlapplication. cc:1103 :mainmenu (this=0x555556b 03a90) at /widelands/ rendertarget_ ints/src/ wlapplication. cc:1008 03a90) at /widelands/ rendertarget_ ints/src/ wlapplication. cc:445 d918) at /widelands/ rendertarget_ ints/src/ main.cc: 49
0x0000555555de1828 in std::equal_
at /usr/include/
356 { return __x == __y; }
(gdb) bt
#0 0x0000555555de1828 in std::equal_
at /usr/include/
#1 0x00005555563b0eab in std::__
#2 0x00005555563b0b0a in std::__
this=
#3 0x00005555563b0687 in std::_Hashtable
at /usr/include/
#4 0x00005555563b0072 in std::_Hashtable
at /usr/include/
#5 0x00005555563af71e in std::__
#6 0x00005555563aefe5 in std::unordered_
#7 0x00005555563ae47e in Gl::State::do_bind (this=0x555556a
at /widelands/
#8 0x00005555563ae3b6 in Gl::State::bind (this=0x555556a
at /widelands/
#9 0x000055555636751d in BlitProgram::draw (this=0x555556a
at /widelands/
#10 0x0000555556222689 in RenderQueue:
at /widelands/
#11 0x0000555556222585 in RenderQueue::draw (this=0x555556a
at /widelands/
#12 0x0000555555e7f71a in Graphic::refresh (this=0x555556b
#13 0x000055555607e130 in UI::Panel::do_run (this=0x55555f5
#14 0x0000555555dda8a8 in UI::Panel:
#15 0x0000555555eea120 in Widelands:
prefix_
#16 0x0000555555dd4dae in WLApplication:
#17 0x0000555555dd4368 in WLApplication:
#18 0x0000555555dd3d5f in WLApplication:
#19 0x0000555555dd0758 in WLApplication::run (this=0x555556b
#20 0x0000555555dced0d in main (argc=1, argv=0x7fffffff