Merge lp:~widelands-dev/widelands/bug-1647033-immovable-packet-no into lp:widelands

Proposed by GunChleoc
Status: Merged
Merged at revision: 8210
Proposed branch: lp:~widelands-dev/widelands/bug-1647033-immovable-packet-no
Merge into: lp:widelands
Diff against target: 50 lines (+15/-4)
1 file modified
src/logic/map_objects/immovable.cc (+15/-4)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1647033-immovable-packet-no
Reviewer Review Type Date Requested Status
toptopple (community) Approve
Review via email: mp+312530@code.launchpad.net

Commit message

Immovables now have a dynamic packet number, so maps will still load with Build 19: The packet number is only increased if the immovable has a former building.

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 1719. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/181581877.
Appveyor build 1559. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1647033_immovable_packet_no-1559.

Revision history for this message
toptopple (7010622-q-deactivatedaccount) wrote :

I could create a stable version of Gold Rush (online as version 1.2) with this branch and it loads into B19. Noteworthy, the save games of later versions still report the same error when loaded in B19. Not necessarily a problem. Savegames probably will grow incompatible anyway soon, so it's not worth fixing this part.

review: Approve
Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks for testing! Savegames as a rule aren't backwards compatible, only forwards compatible. The important thing is that maps can be loaded with older versions :)

@bunnybot merge

Revision history for this message
SirVer (sirver) wrote :

All other gold rush maps have been deleted, so the bug should not have any more impact.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/logic/map_objects/immovable.cc'
--- src/logic/map_objects/immovable.cc 2016-11-22 19:54:02 +0000
+++ src/logic/map_objects/immovable.cc 2016-12-06 07:46:30 +0000
@@ -532,6 +532,11 @@
532==============================532==============================
533*/533*/
534534
535// We neeed 2 packet versions for map loading: Packet version 7 will load in older versions of
536// Widelands, so we have a dynamic version number - it is only set higher than
537// kCurrentPacketVersionImmovableNoFormerBuildings during saving if we have an immovable with
538// a former building assigned to it.
539constexpr uint8_t kCurrentPacketVersionImmovableNoFormerBuildings = 7;
535constexpr uint8_t kCurrentPacketVersionImmovable = 8;540constexpr uint8_t kCurrentPacketVersionImmovable = 8;
536541
537// Supporting older versions for map loading542// Supporting older versions for map loading
@@ -554,7 +559,7 @@
554 imm.position_ = read_coords_32(&fr, egbase().map().extent());559 imm.position_ = read_coords_32(&fr, egbase().map().extent());
555 imm.set_position(egbase(), imm.position_);560 imm.set_position(egbase(), imm.position_);
556561
557 if (packet_version >= 8) {562 if (packet_version > kCurrentPacketVersionImmovableNoFormerBuildings) {
558 Player* owner = imm.get_owner();563 Player* owner = imm.get_owner();
559 if (owner) {564 if (owner) {
560 DescriptionIndex idx = owner->tribe().building_index(fr.string());565 DescriptionIndex idx = owner->tribe().building_index(fr.string());
@@ -645,7 +650,10 @@
645 // This is in front because it is required to obtain the description650 // This is in front because it is required to obtain the description
646 // necessary to create the Immovable651 // necessary to create the Immovable
647 fw.unsigned_8(HeaderImmovable);652 fw.unsigned_8(HeaderImmovable);
648 fw.unsigned_8(kCurrentPacketVersionImmovable);653 const uint8_t packet_version = former_building_descr_ == nullptr ?
654 kCurrentPacketVersionImmovableNoFormerBuildings :
655 kCurrentPacketVersionImmovable;
656 fw.unsigned_8(packet_version);
649657
650 if (descr().owner_type() == MapObjectDescr::OwnerType::kTribe) {658 if (descr().owner_type() == MapObjectDescr::OwnerType::kTribe) {
651 if (get_owner() == nullptr)659 if (get_owner() == nullptr)
@@ -662,8 +670,11 @@
662670
663 fw.unsigned_8(get_owner() ? get_owner()->player_number() : 0);671 fw.unsigned_8(get_owner() ? get_owner()->player_number() : 0);
664 write_coords_32(&fw, position_);672 write_coords_32(&fw, position_);
665 if (get_owner()) {673 if (get_owner() && former_building_descr_) {
666 fw.string(former_building_descr_ ? former_building_descr_->name() : "");674 assert(packet_version > kCurrentPacketVersionImmovableNoFormerBuildings);
675 fw.string(former_building_descr_->name());
676 } else {
677 assert(packet_version == kCurrentPacketVersionImmovableNoFormerBuildings);
667 }678 }
668679
669 // Animations680 // Animations

Subscribers

People subscribed via source and target branches

to status/vote changes: