45° rotated rectangle clearing / leveling patch

[boekabart]

I'd love this patch to enter the trunk/nightlies. But for that, it's still not clean enough, I'm afraid. Some comments:
* Why did the implementation of the 'normal rect' clear tool / level tool change? I would expect the "if (!p2)" part to remain unchanged, but it's completely rewritten... or old code put back?
* Why the static inline int sign( int x ), what's wrong with sgn(x) ?
* Compare the 'new' line:

Code: Select all

DoCommandP(end_tile, start_tile, _ctrl_pressed ? 1 : 0, CcPlaySound10, CMD_CLEAR_AREA | CMD_MSG(STR_00B5_CAN_T_CLEAR_THIS_AREA));

with the existing line:

Code: Select all

DoCommandP(end_tile, start_tile, _ctrl_pressed, CcBuildCanal, CMD_BUILD_CANAL | CMD_AUTO | CMD_MSG(STR_CANT_BUILD_CANALS));

Passing the parameter the same way would keep coding style more constant.

[boekabart]

... so that's why I modified it a little bit. Comments on code please! BTW I just realized there is no sgn function in the CRTs.. but still removed the one that was introduced here, solved it a little faster.

[Maedhros]

... so that's why I modified it a little bit. Comments on code please! BTW I just realized there is no sgn function in the CRTs.. but still removed the one that was introduced here, solved it a little faster.

Looks much better.

Am I right in thinking that the loop that used to be a macro has just been copied and pasted into two different places though? If so, that should almost certainly become a function instead...

[boekabart]

Am I right in thinking that the loop that used to be a macro has just been copied and pasted into two different places though? If so, that should almost certainly become a function instead...

Well it looks that way but the inner function is different: one raises/lowers the land, the other one clears it. Only functionable if the inner function is passed as a callback... not worth it I guess.

[Zojj]

What is the status on this patch? Does it need more work? Or is it shelved?

[BamBam]

There are changes in the code so the patch needs an update:

Edit 19. June 2007: update to r10227

[mamuf]

I wonder someone didn't implement this feature earlier and why it is not in trunk already. I really love it, thanks!

[Juhl]

As far as I can see this patch implements some generic diagonal dragging functions, which could be used when ever you want to drag?

I guess that there is a good reason why this isn't in trunk.

oh, and by the way: BamBam, thou art a caddish patch maintainer.

[mamuf]

As far as I can see this patch implements some generic diagonal dragging functions, which could be used when ever you want to drag?

I guess that there is a good reason why this isn't in trunk.

Well, why not to impement some better functions then? I really haven't read the sources but the feature itself is great. I can also imagine a circular selection (behaving the same way like when you draw circles in a bitmap editor). But I don't know if this could be really usefull and worth implenting.

[NukeBuster]

This function is really usefull when building diagonal railways on flat land.
You normally would have to excavate tile after tile to get diagonal paths flat. With this patch however, you just press ctrl and drag it into the direction you want.

[Bilbo]

This function is really usefull when building diagonal railways on flat land.
You normally would have to excavate tile after tile to get diagonal paths flat. With this patch however, you just press ctrl and drag it into the direction you want.

Or alternatively, excavate entire diagonal path at once. Along with huge rectangular area around it

This is useful, I hope it will get to trunk soon ...

[chrissicom]

Unfortunately there is an ugly bug in this patch (also in the latest version). When you apply this patch and try to demolish or terraform land while you have no money you get an error message just like before but additionaly the money your action would have cost is subtracted which gives you a big minus on your bank account when trying to demolish water for example
I have looked at the code but I can't find an obvious so maybe somebody with more C knowledge wants to look into this

[mamuf]

(...) somebody with more C knowledge wants to look into this

Or maybe someone who knows how the OTTD code works could take this idea and remake the original code to work with rotated grid . When I saw the code for the first time few days ago I was really wondering how does it actually work . (different part of it btw.)

[NukeBuster]

I've got the cash draining fixed.
Another bug which allowed you to level the entire map, without having nearly enough money, is also fixed.
However I'm now getting the patch up to the latest revision. Expect a release later today.

[chrissicom]

I have just updated my Integrated build which also includes this patch and it has the no money bug fixed
http://www.tt-forums.net/viewtopic.php?t=32698

I will check your update as well in case I have missed something and the bug can still occur in cases I did not think of and therefore did not test.

[NukeBuster]

Money bug also exists when clearing ground, see landscape.cpp.

Also.... try to level the whole map at once see if thats possible... (while pressing "ctrl" offcourse.) You should end up with a big negative amount.

The patch is attached to this post.

Trunk revision: 10341
Patch revision: 169
Edit:[strike]Fixed: Able to level entire map, ending in a big negative amount.[/strike]

Fixed: Money was still subtracted while error message apeared when levelling.
Fixed: Money was still subtracted while error message apeared when clearing.

[NukeBuster]

Apperently I forgot to add the neccesary checks in the clean version I made after fixing the bugs.

So, here is the new update.

Trunk revision: 10341
Patch revision: 170

Fixed: Able to level entire map or big parts of the map(while pressing ctrl), ending in a big negative amount.

[NukeBuster]

And so 2 bugs are fixed, thanks to chrissicom for spotting them.

Here is the new patch.

Fixed: Explosions weren't drawn when clearing. (Diagonal clearing only draws 2 explosions at the moment.)
Fixed: Cost estimates always returned 0 as cost.

Patched against r10362
Patch revision 171

Edit: Deleted the latest patch, did not see conflicted state before patching. Busy bringing patch up to date.

As for something else, my compiler gives me this warning:

Code: Select all

[SRC] Compiling viewport.cpp
/.../src/viewport.cpp: In function ‘bool DraggingDiagonal()’:
/.../src/viewport.cpp:2016: let op: left-hand operand of comma has no effect
/.../src/viewport.cpp:2017: let op: left-hand operand of comma has no effect

Which affects this code:

Code: Select all

static inline bool DraggingDiagonal()
{
	return _ctrl_pressed && ((_thd.select_proc == (VPM_X_AND_Y, DDSP_DEMOLISH_AREA)) ||
	                         (_thd.select_proc == (VPM_X_AND_Y, DDSP_LEVEL_AREA)));
}

What is wrong with the above code sample?

[NukeBuster]

I am now working on getting the explosions in the 2 other corners. Which is not the easiest thing I've ever done.

New patch will probably be finished in the afternoon.

[NukeBuster]

Now we've got 4 explosions this was already the case when you cleared using the normal rectangle. But now, it will also happen with 45 degrees clearing . It took me some time to understand the code, but it works just fine now.

Anyway, here is the new patch. And a screenie for the people watching at home.