[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1831583-fri02 into lp:widelands
Continuous integration builds have changed state: Travis build 5152. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/541383420. Appveyor build 4934. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1831583_fri02-4934. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1831583-fri02/+merge/368348 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1831583-fri02 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-1826744-lobby-commands into lp:widelands
Continuous integration builds have changed state: Travis build 5151. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/541357325. Appveyor build 4933. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1826744_lobby_commands-4933. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826744-lobby-commands/+merge/368285 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1826744-lobby-commands 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/reed-compatibility into lp:widelands
So ... you created a diff of 800 lines by passing objects around so you can
call a method in only a handful of places?? I like it! :)
The changes are mostly looking good to me. Replacing the GOTO might be broken,
though, see my diff comment below.
I haven't tested the changes, mostly because I have no sensible idea how to do
so. But based on the code I guess it shouldn't break anything.
Diff comments:
>
> === modified file 'src/map_io/map_buildingdata_packet.cc'
> --- src/map_io/map_buildingdata_packet.cc 2019-05-18 11:58:43 +
> +++ src/map_io/map_buildingdata_packet.cc 2019-05-26 03:34:22 +
> @@ -615,14 +618,15 @@
> if
> (worker_descr.can_act_as(working_position.first)) {
> while (wp->worker ||
> wp->worker_request) {
> ++wp;
> - if (!--count)
> - goto
> end_working_position;
> + if (!--count) {
> + continue;
> + }
While I appreciate getting rid of GOTO, I am not sure if the logic change here
is intentional. I think the continue will influence the while() loop while I
guess that you want to influence the for() loop?
A possible replacement would be setting a flag and break-ing within the
while(), then afterwards check for the flag, resetting it and using continue.
> }
> found_working_position = true;
> break;
> - } else
> + } else {
> wp += count;
> - end_working_position:;
> + }
> }
>
> if (!found_working_position)
>
> === modified file 'src/map_io/map_flagdata_packet.h'
> --- src/map_io/map_flagdata_packet.h 2019-02-23 11:00:49 +
> +++ src/map_io/map_flagdata_packet.h 2019-05-26 03:34:22 +
> @@ -21,7 +21,14 @@
> #define WL_MAP_IO_MAP_FLAGDATA_PACKET_H
>
> #include "map_io/map_data_packet.h"
> +#include "map_io/tribes_legacy_lookup_table.h"
>
> -MAP_DATA_PACKET(MapFlagdataPacket)
> +namespace Widelands {
> +class MapFlagdataPacket {
> +public:
> + void read(FileSystem&, EditorGameBase&, bool, MapObjectLoader&, const
> TribesLegacyLookupTable& tribes_lookup_table);
> + void write(FileSystem&, EditorGameBase&, MapObjectSaver&);
> +};
> +}
Maybe provide a second macro for the extended version? Just an idea, this way
is okay as well.
>
> #endif // end of include guard: WL_MAP_IO_MAP_FLAGDATA_PACKET_H
--
https://code.launchpad.net/~widelands-dev/widelands/reed-compatibility/+merge/367935
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/reed-compatibility 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/workarea-fixes into lp:widelands
Continuous integration builds have changed state: Travis build 5150. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/541329085. Appveyor build 4932. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_workarea_fixes-4932. -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368342 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/workarea-fixes 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-1831583-fri02 into lp:widelands
Benedikt Straub has proposed merging lp:~widelands-dev/widelands/bug-1831583-fri02 into lp:widelands. Commit message: Add some sleep time to costly loops in fri02 to prevent freezing Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1831583 in widelands: "Crash in Frisians 2" https://bugs.launchpad.net/widelands/+bug/1831583 For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1831583-fri02/+merge/368348 This is a Lua problem. gdb shows that masses of new Lua objects are created, and Lua doesn´t care to release the memory again. Hopefully, some additional sleep() will give the game and the garbage collector time to catch up. I don´t want to fix this by increasing the base sleep time or the objective will be marked done only after an enormous delay. Another way to fix the bug while perhaps improving performance would be to move the checks to C++, but I feel bad about having such specialised functions there. -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1831583-fri02 into lp:widelands. === modified file 'data/campaigns/fri02.wmf/scripting/mission_thread.lua' --- data/campaigns/fri02.wmf/scripting/mission_thread.lua 2019-03-29 18:29:54 + +++ data/campaigns/fri02.wmf/scripting/mission_thread.lua 2019-06-04 18:05:55 + @@ -33,6 +33,7 @@ function check_empire() while true do + sleep(1000) for idx,field in ipairs(all_fields) do sleep(5) local p1c = false @@ -272,9 +273,9 @@ while not p3.defeated do sleep(4513) end set_objective_done(o) - -- If the barbarians already defeated Murilius – well done + -- If the barbarians already defeated Murilius – well done. -- Otherwise, Murilius provokes Reebaud into ordering the player to conquer his entire colony - -- (merely defeating the Empire isn’t enough) + -- (merely defeating the Empire isn’t enough). -- We don't bother to check water, walkable-only and other useless terrains. -- That would be really too much to ask from our poor player, now wouldn't it? if not p2.defeated then @@ -285,6 +286,7 @@ o = add_campaign_objective(obj_defeat_murilius) local def = false while not def do + sleep(1000) def = true for idx,field in ipairs(useful_fields) do if field.owner == p2 then ___ 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-1826744-lobby-commands into lp:widelands
Ah, okay. Now it makes sense. :) The /help command visible in the diff is within an if-block that only triggers for superusers. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826744-lobby-commands/+merge/368285 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1826744-lobby-commands 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/workarea-fixes into lp:widelands
I'm still in favor of deducing these in the engine so that we won't need to maintain these Lua definitions, which will be prone to bugs. Criteria could be: 1. If both buildings mine the same resource 2. If both buildings plant an immovable 3. If both buildings collect an immovable 4. If one building plants an immovable and the other collects it and it is the same attrib 5. If both buildings plant or collect the same bob attrib If we want to go this route, we will need to base it off https://code.launchpad.net/~widelands-dev/widelands/unify-program-parsers/+merge/367936 in order not to get merge conflicts. -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368342 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/workarea-fixes 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/workarea-fixes into lp:widelands
Benedikt Straub has proposed merging lp:~widelands-dev/widelands/workarea-fixes
into lp:widelands.
Commit message:
– Highlight the buildings the overlapping workareas belong to
– Fix behaviour of the W hotkey
– Show productionsite overlaps only for certain building types of interest
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1830345 in widelands: "overlapping workareas are very performance hungry"
https://bugs.launchpad.net/widelands/+bug/1830345
Bug #1830647 in widelands: "Indicate workarea border more clearly"
https://bugs.launchpad.net/widelands/+bug/1830647
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368342
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/workarea-fixes into lp:widelands.
=== modified file 'data/tribes/buildings/productionsites/atlanteans/armorsmithy/init.lua'
--- data/tribes/buildings/productionsites/atlanteans/armorsmithy/init.lua 2018-09-12 03:04:08 +
+++ data/tribes/buildings/productionsites/atlanteans/armorsmithy/init.lua 2019-06-04 16:05:14 +
@@ -55,6 +55,13 @@
--done, including any animations and sounds played.
--See :doc:`productionsite_program`.
--
+--**indicate_workarea_overlaps**
+--*Optional*. The names of other productionsites whose workareas should be highlighted
+--if theirs overlap with this building’s workarea while the player is placing a
+--building of this type e.g.::
+--
+--indicate_workarea_overlaps = { "atlanteans_farm", "atlanteans_blackroot_farm" },
+--
--**out_of_resource_notification**.
--*Optional*. This table defines the message sent by the productionsite
--to the player if it has run out of a resource to collect. There are 4
=== modified file 'data/tribes/buildings/productionsites/atlanteans/blackroot_farm/init.lua'
--- data/tribes/buildings/productionsites/atlanteans/blackroot_farm/init.lua 2019-05-31 12:56:36 +
+++ data/tribes/buildings/productionsites/atlanteans/blackroot_farm/init.lua 2019-06-04 16:05:14 +
@@ -42,6 +42,12 @@
"blackroot"
},
+ indicate_workarea_overlaps = {
+ "atlanteans_blackroot_farm",
+ "atlanteans_farm",
+ "atlanteans_foresters_house",
+ },
+
programs = {
work = {
-- TRANSLATORS: Completed/Skipped/Did not start working because ...
=== modified file 'data/tribes/buildings/productionsites/atlanteans/coalmine/init.lua'
--- data/tribes/buildings/productionsites/atlanteans/coalmine/init.lua 2019-05-29 18:29:28 +
+++ data/tribes/buildings/productionsites/atlanteans/coalmine/init.lua 2019-06-04 16:05:14 +
@@ -43,6 +43,10 @@
atlanteans_miner = 3
},
+ indicate_workarea_overlaps = {
+ "atlanteans_coalmine",
+ },
+
inputs = {
{ name = "smoked_fish", amount = 10 },
{ name = "smoked_meat", amount = 6 },
=== modified file 'data/tribes/buildings/productionsites/atlanteans/crystalmine/init.lua'
--- data/tribes/buildings/productionsites/atlanteans/crystalmine/init.lua 2019-05-29 18:29:28 +
+++ data/tribes/buildings/productionsites/atlanteans/crystalmine/init.lua 2019-06-04 16:05:14 +
@@ -55,6 +55,10 @@
"granite"
},
+ indicate_workarea_overlaps = {
+ "atlanteans_crystalmine",
+ },
+
programs = {
work = {
-- TRANSLATORS: Completed/Skipped/Did not start working because ...
=== modified file 'data/tribes/buildings/productionsites/atlanteans/farm/init.lua'
--- data/tribes/buildings/productionsites/atlanteans/farm/init.lua 2019-05-31 12:56:36 +
+++ data/tribes/buildings/productionsites/atlanteans/farm/init.lua 2019-06-04 16:05:14 +
@@ -46,6 +46,12 @@
"corn"
},
+ indicate_workarea_overlaps = {
+ "atlanteans_blackroot_farm",
+ "atlanteans_farm",
+ "atlanteans_foresters_house",
+ },
+
programs = {
work = {
-- TRANSLATORS: Completed/Skipped/Did not start working because ...
=== modified file 'data/tribes/buildings/productionsites/atlanteans/fishbreeders_house/init.lua'
--- data/tribes/buildings/productionsites/atlanteans/fishbreeders_house/init.lua 2019-05-29 18:29:28 +
+++ data/tribes/buildings/productionsites/atlanteans/fishbreeders_house/init.lua 2019-06-04 16:05:14 +
@@ -47,6 +47,11 @@
},
},
+ indicate_workarea_overlaps = {
+ "atlanteans_fishers_house",
+ "atlanteans_fishbreeders_house",
+ },
+
out_of_resource_notification = {
-- Translators: Short for "Out of ..." for a resource
title = _"No Fish",
=== modified file 'data/tribes/buildings/productionsites/atlanteans/fishers_house/init.lua'
--- data/tribes/buildings/productionsites/atlanteans/fishers_house/init.lua 2019-06-01 11:23:15 +
+++ data/tribes/buildings/productionsites/atlanteans/fishers_house/init.lua 2019-06-04 16:05:14 +
@@ -41,6 +41,11 @@
"fish"
},
+
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/unify_sleep_time into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/unify_sleep_time into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/unify_sleep_time/+merge/368086 -- 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/unify_sleep_time into lp:widelands
Let's have it then :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/unify_sleep_time/+merge/368086 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/bug-1826744-lobby-commands into lp:widelands
Sorry, I was under the impression that users could use the help command as well. The it is just /announcement command that needs to be changed :) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826744-lobby-commands/+merge/368285 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1826744-lobby-commands 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_context_selection into lp:widelands
Will do next weekend :-) -- https://code.launchpad.net/~widelands-dev/widelands/fix_context_selection/+merge/368238 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_context_selection. ___ 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-1826744-lobby-commands into lp:widelands
I can rename /announce, no problem. But what is /help supposed to do for normal users? They only have /me as a "command". And the /help command for superusers is already part of this branch, or do you want something different there? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1826744-lobby-commands/+merge/368285 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1826744-lobby-commands 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
