Area Terraform (in trunk r11759)

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

Do you like the idea of this patch?

Yes, this is useful.
38
83%
No, I don't think that's useful.
4
9%
I don't know...
4
9%
 
Total votes: 46

Roujin
Tycoon
Tycoon
Posts: 1884
Joined: 08 Apr 2007 04:07

Area Terraform (in trunk r11759)

Post by Roujin »

Sooo, i decided today (actually yesterday, past midnight here already :lol: ) to get into coding for OTTD, and made this patch.

It changes the behavior of the raise land and lower land tools to be drag- and dropable. I thought this would be nice to have because i always neglected them and used the level land tool instead.

You can still use them to raise or lower a single dot (only that now it's done on mouse_release not mouse_click) - and new: you can drag an area, and the whole area will be leveled to the level of starttile +1 or -1 when you raise or lower, respectively.


Note that it does not raise/lower each dot exactly by one. I thought about which of the two behaviors i want to make, and took the more level-ish one, because:
1) I think it's more intuitive / useful
2) It was easier to code :wink:

---

Codewise, i decided to give CmdLevelLand a second parameter (it was unused before) that specifies the difference of height to level. e.g. if called with 0, it just levels (old behavior, used by level). if called with -1, it levels to start_tile's height -1 (used by lower). if called with +1 ut levels to start_tile's height +1 (used by raise).
theoretically, you could call it with +5 and it'll still work, leveling all to 5 higher than the start tile. but that is never used.

sooo what do you think? all kinds of reviews welcome :lol:

---
Known Bugs:
none atm

---
Screenshots
Lowering a square of 7x2 edges.
Lowering a square of 7x2 edges.
AreaTerraform1.png (205.69 KiB) Viewed 8112 times
Raising a sqare of 2x8 edges, some are raised more than 1 level, to get the same height overall.
Raising a sqare of 2x8 edges, some are raised more than 1 level, to get the same height overall.
AreaTerraform2.png (204.83 KiB) Viewed 8112 times
---
Credits
Thanks to:
boekabart, dihedral and frosch for your kind help :)
all who tested this and gave me feedback 8)

---
edit: Screenshots.
edit: Known Bug.
edit: Credits.
edit: new version.
Attachments
area_terraform_r11754.diff
(3.5 KiB) Downloaded 154 times
Last edited by Roujin on 04 Jan 2008 19:02, edited 9 times in total.
* @Belugas wonders what is worst... a mom or a wife...
<Lakie> Well, they do the same thing but the code is different.

______________
My patches
check my wiki page (sticky button) for a complete list

ImageImage
ImageImageImageImageImageImageImage
User avatar
athanasios
Tycoon
Tycoon
Posts: 3138
Joined: 23 Jun 2005 00:09
Contact:

Re: Area Terraform (my first patch)

Post by athanasios »

To be honest I will not compile this patch, but it is very cool and I like such a feature to be in trunk! :D
http://members.fortunecity.com/gamesart
"If no one is a fool I am also a fool." -The TTD maniac.


I prefer to be contacted through PMs. Thanks.
User avatar
burty
Transport Coordinator
Transport Coordinator
Posts: 326
Joined: 16 Jun 2006 17:18
Location: Somwhere near a computer

Re: Area Terraform (my first patch)

Post by burty »

By what i have heard about adding patches with Tortoisesvn is right click and appply patch which i did and then compile and so far it worked the files were updated and it compiled loaded a new game went into the landscapeing tools and tried to drag and drop increas/decrease some land and it didnt work it just worked like the old way???
Hope this was helpfull!
Roujin
Tycoon
Tycoon
Posts: 1884
Joined: 08 Apr 2007 04:07

Re: Area Terraform (my first patch)

Post by Roujin »

hm i don't know why it wouldn't work... has someone else tested it already? :| anyoooone? *echo* :lol:


edit: misusing old post for uploading image
Attachments
patches_v1_title.png
patches_v1_title.png (493 Bytes) Viewed 46890 times
patches_v1_wiki.png
patches_v1_wiki.png (266 Bytes) Viewed 512127 times
patches_v1_seperator.png
patches_v1_seperator.png (211 Bytes) Viewed 515249 times
Last edited by Roujin on 01 Mar 2008 13:17, edited 1 time in total.
* @Belugas wonders what is worst... a mom or a wife...
<Lakie> Well, they do the same thing but the code is different.

______________
My patches
check my wiki page (sticky button) for a complete list

ImageImage
ImageImageImageImageImageImageImage
oVRoM
Engineer
Engineer
Posts: 37
Joined: 11 Jan 2003 10:40
Location: Utah

Re: Area Terraform (my first patch)

Post by oVRoM »

I've just compiled it; works fine here.
Roujin
Tycoon
Tycoon
Posts: 1884
Joined: 08 Apr 2007 04:07

Re: Area Terraform (my first patch)

Post by Roujin »

okaay... and? do you like it? some feedback would be really nice, even if it says my patch is utter crap :lol:

am I really the only one who thinks that the current raise/lower tool are missing area usability? :? there doesn't seem to be much interest in this :(

edit: misusing post for images
Attachments
patches_v1_patch1.png
patches_v1_patch1.png (605 Bytes) Viewed 46862 times
patches_v1_patch2.png
patches_v1_patch2.png (926 Bytes) Viewed 46855 times
patches_v1_patch3.png
patches_v1_patch3.png (661 Bytes) Viewed 512027 times
Last edited by Roujin on 01 Mar 2008 15:56, edited 1 time in total.
* @Belugas wonders what is worst... a mom or a wife...
<Lakie> Well, they do the same thing but the code is different.

______________
My patches
check my wiki page (sticky button) for a complete list

ImageImage
ImageImageImageImageImageImageImage
oVRoM
Engineer
Engineer
Posts: 37
Joined: 11 Jan 2003 10:40
Location: Utah

Re: Area Terraform (my first patch)

Post by oVRoM »

To be honest, I can't really think of a moment when I'd use this new behaviour...
Levelling land is one thing, but levelling and also (de|in)creasing the height doesn't seem that useful to me.
Mchl
Director
Director
Posts: 611
Joined: 05 Jan 2007 15:50
Location: Poland
Contact:

Re: Area Terraform (my first patch)

Post by Mchl »

I think it would be pretty useful.
It might save you two clicks in several situations. And two clicks * several situations is a lot of time saved.
Roujin
Tycoon
Tycoon
Posts: 1884
Joined: 08 Apr 2007 04:07

Re: Area Terraform (my first patch)

Post by Roujin »

well, sometimes i want to lower or raise some adjacent edges, say if i want to make a tunnel entrance (two edges down) or a hill or some sorts (possibly several edges up) - currently for the first example i would click two times with lower tool, for the latter example i would probably raise one tile and then switch to the level land tool.

the behavior of my patch would reduce this a bit, especially the "raise one edge then switch to level tool to make the rest" which i personally find annoying.

I know it's not the ultra super duper new feature ( :lol: ), but i do think there are some little situations where this can be useful :)
Plus, it doesn't break any old behavior, except that the single tile raise is now on mouse_release instead of mouse_down, which I think is not really that bothersome.

thanks for your feedback :)


PS: Maybe I should make a vote to get an overall impression if people find this useful or not...

edit: misusing post for images
Attachments
patches_v1_patch4.png
patches_v1_patch4.png (596 Bytes) Viewed 512544 times
patches_v1_patch5.png
patches_v1_patch5.png (620 Bytes) Viewed 46879 times
patches_v1_patch6.png
patches_v1_patch6.png (429 Bytes) Viewed 511806 times
Last edited by Roujin on 01 Mar 2008 15:57, edited 1 time in total.
* @Belugas wonders what is worst... a mom or a wife...
<Lakie> Well, they do the same thing but the code is different.

______________
My patches
check my wiki page (sticky button) for a complete list

ImageImage
ImageImageImageImageImageImageImage
boekabart
Transport Coordinator
Transport Coordinator
Posts: 333
Joined: 25 Aug 2005 09:44
Location: Eindhoven, Netherlands

Re: Area Terraform (now with vote)

Post by boekabart »

Good patch, I don't see what anyone can have against it, in fact.

Coding style is flawless, but there is one thing I would do:

Rename the enums to match what they do a little more precisely:
DDSP_RAISE_AREA -> DDSP_LEVEL_AND_RAISE_AREA
(and similar for LOWER of course)

Raise Area would do the other thing you describe (probably less useful, indeed) - raise the area by 1 level.

Congrats on this patch - If I were you, I'd add it as a feature 'bug' in bugs.openttd.org so you make sure the devs see it!
Moriarty
Tycoon
Tycoon
Posts: 1395
Joined: 12 Jun 2004 00:37
Location: United Kingdom of Great Britain and Northern Ireland
Contact:

Re: Area Terraform (now with vote)

Post by Moriarty »

That looks nice, but can it do diagnals? Because I can do straights easily enough using the "level" tool (admittedly cheating, and this is better, but still). This leaves diagnals as requiring lots of clicks, just like straights used to use too.
Roujin
Tycoon
Tycoon
Posts: 1884
Joined: 08 Apr 2007 04:07

Re: Area Terraform (now with vote)

Post by Roujin »

@boekabart:
Coding style is flawless
that's because i didn't have to write much myself :lol: just reused stuff and changed behavior a little trying not to break existing coding style :mrgreen:
DDSP_RAISE_AREA -> DDSP_LEVEL_AND_RAISE_AREA
will change that, thanks for suggestion. though LEVEL_AND_RAISE_AREA sounds to me like it is used for leveling and for raising, not for one action called "level and raise". i don't know how to name it else to make it clear... maybe DDSP_LEVELRAISE_AREA?
Congrats on this patch - If I were you, I'd add it as a feature 'bug' in bugs.openttd.org so you make sure the devs see it!
Thanks :) I was hesitating about that, because i got the impression there's not much interest in this patch. But now with all the positive answers, I'll add it to flyspray. But not just now, because i found a bug myself yet to fix. (see below)

---

@Moriarty: i think someone else made a patch for diagonal levelling. I don't know what's the current status, but if it works i could have a look at it and maybe try to combine it with my patch, with author's permission. CTRL is unused for raise and lower, so that could also be used to toggle from rectangular to diagonal.

But that is for later on, first i have to fix bug. Thanks for the suggestion though.

---

Bug i found myself and am fixing now: lowering under min_altitude freezes game. raising over max altitude doesn't show an error (does nothing).
I just forgot to think about these cases while reusing/altering the level command :roll: of course leveling can't possibly go under min_altitude or over max_altitude :lol:


edit: misusing post for images
Attachments
patches_v1_patch7.png
patches_v1_patch7.png (588 Bytes) Viewed 513145 times
patches_v1_patch8.png
patches_v1_patch8.png (558 Bytes) Viewed 46889 times
patches_v1_patch9.png
patches_v1_patch9.png (571 Bytes) Viewed 46938 times
Last edited by Roujin on 01 Mar 2008 15:58, edited 1 time in total.
* @Belugas wonders what is worst... a mom or a wife...
<Lakie> Well, they do the same thing but the code is different.

______________
My patches
check my wiki page (sticky button) for a complete list

ImageImage
ImageImageImageImageImageImageImage
Roujin
Tycoon
Tycoon
Posts: 1884
Joined: 08 Apr 2007 04:07

Re: Area Terraform (now with vote)

Post by Roujin »

new version, bumped to r11278

Changes:
- renamed DDSP_RAISE_AREA -> DDSP_RAISE_AND_LEVEL_AREA
- renamed DDSP_LOWER_AREA -> DDSP_LOWER_AND_LEVEL_AREA

Bugfixes:
- lowering below zero and raising above max_height now properly shows error


---

another thing i was thinking about was which sound to use, the one for terraforming, or the explosion sound the level tool uses. I decided to stick with the old one ("crrrrrp" ;) ) for now, also because of consistency.

i think i fixed all bugs now, and hopefully didn't introduce any new ones ;) please tell me if you find anything, if nothing shows up, i'll consider adding it to flyspray to have the devs have a look at it


download @first post
* @Belugas wonders what is worst... a mom or a wife...
<Lakie> Well, they do the same thing but the code is different.

______________
My patches
check my wiki page (sticky button) for a complete list

ImageImage
ImageImageImageImageImageImageImage
Roujin
Tycoon
Tycoon
Posts: 1884
Joined: 08 Apr 2007 04:07

Re: Area Terraform (new version r11278)

Post by Roujin »

bumped to r11754.
* @Belugas wonders what is worst... a mom or a wife...
<Lakie> Well, they do the same thing but the code is different.

______________
My patches
check my wiki page (sticky button) for a complete list

ImageImage
ImageImageImageImageImageImageImage
User avatar
Octopussy
Engineer
Engineer
Posts: 122
Joined: 01 May 2006 21:57

Re: Area Terraform (r11754)

Post by Octopussy »

This patch can be very usefull. Nice.
User avatar
rbenevid
Traffic Manager
Traffic Manager
Posts: 192
Joined: 14 Oct 2004 20:08
Location: Brazil

Re: Area Terraform (r11754)

Post by rbenevid »

I think it's a nice patch. It's much more intuitive than lots of clicks or one click with raise tool, then drag with level tool (when raising an area).
User avatar
planetmaker
OpenTTD Developer
OpenTTD Developer
Posts: 9432
Joined: 07 Nov 2007 22:44
Location: Sol d

Re: Area Terraform (r11754)

Post by planetmaker »

rbenevid wrote:I think it's a nice patch. It's much more intuitive than lots of clicks or one click with raise tool, then drag with level tool (when raising an area).
I do agree. I frequently wonder why the lower and raise work differently from the level tool. I hope it will make it into trunk.
peter1138
OpenTTD Developer
OpenTTD Developer
Posts: 1732
Joined: 30 Mar 2005 09:43

Re: Area Terraform (r11754)

Post by peter1138 »

planetmaker wrote:I hope it will make it into trunk.
It has! In r11759...
He's like, some kind of OpenTTD developer.
LordAzamath
Tycoon
Tycoon
Posts: 1656
Joined: 08 Jun 2007 08:00

Re: Area Terraform (r11754)

Post by LordAzamath »

<CIA-1> OpenTTD: peter1138 * r11759 /trunk/src/ (clear_cmd.cpp gui.h main_gui.cpp terraform_gui.cpp): -Feature: Add drag-n-drop support to the raise/lower land tools. Land is raised/lowered at the start and the rest of the area levelled to match. Patch by Roujin.
Yaaay
PS: And I stopped the propaganda to support Dave Worley since he got a nice new red hat now.[/color]
I know I have a BBCode error in my signature but I really cba to fix it.
mcbane
Engineer
Engineer
Posts: 15
Joined: 25 Sep 2006 20:25
Location: Munich

Re: Area Terraform (r11754)

Post by mcbane »

congratulation on this nice feat.
Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 54 guests