[Widelands-dev] [Merge] lp:~widelands-dev/widelands/save_refactor into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/save_refactor into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/save_refactor/+merge/322622 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/save_refactor. ___ 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-1678598-expedition-no-eco-crash into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash into
lp:widelands.
Commit message:
Only cancel expedition if there is a reachable portdock.
- Reset the expedition state to "Waiting" if no portdock could be found
- Show a warning message to owner
- AI will continue exploring
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1678598 in widelands: "END OF EXPEDITION due to time-out schip.cc
assertion failed "
https://bugs.launchpad.net/widelands/+bug/1678598
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash/+merge/323004
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash into
lp:widelands.
=== modified file 'src/ai/defaultai_seafaring.cc'
--- src/ai/defaultai_seafaring.cc 2017-01-28 14:53:28 +
+++ src/ai/defaultai_seafaring.cc 2017-04-23 12:22:45 +
@@ -19,6 +19,8 @@
#include "ai/defaultai.h"
+#include "economy/fleet.h"
+
using namespace Widelands;
// this scores spot for potential colony
@@ -358,6 +360,14 @@
log("%d: %s at %3dx%3d: END OF EXPEDITION due to time-out\n", pn,
so.ship->get_shipname().c_str(), so.ship->get_position().x, so.ship->get_position().y);
+ // In case there is no port left to get back to, continue exploring
+ if (!so.ship->get_fleet() || !so.ship->get_fleet()->has_ports()) {
+ log("%d: %s at %3dx%3d: END OF EXPEDITION without port, continue exploring\n", pn,
+ so.ship->get_shipname().c_str(), so.ship->get_position().x, so.ship->get_position().y);
+ persistent_data->expedition_start_time = gametime;
+ return;
+ }
+
// For known and running expedition
} else {
// set persistent_data->colony_scan_area based on elapsed expedition time
=== modified file 'src/economy/flag.cc'
--- src/economy/flag.cc 2017-01-25 18:55:59 +
+++ src/economy/flag.cc 2017-04-23 12:22:45 +
@@ -652,12 +652,13 @@
always_call_for_flag_ = nullptr;
}
-void Flag::init(EditorGameBase& egbase) {
+bool Flag::init(EditorGameBase& egbase) {
PlayerImmovable::init(egbase);
set_position(egbase, position_);
animstart_ = egbase.get_gametime();
+ return true;
}
/**
=== modified file 'src/economy/flag.h'
--- src/economy/flag.h 2017-01-25 18:55:59 +
+++ src/economy/flag.h 2017-04-23 12:22:45 +
@@ -142,7 +142,7 @@
void log_general_info(const EditorGameBase&) override;
protected:
- void init(EditorGameBase&) override;
+ bool init(EditorGameBase&) override;
void cleanup(EditorGameBase&) override;
void draw(uint32_t gametime,
=== modified file 'src/economy/fleet.cc'
--- src/economy/fleet.cc 2017-01-25 18:55:59 +
+++ src/economy/fleet.cc 2017-04-23 12:22:45 +
@@ -93,19 +93,22 @@
* Initialize the fleet, including a search through the map
* to rejoin with the next other fleet we can find.
*/
-void Fleet::init(EditorGameBase& egbase) {
+bool Fleet::init(EditorGameBase& egbase) {
MapObject::init(egbase);
if (ships_.empty() && ports_.empty()) {
molog("Empty fleet initialized; disband immediately\n");
remove(egbase);
- return;
+ return false;
}
find_other_fleet(egbase);
- if (active())
+ if (active()) {
update(egbase);
+ return true;
+ }
+ return false;
}
struct StepEvalFindFleet {
@@ -564,6 +567,10 @@
}
}
+bool Fleet::has_ports() {
+ return !ports_.empty();
+}
+
/**
* Search among the docks of the fleet for the one that has the given flag as its base.
*
=== modified file 'src/economy/fleet.h'
--- src/economy/fleet.h 2017-01-25 18:55:59 +
+++ src/economy/fleet.h 2017-04-23 12:22:45 +
@@ -91,7 +91,7 @@
bool active() const;
- void init(EditorGameBase&) override;
+ bool init(EditorGameBase&) override;
void cleanup(EditorGameBase&) override;
void update(EditorGameBase&);
@@ -99,6 +99,7 @@
void remove_ship(EditorGameBase& egbase, Ship* ship);
void add_port(EditorGameBase& egbase, PortDock* port);
void remove_port(EditorGameBase& egbase, PortDock* port);
+ bool has_ports();
void log_general_info(const EditorGameBase&) override;
=== modified file 'src/economy/portdock.cc'
--- src/economy/portdock.cc 2017-02-22 20:23:49 +
+++ src/economy/portdock.cc 2017-04-23 12:22:45 +
@@ -144,7 +144,7 @@
// do nothing
}
-void PortDock::init(EditorGameBase& egbase) {
+bool PortDock::init(EditorGameBase& egbase) {
PlayerImmovable::init(egbase);
for (const Coords& coords : dockpoints_) {
@@ -152,6 +152,7 @@
}
init_fleet(egbase);
+ return true;
}
/**
=== modified file 'src/economy/portdock.h'
--- src/economy/portdock.h 2017-01-25 18:55:59 +
+++ src/economy/portdock.h 2017-04-23 12:22:45 +
@@ -102,7 +102,7 @@
float scale,
RenderTarget* dst) override;
- void init(EditorGameBase&) override;
+ bool init(EditorGameBase&) override;
void cleanup(EditorGameBase&) override;
void add_neighbours(
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash into lp:widelands
1st Code Review -- https://code.launchpad.net/~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash/+merge/323004 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash into lp:widelands has been updated. Commit Message changed to: Only cancel expedition if there is a reachable portdock. - Reset the expedition state to "Waiting" if no portdock could be found - Show a warning message to owner - AI will continue exploring - Added a boolean success semantics to MapObject::init() to achieve this. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash/+merge/323004 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash 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-1678598-expedition-no-eco-crash into lp:widelands
Review: Approve code revie, compile, test Original Nile crash now shows: 5: Azurea Sea at 101x100: END OF EXPEDITION without port, continue exploring 5: last command for ship Azurea Sea at 102x102 was 41518 seconds ago, something wrong here?... 5: Azurea Sea at 102x102: explore uphold, visited first time 5: Azurea Sea: continue island circumvention, dir=1 and the game doe not crash ok. Tried the game attached to 1678598 in trunk -> crashes when canceling the expedition without harbours, expected. Tried the same in this brach, got the expected message. -> Fine for me will try to finish that nile szenario now, using that branch. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash/+merge/323004 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash. ___ 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-1678598-expedition-no-eco-crash into lp:widelands
Continuous integration builds have changed state: Travis build 2114. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/224895585. Appveyor build 1949. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1678598_expedition_no_eco_crash-1949. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash/+merge/323004 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash. ___ 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-1678598-expedition-no-eco-crash into lp:widelands
You can ignore the something wrong here? bit. I am only resetting the expedition status, not giving any new commands to the Ai ship. So, the ship will be waiting until the AI issues the next round of commands. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash/+merge/323004 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1678598-expedition-no-eco-crash. ___ 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/rendertarget_ints into lp:widelands
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.
0x55de1828 in std::equal_to::operator()
(this=0x56ad91b8 , __x=@0x7fff9100:
27123, __y=)
at /usr/include/c++/6/bits/stl_function.h:356
356 { return __x == __y; }
(gdb) bt
#0 0x55de1828 in std::equal_to::operator()
(this=0x56ad91b8 , __x=@0x7fff9100:
27123, __y=)
at /usr/include/c++/6/bits/stl_function.h:356
#1 0x563b0eab in std::__detail::_Equal_helper, std::__detail::_Select1st,
std::equal_to, unsigned long, false>::_S_equals (__eq=...,
__extract=..., __k=@0x7fff9100: 27123, __n=0x608f5b40) at
/usr/include/c++/6/bits/hashtable_policy.h:1331
#2 0x563b0b0a in std::__detail::_Hashtable_base, std::__detail::_Select1st,
std::equal_to, std::hash,
std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash,
std::__detail::_Hashtable_traits >::_M_equals (
this=0x56ad91b8 ,
__k=@0x7fff9100: 27123, __c=27123, __n=0x608f5b40) at
/usr/include/c++/6/bits/hashtable_policy.h:1702
#3 0x563b0687 in std::_Hashtable, std::allocator >, std::__detail::_Select1st, std::equal_to,
std::hash, std::__detail::_Mod_range_hashing,
std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy,
std::__detail::_Hashtable_traits >::_M_find_before_node
(this=0x56ad91b8 , __n=27123,
__k=@0x7fff9100: 27123, __code=27123)
at /usr/include/c++/6/bits/hashtable.h:1420
#4 0x563b0072 in std::_Hashtable, std::allocator >, std::__detail::_Select1st, std::equal_to,
std::hash, std::__detail::_Mod_range_hashing,
std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy,
std::__detail::_Hashtable_traits >::_M_find_node
(this=0x56ad91b8 , __bkt=27123,
__key=@0x7fff9100: 27123, __c=27123)
at /usr/include/c++/6/bits/hashtable.h:634
#5 0x563af71e in std::__detail::_Map_base, std::allocator >, std::__detail::_Select1st, std::equal_to, std::hash, std::__detail::_Mod_range_hashing,
std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy,
std::__detail::_Hashtable_traits, true>::operator[]
(this=0x56ad91b8 , __k=@0x7fff9100:
27123) at /usr/include/c++/6/bits/hashtable_policy.h:591
#6 0x563aefe5 in std::unordered_map, std::equal_to,
std::allocator > >::operator[]
(this=0x56ad91b8 , __k=@0x7fff9100:
27123) at /usr/include/c++/6/bits/unordered_map.h:904
#7 0x563ae47e in Gl::State::do_bind (this=0x56ad9180
, target=33985, texture=27123)
at /widelands/rendertarget_ints/src/graphic/gl/utils.cc:196
#8 0x563ae3b6 in Gl::State::bind (this=0x56ad9180
, target=33985, texture=27123)
at /widelands/rendertarget_ints/src/graphic/gl/utils.cc:180
#9 0x5636751d in BlitProgram::draw (this=0x56ad8e60
, arguments=std::vector of length 416,
capacity 512 = {...})
at /widelands/rendertarget_ints/src/graphic/gl/blit_program.cc:166
#10 0x56222689 in RenderQueue::draw_items (this=0x56ad6a60
, items=std::vector of length 455,
capacity 1024 = {...})
at /widelands/rendertarget_ints/src/graphic/render_queue.cc:226
#11 0x56222585 in RenderQueue::draw (this=0x56ad6a60
, screen_width=1920, screen_height=1080)
at /widelands/rendertarget_ints/src/graphic/render_queue.cc:213
#12 0x55e7f71a in Graphic::refresh (this=0x56b44630) at
/widelands/rendertarget_ints/src/graphic/graphic.cc:206
#13 0x5607e130 in UI::Panel::do_run (this=0x5f50c240) at
/widelands/rendertarget_ints/src/ui_basic/panel.cc:197
#14 0x55dda8a8 in UI::Panel::run
(this=0x5f50c240) at ../src/ui_basic/panel.h:96
#15 0x55eea120 in Widelands::Game::run (this=0x7fff
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1512093-workers into lp:widelands
OK, let's merge this then :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1512093-workers/+merge/322981 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1512093-workers. ___ 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/savegame-menu into lp:widelands
Colors are reversed between the Fullscreen menus and the in.game menus on purpose. This is necessary because the background makes non-bold text unreadable in the Fullscreen menus - the coloring is consistent with the mapselect screen though. I used the arrow in the replay in an attempt to signal that the gametime is the start of the replay, not the end or length. Regarding the load/save screens, we have map descriptions in the load menu and actual filenames in the save menu. Do you thing that we need a button to toggle those like in the editor load/save map? H means Host, we are distinguishing host from client here. MP is reserved for multiplayer client games. How about MPH/MPC? And yes, the savegame menu is consistent with the editor menus, not the Fullscreen menus. I can look into changing the button positions. I don't see a natural cutoff point to make the editbox shorter that would be visually pleasing - ideas? -- https://code.launchpad.net/~widelands-dev/widelands/savegame-menu/+merge/322924 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/savegame-menu 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/rendertarget_ints into lp:widelands
Thanks for the review :) The crash looks unfamiliar, I'll have to look into this. > Is there a reason that Bob::calc_drawpos() (and derived methods) are still > returning floats? Yes, these are there for the zoom function, map elements can now end up not being positioned pixel perfect. > UI::correct_for_align() and UI::center_vertically() are somehow related from > what they are doing but are offering quite different interfaces. Not really. center_vertically() is omitting the align parameter, because align is always centered here. > correct_for_align(kCenter) is called but correct_for_align(kLeft) is omitted. I'll go over these again when we do the final RTL GUI switching code. -- https://code.launchpad.net/~widelands-dev/widelands/rendertarget_ints/+merge/322996 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/rendertarget_ints into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1512093-workers into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1512093-workers into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1512093-workers/+merge/322981 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1512093-workers. ___ 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/savegame-menu into lp:widelands
> Colors are reversed between the Fullscreen menus and the in.game menus on
> purpose. This is necessary because the background makes non-bold text
> unreadable in the Fullscreen menus
Can't follow you here. The Save game menu shows currently (in this branch) bold
white constants ('Map name:') with non bold yellow variables (e.g. "Crater").
What is against having bold yellow constants with bold white variables like it
is all other gameplay menus?
> - the coloring is consistent with the
> mapselect screen though.
You mean mapselect screen in Editor? I think this branch do not touch the menus
in editor. The Choose map screen for single player game shows also bold yellow
constants with bold white variables.
Maybe i am too fussy :)
> I used the arrow in the replay in an attempt to signal that the gametime is
> the start of the replay, not the end or length.
Ok, never associate it with the beginning of the replay... maybe has to be
clarified in the tooltip? Or, since the same information is also on the right
side, describe it it on the right side. So instead: 'Game Time: 20:44' ->
'Start of replay: 20:44'. One disadvantage when the game time is in the column:
Clicking on the column header sort the entrys by gametime then. Don't know if
gametime is a necessary information to sort for.
> Regarding the load/save
> screens, we have map descriptions in the load menu and actual filenames in the
> save menu. Do you thing that we need a button to toggle those like in the
> editor load/save map?
No, i don't think so. I had trouble to write what i meant. Another try:
The column description in Load save game menu contains for example:
1. The Strands of Malac'mor (Build blockhouse) -> Campaign(Objective)
2. test (Crater) -> filename (Mapname)
3. Autosave 00: Crater -> somename: Mapname
Especially the 2. and 3. example shows different formats of the same kind of
information. Having an extra string for autosave files is ok for me, but the
format should be equal in both entries, imho. To distinguish between campaigns
both entries may be in format 'name: mapname'. The resulting entries would then
be:
1. The Strands of Malac'mor (Build blockhouse)
2. test: Crater -> removed curved brackets
3. Autosave 00: Crater
> H means Host, we are distinguishing host from client here. MP is reserved for
> multiplayer client games. How about MPH/MPC?
I don't mind. For me all those abbreviations are not self explaining *lol*
> And yes, the savegame menu is consistent with the editor menus, not the
> Fullscreen menus. I can look into changing the button positions.
I think distinguishing between editor and playing menus, if feasible, would be
good, since editor and playing are different kind of programs.
> I don't see a natural cutoff point to make the editbox shorter that would be
> visually pleasing - ideas?
In the old save game menu the editbox was visual a part of the table. I liked
that...
--
https://code.launchpad.net/~widelands-dev/widelands/savegame-menu/+merge/322924
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/savegame-menu into lp:widelands.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1664052-expedition-shipwindow-crash into lp:widelands
Continuous integration builds have changed state: Travis build 2116. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/225038488. Appveyor build 1951. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1664052_expedition_shipwindow_crash-1951. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1664052-expedition-shipwindow-crash/+merge/318986 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1664052-expedition-shipwindow-crash. ___ 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
