Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786597-test-suite into lp:widelands
@bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite. ___ 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-1786597-test-suite into lp:widelands
Transient error on Travis @bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite. ___ 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-1786597-test-suite into lp:widelands
Thanks for the review! :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite. ___ 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-1786597-test-suite into lp:widelands
Review: Approve As far as it concerns me, this looks ok ;-P -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite. ___ 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-1786597-test-suite into lp:widelands
No, it will fix the test suite. See the changes to the binary file on he bottom ;) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite. ___ 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-1786597-test-suite into lp:widelands
Review: Needs Information Does this end this branch' reason of existence? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite. ___ 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-1786597-test-suite into lp:widelands
I tried to add https://bugs.launchpad.net/widelands/+bug/1535115/comments/33 into the compatibility code but it broke the test suite again, so I used the branch to update the mapobjectpacket in the problematic test scenario and removed the savegame compatibility. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite. ___ 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-1786597-test-suite into lp:widelands
In current problematic design, a carrier in an invalid state can easily get stuck, so we will need to test some actual savegames to answer that. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite. ___ 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-1786597-test-suite into lp:widelands
Oops... fixed! If this code sufficient or will it barf it the carrier is in init rather than wait? I don't care much about 100% correctness for the saveloading at this point. I am working on a follow-up branch and use an enum class for the states. Working on that has flagged up lots of hard-coded and fragile stuff from old code. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite. ___ 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-1786597-test-suite into lp:widelands
Inline comment.
Also, if you haven't done it, check this comment:
https://bugs.launchpad.net/widelands/+bug/1535115/comments/33
Diff comments:
> === modified file 'src/logic/map_objects/tribes/carrier.cc'
> --- src/logic/map_objects/tribes/carrier.cc 2018-08-09 11:11:15 +
> +++ src/logic/map_objects/tribes/carrier.cc 2018-08-11 11:58:45 +
> @@ -433,9 +435,14 @@
> try {
>
> uint8_t packet_version = fr.unsigned_8();
> - if (packet_version == kCurrentPacketVersion) {
> + if (packet_version >= 1 && packet_version <=
> kCurrentPacketVersion) {
> Carrier& carrier = get();
> carrier.operation_ = fr.signed_32();
> + if (packet_version == 1) {
> + if (carrier.operation_ == NO_OPERATION) {
> + carrier.operation_ == INIT;
This looks like it should be =
> + }
> + }
> } else {
> throw UnhandledVersionError("Carrier", packet_version,
> kCurrentPacketVersion);
> }
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1786597-test-suite 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
