From kde-games-devel Sat Mar 09 20:55:32 2013 From: "Jaime Torres Amate" Date: Sat, 09 Mar 2013 20:55:32 +0000 To: kde-games-devel Subject: Re: [Kde-games-devel] Review Request 106772: add option to allow adjacent ships Message-Id: <20130309205532.24800.66851 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kde-games-devel&m=136286254617633 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============0636701377700243776==" --===============0636701377700243776== Content-Type: multipart/alternative; boundary="===============2090269333661516433==" --===============2090269333661516433== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit > On March 9, 2013, 6:02 p.m., Roney Gomes wrote: > > src/networkentity.cpp, line 43 > > > > > > Would not be better to work this second feature in a separate patch? Yes, of course. That is why the TODO is there. The message always sends a "true" until it is implemented. But the GameOptionsMessage (receiver) already parses the two options because, in my opinion, it is much better to change as few as possible the network messages. Otherwise, I will change the patch to handle only one option (and change the message name, of course), and in the future there would be another message for the second feature. - Jaime Torres ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106772/#review28849 ----------------------------------------------------------- On March 6, 2013, 9:18 p.m., Jaime Torres Amate wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106772/ > ----------------------------------------------------------- > > (Updated March 6, 2013, 9:18 p.m.) > > > Review request for KDE Games. > > > Description > ------- > > Add the option to allow adjacent ships. > When it is not checked, the ships can not be placed adjacent. > > > This addresses bug 168659. > http://bugs.kde.org/show_bug.cgi?id=168659 > > > Diffs > ----- > > src/battlefield.h ae78166 > src/battlefield.cpp 32aa525 > src/controller.h bbae7a6 > src/controller.cpp 761eda4 > src/kbattleship.kcfg b615f30 > src/kbattleshipui.rc 7c51ac6 > src/mainwindow.cpp 82d9a14 > src/message.h 2757cf7 > src/message.cpp 59de527 > src/networkentity.cpp 5fff5b8 > src/playfield.h 56934dc > src/playfield.cpp 61c0ac6 > src/protocol.cpp c5cbacc > src/sea.h e76be24 > src/sea.cpp de8b697 > src/ship.cpp 86858d9 > > Diff: http://git.reviewboard.kde.org/r/106772/diff/ > > > Testing > ------- > > tested locally. > > > Thanks, > > Jaime Torres Amate > > --===============2090269333661516433== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106772/

On March 9th, 2013, 6:02 p.m. UTC, Roney Gomes wrote:

src/networkentity.cpp (Diff revision 4)
void NetworkEntity::start(bool ask)
		43	m_protocol->send(MessagePtr(new GameOptionsMessage(QString(Settings::adjacentShips() ? "true" : "false"), /* TODO */"true")));

Would not be better to work this second feature in a separate patch?

Yes, of course. That is why the TODO is there. The message always sends a "true" until it is implemented.
But the GameOptionsMessage (receiver) already parses the two options because, in my opinion, it is much better to change as few as possible the network messages.
Otherwise, I will change the patch to handle only one option (and change the message name, of course), and in the future there would be another message for the second feature.

- Jaime Torres

On March 6th, 2013, 9:18 p.m. UTC, Jaime Torres Amate wrote:

Review request for KDE Games.

By Jaime Torres Amate.

Updated March 6, 2013, 9:18 p.m.

Description

Add the option to allow adjacent ships.
When it is not checked, the ships can not be placed adjacent.

Testing

tested locally.

Bugs: 168659

Diffs

src/battlefield.h (ae78166)
src/battlefield.cpp (32aa525)
src/controller.h (bbae7a6)
src/controller.cpp (761eda4)
src/kbattleship.kcfg (b615f30)
src/kbattleshipui.rc (7c51ac6)
src/mainwindow.cpp (82d9a14)
src/message.h (2757cf7)
src/message.cpp (59de527)
src/networkentity.cpp (5fff5b8)
src/playfield.h (56934dc)
src/playfield.cpp (61c0ac6)
src/protocol.cpp (c5cbacc)
src/sea.h (e76be24)
src/sea.cpp (de8b697)
src/ship.cpp (86858d9)

--===============2090269333661516433==-- --===============0636701377700243776== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kde-games-devel mailing list kde-games-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-games-devel --===============0636701377700243776==--