Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/Economy_tutorial_fix into lp:widelands
Have you checked how it looks like in the Game. Li_image with text looks only good with short texts in my eyes. If not I will test on Tuesday and merge then -- https://code.launchpad.net/~widelands-dev/widelands/Economy_tutorial_fix/+merge/343770 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/Economy_tutorial_fix. ___ 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/ai_too_many_prodsites2 into lp:widelands
TiborB has proposed merging lp:~widelands-dev/widelands/ai_too_many_prodsites2
into lp:widelands.
Commit message:
AI change - ware preciousness is not capped to 25, or if it is returned as
kInvalidWare to 1.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/ai_too_many_prodsites2/+merge/344802
AI change - ware preciousness is now capped to 25, or if it is returned as
kInvalidWare to 1.
The logic is that if author omits preciousness in init.lua file, probably this
is not an important ware and might be set to 1.
Also too big differences in ware preciousness are not good for AI...
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/ai_too_many_prodsites2 into lp:widelands.
=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc 2018-04-08 22:33:43 +
+++ src/ai/defaultai.cc 2018-04-28 19:24:43 +
@@ -70,6 +70,8 @@
constexpr uint16_t kTargetQuantCap = 30;
+constexpr uint8_t kPreciousnessCap = 25;
+
// this is intended for map developers & testers, should be off by default
constexpr bool kPrintStats = false;
@@ -2544,8 +2546,8 @@
bo.new_building == BuildingNecessity::kNeeded);
if (ai_training_mode_ && bo.type == BuildingObserver::Type::kProductionsite &&
(gametime % 20 == 0 || log_needed)) {
- log("%2d: %-35s(%2d now) %-11s: max prec: %2d/%2d, primary priority: %4d, overdue: %3d\n",
- player_number(), bo.name, bo.total_count(), (log_needed) ? "needed" : "not needed",
+ log("%2d: %-35s(%2d now, prod: %3d) %-11s: max prec: %2d/%2d, primary priority: %4d, overdue: %3d\n",
+ player_number(), bo.name, bo.total_count(), bo.current_stats, (log_needed) ? "needed" : "not needed",
bo.max_needed_preciousness, bo.max_preciousness, bo.primary_priority,
bo.new_building_overdue);
}
@@ -4838,10 +4840,19 @@
// at least 1
target = std::max(target, 1);
- // it seems there are wares with 0 preciousness (no entry in init files?), but we need
- // positive value here.
- const uint16_t preciousness =
- std::max(wares.at(bo.outputs.at(m)).preciousness, 1);
+ // We need to have value in range 1 - kPreciousnessCap. If ware preciousness is not set in init.lua
+ // it gets 1 as default. otherwise it is just capped to kPreciousnessCap
+ const uint16_t preciousness = (wares.at(bo.outputs.at(m)).preciousness == kInvalidWare) ? 1 :
+ std::min(std::max(wares.at(bo.outputs.at(m)).preciousness, 1), kPreciousnessCap);
+ if (wares.at(bo.outputs.at(m)).preciousness > kPreciousnessCap) {
+log ("AI WARNING: Preciousness for %s (%s) received as: %d%s, capping to %d.%s\n",
+ tribe_->get_ware_descr(wt)->descname().c_str(),
+ tribe_->name().c_str(),
+ wares.at(bo.outputs.at(m)).preciousness,
+ (wares.at(bo.outputs.at(m)).preciousness == kInvalidWare) ? " (kInvalidWare)" : "",
+ preciousness,
+ (wares.at(bo.outputs.at(m)).preciousness == kInvalidWare) ? " Perhaps not set in init.lua" : " Value in init.lua too high.");
+ }
if (calculate_stocklevel(wt) < target ||
site_needed_for_economy == BasicEconomyBuildingStatus::kEncouraged) {
@@ -4857,6 +4868,8 @@
if (bo.max_preciousness < preciousness) {
bo.max_preciousness = preciousness;
}
+ assert(bo.max_preciousness <= kPreciousnessCap);
+ assert(bo.max_needed_preciousness <= kPreciousnessCap);
}
}
___
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/Economy_tutorial_fix into lp:widelands
Review: Approve I have done some edits - please proofread my changes. I also noticed that the objective titles had . at the end of them, and I fixed those too. Merge if you're happy with my changes. -- https://code.launchpad.net/~widelands-dev/widelands/Economy_tutorial_fix/+merge/343770 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/Economy_tutorial_fix. ___ 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-1358880-ship-statistics into lp:widelands
Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways. Travis build 3386. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/367231513. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ships_optr. ___ 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-1358880-ship-statistics into lp:widelands
Thanks for the review and testing again! I am still tweaking my IDE's layout function as best as I can. bunnybot will take care of the misaligned assert :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ships_optr. ___ 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/compiler_warnings_201804 into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/compiler_warnings_201804 into lp:widelands.
Commit message:
Fixed some compiler warnings for GCC and Clang.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/compiler_warnings_201804/+merge/344800
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/compiler_warnings_201804 into lp:widelands.
=== modified file 'src/ai/ai_help_structs.h'
--- src/ai/ai_help_structs.h 2018-04-08 22:33:43 +
+++ src/ai/ai_help_structs.h 2018-04-28 14:48:46 +
@@ -267,7 +267,7 @@
struct FindNodeAcceptAll {
bool accept(const Map&, FCoords) const {
return true;
- };
+ }
};
struct NearFlag {
=== modified file 'src/logic/campaign_visibility.cc'
--- src/logic/campaign_visibility.cc 2018-04-07 16:59:00 +
+++ src/logic/campaign_visibility.cc 2018-04-28 14:48:46 +
@@ -78,8 +78,8 @@
// Variable declaration
int32_t i = 0;
int32_t imap = 0;
- char csection[12];
- char number[4];
+ char csection[24];
+ char number[12];
std::string mapsection;
std::string cms;
@@ -98,8 +98,8 @@
{
Section& vis = campvisw.pull_section("campaigns");
sprintf(csection, "campsect%i", i);
- char cvisible[12];
- char cnewvisi[12];
+ char cvisible[24];
+ char cnewvisi[24];
while (cconf_s.get_string(csection)) {
sprintf(cvisible, "campvisi%i", i);
sprintf(cnewvisi, "cnewvisi%i", i);
=== modified file 'src/map_io/map_players_view_packet.cc'
--- src/map_io/map_players_view_packet.cc 2018-04-07 16:59:00 +
+++ src/map_io/map_players_view_packet.cc 2018-04-28 14:48:46 +
@@ -81,7 +81,7 @@
constexpr uint8_t kCurrentPacketVersionBorder = 1;
#define BORDER_FILENAME_TEMPLATE DIRNAME_TEMPLATE "/border_%u"
-#define FILENAME_SIZE 48
+#define FILENAME_SIZE 64
enum {
UNSEEN_NONE = 0,
=== modified file 'src/network/internet_gaming.cc'
--- src/network/internet_gaming.cc 2018-04-07 16:59:00 +
+++ src/network/internet_gaming.cc 2018-04-28 14:48:46 +
@@ -402,7 +402,7 @@
// Our work is done
return;
}
- } catch (const std::exception& e) {
+ } catch (const std::exception&) {
log("InternetGaming: Error when trying to transmit secondary IP.\n");
return;
}
=== modified file 'src/wui/interactive_base.cc'
--- src/wui/interactive_base.cc 2018-04-27 13:19:15 +
+++ src/wui/interactive_base.cc 2018-04-28 14:48:46 +
@@ -284,8 +284,8 @@
toolbar_.add(button);
if (window) {
window->opened.connect(
- [this, button] { button->set_style(UI::Button::Style::kPermpressed); });
- window->closed.connect([this, button] { button->set_style(UI::Button::Style::kRaised); });
+ [button] { button->set_style(UI::Button::Style::kPermpressed); });
+ window->closed.connect([button] { button->set_style(UI::Button::Style::kRaised); });
if (bind_default_toggle) {
button->sigclicked.connect(
___
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-1358880-ship-statistics into lp:widelands
Review: Approve Thanks for the fixes/changes! Testing and reviewing the commits went fine. One really minor nit: Some indentations are now not correctly aligned (some assert() somewhere). ;) Unfortunately I don't know any steps to reproduce the memory leak. It didn't seem to turn up every time when I tested it. But the changes look good and I played around with the window and wasn't able to trigger any further ASAN complains, so I guess its fine now. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ships_optr. ___ 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/ai_too_many_prodsites into lp:widelands
Something is wrong here - the branch is empty -- https://code.launchpad.net/~widelands-dev/widelands/ai_too_many_prodsites/+merge/344789 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/ai_too_many_prodsites 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-1358880-ship-statistics into lp:widelands
Review: Resubmit I have now fixed everything except maybe for this one: - Memory leak with create_shipinfo I thought I had caught that one already, but I have worked on the function just in case - it now returns a unique_ptr. That function had also produced a compiler warning which I fixed (yay for upgrading my Ubuntu). Do you have steps to reproduce the memory leak? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ships_optr. ___ 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-1358880-ship-statistics into lp:widelands
I guess I would have called it "Ship List" or something like that. But consistency is a good argument, lets leave it as it is. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ships_optr. ___ 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-1623375-multiplayer-starting-view into lp:widelands
Indeed, spectators are working. Guess I mixed something up. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1623375-multiplayer-starting-view/+merge/343287 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1623375-multiplayer-starting-view. ___ 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/productivity-string-fixes into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/productivity-string-fixes into lp:widelands.
Commit message:
Some string fixes for the Frisian helptexts
- Got rid of empty translation strings
- Added 'on average' to productivity helptexts, to be consistent with the other
tribes
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/productivity-string-fixes/+merge/344796
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/productivity-string-fixes into lp:widelands.
=== modified file 'data/tribes/buildings/militarysites/frisians/fortress/helptexts.lua'
--- data/tribes/buildings/militarysites/frisians/fortress/helptexts.lua 2018-01-22 13:40:13 +
+++ data/tribes/buildings/militarysites/frisians/fortress/helptexts.lua 2018-04-28 06:28:06 +
@@ -1,11 +1,14 @@
+-- This include can be removed when all help texts have been defined.
+include "tribes/scripting/help/global_helptexts.lua"
+
function building_helptext_lore ()
- -- TRANSLATORS: Lore helptext for a building
- return pgettext ("frisians_building", "")
+ -- TRANSLATORS#: Lore helptext for a building
+ return no_lore_text_yet()
end
function building_helptext_lore_author ()
- -- TRANSLATORS: Lore author helptext for a building
- return pgettext ("frisians_building", "")
+ -- TRANSLATORS#: Lore author helptext for a building
+ return no_lore_author_text_yet()
end
function building_helptext_purpose()
=== modified file 'data/tribes/buildings/militarysites/frisians/outpost/helptexts.lua'
--- data/tribes/buildings/militarysites/frisians/outpost/helptexts.lua 2018-01-22 13:40:13 +
+++ data/tribes/buildings/militarysites/frisians/outpost/helptexts.lua 2018-04-28 06:28:06 +
@@ -1,11 +1,14 @@
+-- This include can be removed when all help texts have been defined.
+include "tribes/scripting/help/global_helptexts.lua"
+
function building_helptext_lore ()
- -- TRANSLATORS: Lore helptext for a building
- return pgettext ("frisians_building", "")
+ -- TRANSLATORS#: Lore helptext for a building
+ return no_lore_text_yet()
end
function building_helptext_lore_author ()
- -- TRANSLATORS: Lore author helptext for a building
- return pgettext ("frisians_building", "")
+ -- TRANSLATORS#: Lore author helptext for a building
+ return no_lore_author_text_yet()
end
function building_helptext_purpose()
=== modified file 'data/tribes/buildings/militarysites/frisians/sentinel/helptexts.lua'
--- data/tribes/buildings/militarysites/frisians/sentinel/helptexts.lua 2018-01-22 13:40:13 +
+++ data/tribes/buildings/militarysites/frisians/sentinel/helptexts.lua 2018-04-28 06:28:06 +
@@ -1,11 +1,14 @@
+-- This include can be removed when all help texts have been defined.
+include "tribes/scripting/help/global_helptexts.lua"
+
function building_helptext_lore ()
- -- TRANSLATORS: Lore helptext for a building
- return pgettext ("frisians_building", "")
+ -- TRANSLATORS#: Lore helptext for a building
+ return no_lore_text_yet()
end
function building_helptext_lore_author ()
- -- TRANSLATORS: Lore author helptext for a building
- return pgettext ("frisians_building", "")
+ -- TRANSLATORS#: Lore author helptext for a building
+ return no_lore_author_text_yet()
end
function building_helptext_purpose()
=== modified file 'data/tribes/buildings/militarysites/frisians/tower/helptexts.lua'
--- data/tribes/buildings/militarysites/frisians/tower/helptexts.lua 2018-01-22 13:40:13 +
+++ data/tribes/buildings/militarysites/frisians/tower/helptexts.lua 2018-04-28 06:28:06 +
@@ -1,11 +1,14 @@
+-- This include can be removed when all help texts have been defined.
+include "tribes/scripting/help/global_helptexts.lua"
+
function building_helptext_lore ()
- -- TRANSLATORS: Lore helptext for a building
- return pgettext ("frisians_building", "")
+ -- TRANSLATORS#: Lore helptext for a building
+ return no_lore_text_yet()
end
function building_helptext_lore_author ()
- -- TRANSLATORS: Lore author helptext for a building
- return pgettext ("frisians_building", "")
+ -- TRANSLATORS#: Lore author helptext for a building
+ return no_lore_author_text_yet()
end
function building_helptext_purpose()
=== modified file 'data/tribes/buildings/militarysites/frisians/wooden_tower/helptexts.lua'
--- data/tribes/buildings/militarysites/frisians/wooden_tower/helptexts.lua 2018-01-22 13:40:13 +
+++ data/tribes/buildings/militarysites/frisians/wooden_tower/helptexts.lua 2018-04-28 06:28:06 +
@@ -1,11 +1,14 @@
+-- This include can be removed when all help texts have been defined.
+include "tribes/scripting/help/global_helptexts.lua"
+
function building_helptext_lore ()
- -- TRANSLATORS: Lore helptext for a building
- return pgettext ("frisians_building", "")
+ -- TRANSLATORS#: Lore helptext for a building
+ return no_lore_text_yet()
end
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1358880-ship-statistics into lp:widelands
Thanks for the review! Everything that I haven't added a comment to, I will
implement exactly as you suggested.
As to the name - I guess that's for consistency, just like we have the
"Building Statistics", which is also a mix of info and unit access. I am open
to suggestions though :)
Diff comments:
>
> === modified file 'src/logic/map_objects/tribes/ship.cc'
> --- src/logic/map_objects/tribes/ship.cc 2018-04-07 16:59:00 +
> +++ src/logic/map_objects/tribes/ship.cc 2018-04-16 15:39:30 +
> @@ -847,14 +849,14 @@
>pgettext("ship", "Expedition"), _("Expedition Ready"),
>_("An expedition ship is waiting for your commands."),
>"images/wui/buildings/start_expedition.png");
> - Notifications::publish(NoteShipMessage(this,
> NoteShipMessage::Message::kWaitingForCommand));
> + Notifications::publish(NoteShip(this,
> NoteShip::Action::kWaitingForCommand));
I think there's a reason that I set the state at the top of the function and
notify on the bottom. I implemented this a ear ago, so I don't remember the
particulars. I dimly remember that there was a problem somewhere that made me
split this in some instances. I have taken a note though and will experiment
and ad a comment if the separation is indeed needed here.
> }
>
> /// Initializes / changes the direction of scouting to @arg direction
> /// @note only called via player command
> void Ship::exp_scouting_direction(Game&, WalkingDir scouting_direction) {
> assert(expedition_);
> - ship_state_ = ShipStates::kExpeditionScouting;
> + set_ship_state_and_notify(ShipStates::kExpeditionScouting,
> NoteShip::Action::kDestinationChanged);
> expedition_->scouting_direction = scouting_direction;
> expedition_->island_exploration = false;
> }
>
> === modified file 'src/logic/player.cc'
> --- src/logic/player.cc 2018-04-07 16:59:00 +
> +++ src/logic/player.cc 2018-04-16 15:39:30 +
> @@ -372,6 +373,18 @@
> game->cmdqueue().enqueue(new CmdDeleteMessage(game->get_gametime(),
> player_number_, message_id));
> }
>
> +const std::set& Player::ships() const {
> + return ships_;
> +}
> +void Player::add_ship(Serial ship) {
> + ships_.insert(ship);
> +}
> +void Player::remove_ship(Serial ship) {
> + if (ships_.count(ship) == 1) {
> + ships_.erase(ship);
> + }
It would, the complexity of find() is logarithmic. So, it does matter :)
> +}
> +
> /*
> ===
> Return filtered buildcaps that take the player's territory into account.
>
> === modified file 'src/scripting/lua_game.cc'
> --- src/scripting/lua_game.cc 2018-04-07 16:59:00 +
> +++ src/scripting/lua_game.cc 2018-04-16 15:39:30 +
> @@ -672,30 +672,16 @@
> */
> int LuaPlayer::get_ships(lua_State* L) {
> EditorGameBase& egbase = get_egbase(L);
> - const Map& map = egbase.map();
> PlayerNumber p = (get(L, egbase)).player_number();
> lua_newtable(L);
> uint32_t cidx = 1;
> -
> - std::set found_ships;
> - for (int16_t y = 0; y < map.get_height(); ++y) {
> - for (int16_t x = 0; x < map.get_width(); ++x) {
ROFL yep. Spending a bit of memory to keep track of them is certainly a lot
easier to deal with!
> - FCoords f = map.get_fcoords(Coords(x, y));
> - // there are too many bobs on the map so we investigate
> - // only bobs on water
> - if (f.field->nodecaps() & MOVECAPS_SWIM) {
> - for (Bob* bob = f.field->get_first_bob(); bob;
> bob = bob->get_next_on_field()) {
> - if (upcast(Ship, ship, bob)) {
> - if
> (ship->get_owner()->player_number() == p && !found_ships.count(ship)) {
> -
> found_ships.insert(ship);
> - lua_pushuint32(L,
> cidx++);
> -
> LuaMaps::upcasted_map_object_to_lua(L, ship);
> - lua_rawset(L, -3);
> - }
> - }
> - }
> - }
> - }
> + for (const auto& serial : egbase.player(p).ships()) {
> + Widelands::MapObject* obj = egbase.objects().get_object(serial);
> + assert(obj->descr().type() == Widelands::MapObjectType::SHIP);
> + upcast(Widelands::Ship, ship, obj);
> + lua_pushuint32(L, cidx++);
> + LuaMaps::upcasted_map_object_to_lua(L, ship);
> + lua_rawset(L, -3);
> }
> return 1;
> }
>
> === modified file 'src/wui/interactive_player.cc'
> --- src/wui/interactive_player.cc 2018-04-07 16:59:00 +
> +++ src/wui/interactive_player.cc
