Review: Approve
Code LGTM - 2 tiny nits. Not tested.
Diff comments:
>
> === modified file 'src/logic/cmd_luacoroutine.cc'
> --- src/logic/cmd_luacoroutine.cc 2017-01-25 18:55:59 +0000
> +++ src/logic/cmd_luacoroutine.cc 2017-07-05 21:50:35 +0000
> @@ -51,11 +53,11 @@
> log("%s\n", e.what());
> log("Send message to all players and pause game\n");
> for (int i = 1; i <= game.map().get_nrplayers(); i++) {
> - Widelands::Message& msg = *new Widelands::Message(
> + std::unique_ptr<Message >msg(new Widelands::Message(
<Message >msg => <Message> msg. Does clang-format fix this?
> Message::Type::kGameLogic, game.get_gametime(),
> "Coroutine",
> "images/ui_basic/menu_help.png", "Lua Coroutine
> Failed",
> - (boost::format("<rt><p font-size=12>%s</p></rt>") %
> e.what()).str());
> - game.player(i).add_message(game, msg, true);
> + (boost::format("<rt><p font-size=12>%s</p></rt>") %
> e.what()).str()));
> + game.player(i).add_message(game, std::move(msg), true);
> }
> game.game_controller()->set_desired_speed(0);
> }
>
> === modified file 'src/logic/message_queue.h'
> --- src/logic/message_queue.h 2017-01-25 18:55:59 +0000
> +++ src/logic/message_queue.h 2017-07-05 21:50:35 +0000
> @@ -39,29 +42,27 @@
> } //
> C++0x:
>
> ~MessageQueue() {
> - while (size()) {
> - delete begin()->second;
> - erase(begin());
> - }
> }
>
> // Make some selected inherited members public.
> - MessageQueue::const_iterator begin() const {
> - return std::map<MessageId, Message*>::begin();
> - }
> - MessageQueue::const_iterator end() const {
> - return std::map<MessageId, Message*>::end();
> - }
> - size_type count(uint32_t const i) const {
> + // TODO(sirver): This is weird design. Instead pass out a const ref& to
> + // 'messages_'?
> + MessageMap::const_iterator begin() const {
> + return messages_.begin();
> + }
> + MessageMap::const_iterator end() const {
> + return messages_.end();
> + }
> + size_t count(uint32_t const i) const {
> assert_counts();
> - return std::map<MessageId, Message*>::count(MessageId(i));
> + return messages_.count(MessageId(i));
> }
>
> /// \returns a pointer to the message if it exists, otherwise 0.
otherwise nullptr
> Message const* operator[](const MessageId& id) const {
> assert_counts();
> - MessageQueue::const_iterator const it = find(MessageId(id));
> - return it != end() ? it->second : nullptr;
> + const auto it = messages_.find(MessageId(id));
> + return it != end() ? it->second.get() : nullptr;
> }
>
> /// \returns the number of messages with the given status.
--
https://code.launchpad.net/~widelands-dev/widelands/00_private_inheritance/+merge/326886
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/00_private_inheritance.
_______________________________________________
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
