change planer algorithms to match the original

[libv]

S2 planes the area to match the altitude of the flag, not the altitude of the building tile. This means both a difference in what altitude is being planed to, and also changes the calculation for deciding to plane or not. S2 changes the height of the building tile to match the flag when the planer has done his round and returns to the central building tile.

[Flamefire]

Just to be clear on the algorithm:

• planer goes to the building
• then walks around to each neighbour point except the flag and planes it
• while doing that, instead of going to flag (i.e. after finishing the cycle) goes back to building
• building levels too and planer goes home

I.e. it still doesn't work on the actual building site but that still gets modified.
Correct?

@Spikeone agreed?

An issue I see with this: The change is fundamental enough that existing replays will likely stop working.

[Flamefire]

To make this easier to understand:

Suggested change

	int altitude = world->GetNode(this->GetFlagPos()).altitude;
	const auto flagAltitude = world->GetNode(this->GetFlagPos()).altitude;

[Flamefire]

Enhance check (same below) and indent

Suggested change

	if(altitude - world->GetNode(pos).altitude != 0)
	state = BuildingSiteState::Planing;
	if(altitude != world->GetNode(pos).altitude)
	state = BuildingSiteState::Planing;

And put the loop into an else-branch

[libv]

I actually removed the tabbed indents by hand, as my editor is set to kernel style. And the very awkward calculation matches the rest of the code. So when reviewing this patch, please see the context, i have done extra work to match the rest of the codebase.

[Flamefire]

I now see there is a tab left, yes. You can use clang-format to auto-format. If it is found by configure there is even a build target for that. CI will tell you too once run

This wasn't criticism of your work and matching this (strange) style is certainly right in general.

My suggestion here is to fix/use the != style for the new code and adapt the old code below to match this. I see no reason for keeping the old style which raises questions e.g. if it could run into UB. Doesn't here, but != avoids the question and makes the intent clearer.

[libv]

Well, from where I sit, once you start refactoring code, you never stop :) I would just use the old style here and let it be until someone uses lindent on whole files.

[libv]

Just to be clear on the algorithm:

* planer goes to the building
* then walks around to each neighbour point except the flag and planes it
* while doing that, instead of going to flag (i.e. after finishing the cycle) goes back to building
* building levels too and planer goes home

Yes. Same algorithm as rttr before, just to the level of the flag, and the building tile just pops into place as the planer finishes his round, heads to the building tile, then walks off through the flag tile.

I.e. it still doesn't work on the actual building site but that still gets modified. Correct?

If you mean that this does not change the altitude of the actual building site, then no. This changes the altitude of the building site to also match the altitude of the flag. The whole site gets planed to the altitude of the flag.

@Spikeone agreed?

An issue I see with this: The change is fundamental enough that existing replays will likely stop working.

Quite likely, but this is how S2 works, and i do not think i have ever known it differently. Fire up dosbox, and an original s2 game, plane a building site where the flag is a different altitude from the main buiding tile :)

[Flamefire]

If you mean that this does not change the altitude of the actual building site, then no. This changes the altitude of the building site to also match the altitude of the flag. The whole site gets planed to the altitude of the flag.

The question was if the building side changes altitude as soon as the planer steps on it (at the end) or if he does some work there as he does on the other nodes.

So now the building site has the same level as all nodes around it, while the flag could be different before.

[libv]

The question was if the building side changes altitude as soon as the planer steps on it (at the end) or if he does some work there as he does on the other nodes.

It changes at the same time the planer steps on the central building tile/site/point, and no further work is done, he just walks off over the flag.