GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-988831 into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #988831 in widelands: "Remove message expiry feature."
https://bugs.launchpad.net/widelands/+bug/988831
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-988831/+merge/233885
Messages no longer expire
--
https://code.launchpad.net/~widelands-dev/widelands/bug-988831/+merge/233885
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-988831 into lp:widelands.
=== 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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +0000
@@ -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.
/// 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)
{}
=== 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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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-09 08:28:48 +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)
_______________________________________________
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
