[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bridges into lp:widelands has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Continuous integration builds have changed state: Travis build 5186. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/544760083. Appveyor build 4966. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bridges-4966. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Continuous integration builds have changed state: Travis build 5063. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/537402794. Appveyor build 4843. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bridges-4843. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Yep, let's wait for the ferry branch and do a final round of code review and testing in this branch when that's done. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
I already reworked that whole stuff in a different way for the ferries. The TODO I think you´re referring to is also solved there already. I prefer my solution because it gets rid of this bitwise hacking, and there´s no need to invent the wheel twice… -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Mhhh, We could iprove the RoadType via 4 bitflags and some inline fucntions: 1000 ^ Road ^ Bridge ^ Waterway ^ Busy kRoad = 1 kNormalRoad = 1 kBusyRoad = 9 kBridge = 2 kBridgeNormal = 2 kBridgeBusy = 10 kWaterway = 4 kNormalWaterway = 4 kBusyWaterway = 12 kBusy = 8 This woud avoid a lot of if(this && that || soemthingElse) code? Once this is in I would address SirVers TODO and extract some RoadDirection enum -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
The ferry branch needs to be merged first because this one is stacked on it. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Now, can ths be merged or do we want another review? -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Continuous integration builds have changed state: Travis build 4841. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/526398829. Appveyor build 4622. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bridges-4622. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Continuous integration builds have changed state: Travis build 4838. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/526055823. Appveyor build 4619. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bridges-4619. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Maybe i should test the ferrys in the appropriate branch... can't get them to work... Regarding the bridges i think having a bridge for normal roads which looks more like a wooden footbridge would be nicer. Lifting the workers when walking over the bridge is really nice :) -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Continuous integration builds have changed state: Travis build 4668. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/515650670. Appveyor build 4454. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bridges-4454. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Thanks for reporting, I pushed a fix for the bug. It was a small error in the loading code; the savegame can now be loaded. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Forget: To reproduce 1. load 'test_ferries.wgf' 2. Build a waterway from flag 30,28 to node 31,27 3. Save the game 4. Try to load the previous saved game -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Review: Needs Fixing I get an error when trying to load this savegame: https://bugs.launchpad.net/widelands/+bug/734193/+attachment/5252593/+files/test_ferries_1.wgf This savegame was made close before the other one. The difference is this savegame has no waterway yet. The other savegame has a waterway, which has no ferry: https://bugs.launchpad.net/widelands/+bug/734193/+attachment/5252594/+files/test_ferries.wgf -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Continuous integration builds have changed state: Travis build 4652. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/511593017. Appveyor build 4439. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bridges-4439. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Continuous integration builds have changed state: Travis build 4651. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/511385042. Appveyor build 4438. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bridges-4438. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Continuous integration builds have changed state: Travis build 4626. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/510312899. Appveyor build 4413. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bridges-4413. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Review: Approve Done a bit of testing ad it looks fine. There is 2 small z-layering glitch for bridges going north-south: Workers walking on roads behind them to their upper flag are blitted after the bridge is, so they appear to be on top of it. It's not a biggie though and shouldn't block this branch. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/bridges into lp:widelands
Fixed the assert. Thanks for taking care of the warnings :) -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bridges 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
I have fixed 2 compiler warnings. You need to use the PRIuS macro for logging size_t, because the exact data type depends on the operating system. Also, uint8_t will be interpreted as a character code point rather than a number, so it needs an annoying explicit cast to every time you want to print it. I am getting a failed assert: widelands: ../src/logic/editor_game_base.cc:526: void Widelands::EditorGameBase::set_road(const Widelands::FCoords&, uint8_t, uint8_t): Assertion `roadtype == RoadType::kNone || roadtype == RoadType::kNormal || roadtype == RoadType::kBusy || roadtype == RoadType::kWaterway' failed. This happened after starting a new game with all 4 tribes in Archipelago Sea, and the AI built something somewhere. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bridges 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Continuous integration builds have changed state: Travis build 4615. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/508306031. Appveyor build 4402. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bridges-4402. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bridges 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
you need to merge trunk to make appveyor happy and working -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bridges 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Continuous integration builds have changed state: Travis build 4609. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/508054226. Appveyor build 4396. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bridges-4396. -- https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bridges 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Diff comments:
> === modified file 'src/economy/roadbase.cc'
> --- src/economy/roadbase.cc 2019-03-12 12:11:23 +
> +++ src/economy/roadbase.cc 2019-03-12 12:11:50 +
> @@ -69,6 +71,68 @@
> return *flags_[FlagStart];
> }
>
> +// This returns true if and only if this is a road that covers the specified
> edge and
> +// both triangles adjacent to that edge are unwalkable
> +bool RoadBase::is_bridge(const EditorGameBase& egbase, const FCoords& field,
> uint8_t dir) const {
OK, let's leave it as it is then :)
> + if (descr().type() != MapObjectType::ROAD) {
> + // waterways can't be bridges...
> + return false;
> + }
> +
> + const Map& map = egbase.map();
> +
> + FCoords iterate = map.get_fcoords(path_.get_start());
> + const Path::StepVector::size_type nr_steps = path_.get_nsteps();
> + bool found = false;
> + for (Path::StepVector::size_type i = 0; i <= nr_steps; ++i) {
> + if (iterate == field) {
> + if ((i < nr_steps && path_[i] == dir) || (i > 0 &&
> path_[i - 1] == get_reverse_dir(dir))) {
> + found = true;
> + break;
> + }
> + return false;
> + }
> + if (i < nr_steps) {
> + map.get_neighbour(iterate, path_[i], &iterate);
> + }
> + }
> + if (!found) {
> + return false;
> + }
> +
> + FCoords fr, fd;
> + switch (dir) {
> + case WALK_SW:
> + fd = field;
> + map.get_ln(field, &fr);
> + break;
> + case WALK_SE:
> + fd = field;
> + fr = field;
> + break;
> + case WALK_NW:
> + map.get_tln(field, &fd);
> + fr = fd;
> + break;
> + case WALK_NE:
> + map.get_trn(field, &fd);
> + map.get_tln(field, &fr);
> + break;
> + case WALK_W:
> + map.get_tln(field, &fd);
> + map.get_ln(field, &fr);
> + break;
> + case WALK_E:
> + map.get_trn(field, &fd);
> + fr = field;
> + break;
> + default:
> + NEVER_HERE();
> + }
> + return (egbase.world().terrain_descr(fd.field->terrain_d()).get_is() &
> TerrainDescription::Is::kUnwalkable) &&
> +
> (egbase.world().terrain_descr(fr.field->terrain_r()).get_is() &
> TerrainDescription::Is::kUnwalkable);
> +}
> +
> /**
> * Return the cost of getting from fromflag to the other flag.
> */
> @@ -107,15 +171,25 @@
> // mark the road that leads up to this field
> if (steps > 0) {
> const Direction dir = get_reverse_dir(path_[steps - 1]);
> - if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E)
> - egbase.set_road(curf, dir, type_);
> + if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E) {
> + uint8_t type = type_;
> + if (is_bridge(egbase, curf, dir)) {
> + type += RoadType::kBridgeNormal -
> RoadType::kNormal;
OK :)
> + }
> + egbase.set_road(curf, dir, type);
> + }
> }
>
> // mark the road that leads away from this field
> if (steps < path_.get_nsteps()) {
> const Direction dir = path_[steps];
> - if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E)
> - egbase.set_road(curf, dir, type_);
> + if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E) {
> + uint8_t type = type_;
> + if (is_bridge(egbase, curf, dir)) {
> + type += RoadType::kBridgeNormal -
> RoadType::kNormal;
> + }
> + egbase.set_road(curf, dir, type);
> + }
>
> map.get_neighbour(curf, dir, &curf);
> }
--
https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bridges 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Replied to diff comments
Diff comments:
> === modified file 'src/economy/roadbase.cc'
> --- src/economy/roadbase.cc 2019-03-12 12:11:23 +
> +++ src/economy/roadbase.cc 2019-03-12 12:11:50 +
> @@ -69,6 +71,68 @@
> return *flags_[FlagStart];
> }
>
> +// This returns true if and only if this is a road that covers the specified
> edge and
> +// both triangles adjacent to that edge are unwalkable
> +bool RoadBase::is_bridge(const EditorGameBase& egbase, const FCoords& field,
> uint8_t dir) const {
This function is currently called only from mark_map() when a new road is
initialized or its state changes between normal and busy. The result is cached
in the RoadType by mark_map().
I deliberately don´t recalculate anything on terrain change, because I think it
looks more natural when a bridge isn´t built/removed immediately when the land
is flooded/drained.
> + if (descr().type() != MapObjectType::ROAD) {
> + // waterways can't be bridges...
> + return false;
> + }
> +
> + const Map& map = egbase.map();
> +
> + FCoords iterate = map.get_fcoords(path_.get_start());
> + const Path::StepVector::size_type nr_steps = path_.get_nsteps();
> + bool found = false;
> + for (Path::StepVector::size_type i = 0; i <= nr_steps; ++i) {
> + if (iterate == field) {
> + if ((i < nr_steps && path_[i] == dir) || (i > 0 &&
> path_[i - 1] == get_reverse_dir(dir))) {
> + found = true;
> + break;
> + }
> + return false;
> + }
> + if (i < nr_steps) {
> + map.get_neighbour(iterate, path_[i], &iterate);
> + }
> + }
> + if (!found) {
> + return false;
> + }
> +
> + FCoords fr, fd;
> + switch (dir) {
> + case WALK_SW:
> + fd = field;
> + map.get_ln(field, &fr);
> + break;
> + case WALK_SE:
> + fd = field;
> + fr = field;
> + break;
> + case WALK_NW:
> + map.get_tln(field, &fd);
> + fr = fd;
> + break;
> + case WALK_NE:
> + map.get_trn(field, &fd);
> + map.get_tln(field, &fr);
> + break;
> + case WALK_W:
> + map.get_tln(field, &fd);
> + map.get_ln(field, &fr);
> + break;
> + case WALK_E:
> + map.get_trn(field, &fd);
> + fr = field;
> + break;
> + default:
> + NEVER_HERE();
> + }
> + return (egbase.world().terrain_descr(fd.field->terrain_d()).get_is() &
> TerrainDescription::Is::kUnwalkable) &&
> +
> (egbase.world().terrain_descr(fr.field->terrain_r()).get_is() &
> TerrainDescription::Is::kUnwalkable);
> +}
> +
> /**
> * Return the cost of getting from fromflag to the other flag.
> */
> @@ -107,15 +171,25 @@
> // mark the road that leads up to this field
> if (steps > 0) {
> const Direction dir = get_reverse_dir(path_[steps - 1]);
> - if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E)
> - egbase.set_road(curf, dir, type_);
> + if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E) {
> + uint8_t type = type_;
> + if (is_bridge(egbase, curf, dir)) {
> + type += RoadType::kBridgeNormal -
> RoadType::kNormal;
kBusy = kNormal + 1
kBridgeBusy = kBridgeNormal + 1
So this calculation catches both cases.
> + }
> + egbase.set_road(curf, dir, type);
> + }
> }
>
> // mark the road that leads away from this field
> if (steps < path_.get_nsteps()) {
> const Direction dir = path_[steps];
> - if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E)
> - egbase.set_road(curf, dir, type_);
> + if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E) {
> + uint8_t type = type_;
> + if (is_bridge(egbase, curf, dir)) {
> + type += RoadType::kBridgeNormal -
> RoadType::kNormal;
> + }
> + egbase.set_road(curf, dir, type);
> + }
>
> map.get_neighbour(curf, dir, &curf);
> }
>
> === modified file 'src/logic/map_objects/tribes/road_textures.cc'
> --- src/logic/map_objec
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Some code review comments
Diff comments:
> === modified file 'src/economy/roadbase.cc'
> --- src/economy/roadbase.cc 2019-03-12 12:11:23 +
> +++ src/economy/roadbase.cc 2019-03-12 12:11:50 +
> @@ -69,6 +71,68 @@
> return *flags_[FlagStart];
> }
>
> +// This returns true if and only if this is a road that covers the specified
> edge and
> +// both triangles adjacent to that edge are unwalkable
> +bool RoadBase::is_bridge(const EditorGameBase& egbase, const FCoords& field,
> uint8_t dir) const {
How is the performance for this function? It might be worth it to remember it
in the map and only recalculate on terrain changes, like we do for seafaring
checks.
> + if (descr().type() != MapObjectType::ROAD) {
> + // waterways can't be bridges...
> + return false;
> + }
> +
> + const Map& map = egbase.map();
> +
> + FCoords iterate = map.get_fcoords(path_.get_start());
> + const Path::StepVector::size_type nr_steps = path_.get_nsteps();
> + bool found = false;
> + for (Path::StepVector::size_type i = 0; i <= nr_steps; ++i) {
> + if (iterate == field) {
> + if ((i < nr_steps && path_[i] == dir) || (i > 0 &&
> path_[i - 1] == get_reverse_dir(dir))) {
> + found = true;
> + break;
> + }
> + return false;
> + }
> + if (i < nr_steps) {
> + map.get_neighbour(iterate, path_[i], &iterate);
> + }
> + }
> + if (!found) {
> + return false;
> + }
> +
> + FCoords fr, fd;
> + switch (dir) {
> + case WALK_SW:
> + fd = field;
> + map.get_ln(field, &fr);
> + break;
> + case WALK_SE:
> + fd = field;
> + fr = field;
> + break;
> + case WALK_NW:
> + map.get_tln(field, &fd);
> + fr = fd;
> + break;
> + case WALK_NE:
> + map.get_trn(field, &fd);
> + map.get_tln(field, &fr);
> + break;
> + case WALK_W:
> + map.get_tln(field, &fd);
> + map.get_ln(field, &fr);
> + break;
> + case WALK_E:
> + map.get_trn(field, &fd);
> + fr = field;
> + break;
> + default:
> + NEVER_HERE();
> + }
> + return (egbase.world().terrain_descr(fd.field->terrain_d()).get_is() &
> TerrainDescription::Is::kUnwalkable) &&
> +
> (egbase.world().terrain_descr(fr.field->terrain_r()).get_is() &
> TerrainDescription::Is::kUnwalkable);
> +}
> +
> /**
> * Return the cost of getting from fromflag to the other flag.
> */
> @@ -107,15 +171,25 @@
> // mark the road that leads up to this field
> if (steps > 0) {
> const Direction dir = get_reverse_dir(path_[steps - 1]);
> - if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E)
> - egbase.set_road(curf, dir, type_);
> + if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E) {
> + uint8_t type = type_;
> + if (is_bridge(egbase, curf, dir)) {
> + type += RoadType::kBridgeNormal -
> RoadType::kNormal;
What about busy bridges?
> + }
> + egbase.set_road(curf, dir, type);
> + }
> }
>
> // mark the road that leads away from this field
> if (steps < path_.get_nsteps()) {
> const Direction dir = path_[steps];
> - if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E)
> - egbase.set_road(curf, dir, type_);
> + if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E) {
> + uint8_t type = type_;
> + if (is_bridge(egbase, curf, dir)) {
> + type += RoadType::kBridgeNormal -
> RoadType::kNormal;
> + }
> + egbase.set_road(curf, dir, type);
> + }
>
> map.get_neighbour(curf, dir, &curf);
> }
>
> === modified file 'src/logic/map_objects/tribes/road_textures.cc'
> --- src/logic/map_objects/tribes/road_textures.cc 2019-03-12 12:11:23
> +
> +++ src/logic/map_objects/tribes/road_textures.cc 2019-03-12 12:11:50
> +
> @@ -17,10 +17,25 @@
> *
> */
>
> +#include "base/wexception.h"
> #include "logic/map_objects/tribes/road_textures.h
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bridges into lp:widelands
Benedikt Straub has proposed merging lp:~widelands-dev/widelands/bridges into
lp:widelands with lp:~widelands-dev/widelands/ferry as a prerequisite.
Commit message:
Roads where both adjacent triangles are unwalkable are displayed as bridges
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #734193 in widelands: "introduce footbridges over water and swamp"
https://bugs.launchpad.net/widelands/+bug/734193
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318
The current road rendering system is too inflexible for this feature. I already
had to rework it in the ferry branch to make waterways possible, and I´m using
these changes here. So this is a follow-up of the ferries.
Every tribe has its own images for a normal and a busy bridge. Bridges may also
be animated. They have a height, which is visible when bobs walk over the
bridge.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bridges into lp:widelands.
=== modified file 'data/tribes/atlanteans.lua'
--- data/tribes/atlanteans.lua 2019-03-12 12:00:25 +
+++ data/tribes/atlanteans.lua 2019-03-12 12:01:02 +
@@ -20,6 +20,10 @@
--
--**animations**: Global animations. Contains subtables for ``frontier`` and ``flag``. Each animation needs the parameters ``pictures`` (table of filenames) and ``hotspot`` (2 integer coordinates), and may also define ``fps`` (integer frames per second).
--
+--**bridges**: Contains animations for ``normal_e``, ``normal_se``, ``normal_sw``, ``busy_e``, ``busy_se`` and ``busy_sw``.
+--
+--**bridge_height**: The height in pixels of each bridge at it's summit at 1x scale.
+--
--**roads**: The file paths for the tribe's road textures in 3 subtables ``busy``, ``normal`` and ``waterway``.
--
--**resource_indicators**: The names for the resource indicators. This table contains a subtable for each resource name plus a subtable named "" for no resources. Each subtable is an array, in which the index of each entry is the highest amount of resources the indicator may indicate.
@@ -68,6 +72,34 @@
}
},
+ bridges = {
+ normal_e = {
+ pictures = path.list_files (dirname .. "images/atlanteans/bridge_normal_e_?.png"),
+ hotspot = { -2, 11 },
+ },
+ normal_se = {
+ pictures = path.list_files (dirname .. "images/atlanteans/bridge_normal_se_?.png"),
+ hotspot = { 5, 2 },
+ },
+ normal_sw = {
+ pictures = path.list_files (dirname .. "images/atlanteans/bridge_normal_sw_?.png"),
+ hotspot = { 36, 3 },
+ },
+ busy_e = {
+ pictures = path.list_files (dirname .. "images/atlanteans/bridge_busy_e_?.png"),
+ hotspot = { -2, 11 },
+ },
+ busy_se = {
+ pictures = path.list_files (dirname .. "images/atlanteans/bridge_busy_se_?.png"),
+ hotspot = { 5, 2 },
+ },
+ busy_sw = {
+ pictures = path.list_files (dirname .. "images/atlanteans/bridge_busy_sw_?.png"),
+ hotspot = { 36, 3 },
+ },
+ },
+ bridge_height = 8,
+
-- Image file paths for this tribe's road and waterway textures
roads = {
busy = {
=== modified file 'data/tribes/barbarians.lua'
--- data/tribes/barbarians.lua 2019-03-12 12:00:25 +
+++ data/tribes/barbarians.lua 2019-03-12 12:01:02 +
@@ -15,6 +15,34 @@
}
},
+ bridges = {
+ normal_e = {
+ pictures = path.list_files (dirname .. "images/barbarians/bridge_normal_e_?.png"),
+ hotspot = { -1, 13 },
+ },
+ normal_se = {
+ pictures = path.list_files (dirname .. "images/barbarians/bridge_normal_se_?.png"),
+ hotspot = { 8, 3 },
+ },
+ normal_sw = {
+ pictures = path.list_files (dirname .. "images/barbarians/bridge_normal_sw_?.png"),
+ hotspot = { 41, 3 },
+ },
+ busy_e = {
+ pictures = path.list_files (dirname .. "images/barbarians/bridge_busy_e_?.png"),
+ hotspot = { -1, 13 },
+ },
+ busy_se = {
+ pictures = path.list_files (dirname .. "images/barbarians/bridge_busy_se_?.png"),
+ hotspot = { 8, 3 },
+ },
+ busy_sw = {
+ pictures = path.list_files (dirname .. "images/barbarians/bridge_busy_sw_?.png"),
+ hotspot = { 41, 3 },
+ },
+ },
+ bridge_height = 8,
+
-- Image file paths for this tribe's road and waterway textures
roads = {
busy = {
=== modified file 'data/tribes/empire.lua'
--- data/tribes/empire.lua 2019-03-12 12:00:25 +
+++ data/tribes/empire.lua 2019-03-12 12:01:02 +
@@ -17,6 +17,34 @@
}
},
+ bridges = {
+ normal_e = {
+ pictures = path.list_files (dirname .. "images/empire/bridge_normal_e_?.png"),
+ hotspot = { -2, 12 },
+ },
+ normal_se = {
+ pictures = path.list_files (dirname .. "images/empire/bridge_normal_se_?.png"),
+ hotspot =
