Review: Resubmit All fixed.
Diff comments: > === modified file 'src/game_io/game_loader.cc' > --- src/game_io/game_loader.cc 2014-07-28 14:23:03 +0000 > +++ src/game_io/game_loader.cc 2014-09-11 17:46:54 +0000 > @@ -32,7 +32,6 @@ > #include "game_io/game_player_info_data_packet.h" > #include "game_io/game_preload_data_packet.h" > #include "io/filesystem/layered_filesystem.h" > -#include "logic/cmd_expire_message.h" > #include "logic/game.h" > #include "logic/player.h" > #include "map_io/widelands_map_map_object_loader.h" > @@ -103,13 +102,6 @@ > Message* m = temp_message.second; > Message_Id m_id = temp_message.first; > > - // Renew expire commands > - Duration const duration = m->duration(); > - if (duration != Forever()) { > - m_game.cmdqueue().enqueue > - (new Cmd_ExpireMessage > - (m->sent() + duration, p, > m_id)); > - } > // Renew MapObject connections > if (m->serial() > 0) { > MapObject* mo = > m_game.objects().get_object(m->serial()); > > === modified file 'src/logic/CMakeLists.txt' > --- src/logic/CMakeLists.txt 2014-07-28 17:12:07 +0000 > +++ src/logic/CMakeLists.txt 2014-09-11 17:46:54 +0000 > @@ -76,8 +76,8 @@ > checkstep.h > cmd_calculate_statistics.cc > cmd_calculate_statistics.h > - cmd_expire_message.cc > - cmd_expire_message.h > + cmd_delete_message.cc > + cmd_delete_message.h > cmd_incorporate.cc > cmd_incorporate.h > cmd_luacoroutine.cc > > === modified file 'src/logic/building.cc' > --- src/logic/building.cc 2014-08-01 12:57:17 +0000 > +++ src/logic/building.cc 2014-09-11 17:46:54 +0000 > @@ -910,8 +910,8 @@ > rt_description += "</p></rt>"; > > Message * msg = new Message > - (msgsender, game.get_gametime(), 60 * 60 * 1000, > - title, rt_description, get_position(), > (link_to_building_lifetime ? m_serial : 0)); > + (msgsender, game.get_gametime(), title, rt_description, > + get_position(), (link_to_building_lifetime ? m_serial : 0)); > > if (throttle_time) > owner().add_message_with_timeout > > === renamed file 'src/logic/cmd_expire_message.cc' => > 'src/logic/cmd_delete_message.cc' > --- src/logic/cmd_expire_message.cc 2013-07-26 20:19:36 +0000 > +++ src/logic/cmd_delete_message.cc 2014-09-11 17:46:54 +0000 > @@ -17,15 +17,15 @@ > * > */ > > -#include "logic/cmd_expire_message.h" > +#include "logic/cmd_delete_message.h" > > #include "logic/game.h" > #include "logic/player.h" > > namespace Widelands { > > -void Cmd_ExpireMessage::execute(Game & game) { > - game.player(player).messages().expire_message(message); > +void Cmd_DeleteMessage::execute(Game & game) { > + game.player(player).messages().delete_message(message); > } > > } > > === renamed file 'src/logic/cmd_expire_message.h' => > 'src/logic/cmd_delete_message.h' > --- src/logic/cmd_expire_message.h 2014-07-26 10:43:23 +0000 > +++ src/logic/cmd_delete_message.h 2014-09-11 17:46:54 +0000 > @@ -17,8 +17,8 @@ > * > */ > > -#ifndef WL_LOGIC_CMD_EXPIRE_MESSAGE_H > -#define WL_LOGIC_CMD_EXPIRE_MESSAGE_H > +#ifndef WL_LOGIC_CMD_DELETE_MESSAGE_H > +#define WL_LOGIC_CMD_DELETE_MESSAGE_H > > #include <memory> > > @@ -27,7 +27,7 @@ > > namespace Widelands { > > -/// Expires the player's message. > +/// Delete the player's message. > /// > /// \note This is not a GameLogicCommand because it should not be saved. Because when a building gets destroyed/dismantled, all messages for the building get deleted. > /// Instead, the commands are recreated separately on load (when both command > @@ -35,8 +35,8 @@ > /// command and then when loading, checking that one exists for each message > /// and if not, warn and recreate it. Such redundancy would also waste space > in > /// the savegame. > -struct Cmd_ExpireMessage : public Command { > - Cmd_ExpireMessage > +struct Cmd_DeleteMessage : public Command { > + Cmd_DeleteMessage > (int32_t const t, Player_Number const p, Message_Id const m) > : Command(t), player(p), message(m) > {} > @@ -51,4 +51,4 @@ > > } > > -#endif // end of include guard: WL_LOGIC_CMD_EXPIRE_MESSAGE_H > +#endif // end of include guard: WL_LOGIC_CMD_DELETE_MESSAGE_H > > === modified file 'src/logic/cmd_luacoroutine.cc' > --- src/logic/cmd_luacoroutine.cc 2014-07-28 14:17:07 +0000 > +++ src/logic/cmd_luacoroutine.cc 2014-09-11 17:46:54 +0000 > @@ -50,8 +50,7 @@ > for (int i = 1; i <= game.map().get_nrplayers(); i++) { > Widelands::Message & msg = > *new Widelands::Message > - ("Game Logic", game.get_gametime(), > - Forever(), "Lua Coroutine Failed", e.what()); > + ("Game Logic", game.get_gametime(), "Lua > Coroutine Failed", e.what()); > game.player(i).add_message(game, msg, true); > } > game.gameController()->setDesiredSpeed(0); > > === modified file 'src/logic/message.h' > --- src/logic/message.h 2014-07-28 14:23:03 +0000 > +++ src/logic/message.h 2014-09-11 17:46:54 +0000 > @@ -46,7 +46,6 @@ > Message > (const std::string & msgsender, > uint32_t sent_time, > - Duration d, > const std::string & t, > const std::string & b, > Widelands::Coords const c = Coords::Null(), > @@ -57,7 +56,6 @@ > m_title(t), > m_body (b), > m_sent (sent_time), > - m_duration(d), > m_position(c), > m_serial (ser), > m_status (s) > @@ -65,7 +63,6 @@ > > const std::string & sender() const {return m_sender;} > uint32_t sent () const {return m_sent;} > - Duration duration() const {return m_duration;} > const std::string & title() const {return m_title;} > const std::string & body () const {return m_body;} > Widelands::Coords position() const {return m_position;} > @@ -78,7 +75,6 @@ > std::string m_title; > std::string m_body; > uint32_t m_sent; > - Duration m_duration; /// will expire after this duration > Widelands::Coords m_position; > Widelands::Serial m_serial; // serial to map object > Status m_status; > > === modified file 'src/logic/message_queue.h' > --- src/logic/message_queue.h 2014-07-28 14:23:03 +0000 > +++ src/logic/message_queue.h 2014-09-11 17:46:54 +0000 > @@ -121,12 +121,11 @@ > > /// Expire the message with the given id so that it no longer exists. > /// Assumes that a message with the given id exists. > - void expire_message(const Message_Id& id) { > + void delete_message(const Message_Id& id) { > assert_counts(); > iterator const it = find(id); > if (it == end()) { > - // Messages can be expired when the timeout runs out, > or when the linked > - // MapObject is removed, or both. In this later case, > two expire commands > + // Messages can be expired when the linked MapObject is > removed. Two expire commands > // will be executed, and the message will not be > present for the second one. > // So we assume here that the message was removed from > an earlier expire cmd. > return; > > === modified file 'src/logic/player.cc' > --- src/logic/player.cc 2014-07-28 16:59:54 +0000 > +++ src/logic/player.cc 2014-09-11 17:46:54 +0000 > @@ -36,7 +36,7 @@ > #include "io/filewrite.h" > #include "logic/building.h" > #include "logic/checkstep.h" > -#include "logic/cmd_expire_message.h" > +#include "logic/cmd_delete_message.h" > #include "logic/cmd_luacoroutine.h" > #include "logic/constants.h" > #include "logic/constructionsite.h" > @@ -309,14 +309,7 @@ > Message_Id Player::add_message > (Game & game, Message & message, bool const popup) > { > - // Expire command > Message_Id id = messages().add_message(message); > - Duration const duration = message.duration(); > - if (duration != Forever()) { > - game.cmdqueue().enqueue > - (new Cmd_ExpireMessage > - (game.get_gametime() + duration, > player_number(), id)); > - } > > // MapObject connection > if (message.serial() > 0) { > @@ -359,14 +352,14 @@ > > void Player::message_object_removed(Message_Id m_id) const > { > - // Send expire command > + // Send delete command > upcast(Game, game, &m_egbase); > if (!game) { > return; > } > > game->cmdqueue().enqueue > - (new Cmd_ExpireMessage > + (new Cmd_DeleteMessage > (game->get_gametime(), m_plnum, m_id)); > } > > > === modified file 'src/logic/ship.cc' > --- src/logic/ship.cc 2014-07-28 16:59:54 +0000 > +++ src/logic/ship.cc 2014-09-11 17:46:54 +0000 > @@ -914,8 +914,7 @@ > rt_description += "</p></rt>"; > > Message * msg = new Message > - (msgsender, game.get_gametime(), 60 * 60 * 1000, > - title, rt_description, get_position(), m_serial); > + (msgsender, game.get_gametime(), title, rt_description, > get_position(), m_serial); > > get_owner()->add_message(game, *msg); > } > > === modified file 'src/logic/soldier.cc' > --- src/logic/soldier.cc 2014-07-28 16:59:54 +0000 > +++ src/logic/soldier.cc 2014-09-11 17:46:54 +0000 > @@ -1571,7 +1571,7 @@ > (game, > *new Message > ("game engine", > - game.get_gametime(), > Forever(), > + game.get_gametime(), > _("Logic error"), > buffer, > get_position(), > @@ -1580,7 +1580,7 @@ > (game, > *new Message > ("game engine", > - game.get_gametime(), > Forever(), > + game.get_gametime(), > _("Logic error"), > buffer, > > opponent.get_position(), > > === modified file 'src/logic/worker.cc' > --- src/logic/worker.cc 2014-08-26 17:25:00 +0000 > +++ src/logic/worker.cc 2014-09-11 17:46:54 +0000 > @@ -947,7 +947,7 @@ > (game, > *new Message > ("geologist " + rdescr->name(), // e.g. > "geologist gold" > - game.get_gametime(), 60 * 60 * 1000, > + game.get_gametime(), > rdescr->descname(), > message, > position, > @@ -1845,7 +1845,7 @@ > (game, > *new Message > ("game engine", > - game.get_gametime(), Forever(), > + game.get_gametime(), > _("Worker got lost!"), > buffer, > get_position()), > > === modified file 'src/map_io/widelands_map_players_messages_data_packet.cc' > --- src/map_io/widelands_map_players_messages_data_packet.cc 2014-07-28 > 14:23:03 +0000 > +++ src/map_io/widelands_map_players_messages_data_packet.cc 2014-09-11 > 17:46:54 +0000 > @@ -56,7 +56,7 @@ > if (begin != messages.end()) { > log > ("ERROR: The message queue for > player %u contains a message " > - "before any messages have been > loade into it. This is a bug " > + "before any messages have been > loaded into it. This is a bug " > "in the savegame loading code. > It created a new message and " > "added it to the queue. This > is only allowed during " > "simulation, not at load. The > following messge will be " > @@ -64,7 +64,6 @@ > "\tsender : %s\n" > "\ttitle : %s\n" > "\tsent : %u\n" > - "\tduration: %u\n" > "\tposition: (%i, %i)\n" > "\tstatus : %u\n" > "\tbody : %s\n", > @@ -72,7 +71,6 @@ > begin->second->sender > ().c_str(), > begin->second->title > ().c_str(), > begin->second->sent (), > - begin->second->duration(), > begin->second->position().x, > begin->second->position().y, > begin->second->status (), > begin->second->body > ().c_str()); > @@ -96,30 +94,7 @@ > "message is sent in > the future: sent at %u but " > "gametime is only %u", > sent, gametime); > - uint32_t duration = Forever(); // > default duration > - if (Section::Value const * const dv = > s->get_val("duration")) { > - duration = dv->get_positive(); > - if (duration == Forever()) > - throw game_data_error > - ( > - "the value %u > is not allowed as duration; it is " > - "a special > value meaning forever, which is the " > - "default; omit > the duration key to make the " > - "message exist > forever", > - Forever()); > - if (sent + duration < sent) > - throw game_data_error > - ( > - "duration %u > is too large; causes numeric " > - "overflow when > added to sent time %u", > - duration, > sent); > - if (sent + duration < gametime) > - throw game_data_error > - ( > - "message > should have expired at %u; sent at %u " > - "with duration > %u but gametime is already %u", > - sent + > duration, sent, duration, gametime); > - } > + > Message::Status status = > Message::Archived; // default status > if (char const * const status_string = > s->get_string("status")) { > try { > @@ -147,15 +122,12 @@ > (*new Message > (s->get_string > ("sender", ""), > sent, > - duration, > s->get_name (), > > s->get_safe_string("body"), > get_coords("position", > extent, Coords::Null(), s), > serial, > status)); > - // Expiration is scheduled for all > messages (with finite > - // duration) after the command queue > has been loaded (in > - // Game_Loader::load_game). > + > previous_message_sent = sent; > } catch (const _wexception & e) { > throw game_data_error > @@ -183,44 +155,11 @@ > message_saver.add (temp_message.first); > const Message & message = *temp_message.second; > assert(message.sent() <= > static_cast<uint32_t>(egbase.get_gametime())); > - assert > - (message.duration() == Forever() || > - message.sent() < message.sent() + > message.duration()); > - if > - (message.duration() != Forever() && > - message.sent() + message.duration() > - < > - static_cast<uint32_t>(egbase.get_gametime())) > - log > - ("ERROR: Trying to save a message that > should have expired:\n" > - "\tsent = %u, duration = %u, expiry = > %u, gametime = %u\n" > - "\tsender = \"%s\"\n" > - "\ttitle: %s\n" > - "\tbody: %s\n" > - "\tposition: (%i, %i)\n" > - "\tstatus: %s\n", > - message.sent(), message.duration(), > - message.sent() + message.duration(), > egbase.get_gametime(), > - message.sender().c_str(), > message.title().c_str(), > - message.body().c_str(), > - message.position().x, > message.position().y, > - message.status() == Message::New > ? "new" : > - message.status() == Message::Read > ? "read" : > - message.status() == Message::Archived > ? "archived" : "ERROR"); > - assert > - (message.duration() == Forever() || > - static_cast<uint32_t>(egbase.get_gametime()) > - <= > - message.sent() + message.duration()); > + > Section & s = > prof.create_section_duplicate(message.title().c_str()); > if (message.sender().size()) > s.set_string("sender", message.sender ()); > s.set_int ("sent", message.sent ()); > - { > - Duration const duration = message.duration(); > - if (duration != Forever()) // The default > duration. Do not write. > - s.set_int("duration", duration); > - } > s.set_string ("body", message.body ()); > if (Coords const c = message.position()) > set_coords("position", c, &s); > > === modified file 'src/scripting/lua_game.cc' > --- src/scripting/lua_game.cc 2014-07-28 16:59:54 +0000 > +++ src/scripting/lua_game.cc 2014-09-11 17:46:54 +0000 > @@ -297,18 +297,12 @@ > std::string title = luaL_checkstring(L, 2); > std::string body = luaL_checkstring(L, 3); > Coords c = Coords::Null(); > - Duration d = Forever(); > Message::Status st = Message::New; > std::string sender = "ScriptingEngine"; > bool popup = false; > > if (n == 4) { > // Optional arguments > - lua_getfield(L, 4, "duration"); > - if (!lua_isnil(L, -1)) > - d = luaL_checkuint32(L, -1); > - lua_pop(L, 1); > - > lua_getfield(L, 4, "field"); > if (!lua_isnil(L, -1)) > c = (*get_user_class<L_Field>(L, -1))->coords(); > @@ -344,7 +338,6 @@ > *new Message > (sender, > game.get_gametime(), > - d, > title, > body, > c, > @@ -1065,7 +1058,6 @@ > PROP_RO(L_Message, title), > PROP_RO(L_Message, body), > PROP_RO(L_Message, sent), > - PROP_RO(L_Message, duration), > PROP_RO(L_Message, field), > PROP_RW(L_Message, status), > {nullptr, nullptr, nullptr}, > @@ -1131,20 +1123,6 @@ > } > > /* RST > - .. attribute:: duration > - > - (RO) The time in milliseconds before this message is > invalidated or nil if > - this message has an endless duration. > -*/ > -int L_Message::get_duration(lua_State * L) { > - uint32_t d = get(L, get_game(L)).duration(); > - if (d == Forever()) > - return 0; > - lua_pushuint32(L, d); > - return 1; > -} > - > -/* RST > .. attribute:: field > > (RO) The field that corresponds to this Message. > > === modified file 'src/scripting/lua_game.h' > --- src/scripting/lua_game.h 2014-07-26 10:43:23 +0000 > +++ src/scripting/lua_game.h 2014-09-11 17:46:54 +0000 > @@ -165,7 +165,6 @@ > int get_sent(lua_State * L); > int get_title(lua_State * L); > int get_body(lua_State * L); > - int get_duration(lua_State * L); > int get_field(lua_State * L); > int get_status(lua_State * L); > int set_status(lua_State * L); > > === modified file 'test/maps/lua_testsuite.wmf/scripting/messages.lua' > --- test/maps/lua_testsuite.wmf/scripting/messages.lua 2014-01-12 > 19:06:22 +0000 > +++ test/maps/lua_testsuite.wmf/scripting/messages.lua 2014-09-11 > 17:46:54 +0000 > @@ -1,52 +1,47 @@ > -- ======================================================================= > --- Testing Messages System > +-- Testing Messages System > -- ======================================================================= > > messages_tests = lunit.TestCase("Messages") > -function messages_tests:setup() > +function messages_tests:setup() > for i,m in ipairs(player1.inbox) do > m.status = "archived" > end > end > > -function messages_tests:test_defaults() > +function messages_tests:test_defaults() > local m = player1:send_message("Hallo", "World!") > assert_equal("Hallo", m.title) > assert_equal("World!", m.body) > assert_equal("ScriptingEngine", m.sender) > assert_equal(0, m.sent) > assert_equal(0, m.sent) > - assert_equal(nil, m.duration) > assert_equal(nil, m.field) > assert_equal("new", m.status) > end > -function messages_tests:test_status_read() > +function messages_tests:test_status_read() > local m = player1:send_message("Hallo", "World!", {status="read"}) > assert_equal("read", m.status) > end > -function messages_tests:test_status_archived() > +function messages_tests:test_status_archived() > local m = player1:send_message("Hallo", "World!", {status="archived"}) > assert_equal("archived", m.status) > end > -function messages_tests:test_status_illegal() > +function messages_tests:test_status_illegal() > assert_error("Illegal status!", function() > player1:send_message("Hallo", "World!", {status="nono"}) > end) > end > -function messages_tests:test_sender() > +function messages_tests:test_sender() > local m = player1:send_message("Hallo", "World!", {sender="i am you"}) > assert_equal("i am you", m.sender) > end > -function messages_tests:test_field() > +function messages_tests:test_field() > local f = map:get_field(23,28) > local m = player1:send_message("Hallo", "World!", {field = f}) > assert_equal(f, m.field) > end > -function messages_tests:test_duration() > - local m = player1:send_message("Hallo", "World!", {duration = 2000}) > - assert_equal(2000, m.duration) > -end > -function messages_tests:test_changing_status() > +function messages_tests:test_changing_status() > local m = player1:send_message("Hallo", "World!") > m.status = "read" > assert_equal("read", m.status) > -- https://code.launchpad.net/~widelands-dev/widelands/bug-988831/+merge/233885 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-988831. _______________________________________________ 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