Transport Tycoon Forums

The place to talk about Transport Tycoon
It is currently Sat Mar 23, 2019 2:03 pm

All times are UTC




Post new topic  Reply to topic  [ 88 posts ]  Go to page 1 2 3 4 5 Next
Author Message
PostPosted: Tue Jun 24, 2008 7:55 am 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
Below you can find the patches diagonal levelling. The "tileiter" patch only implements the tile iterators which can be reused for other purposes. It also rewrites the normal levelling and clearing methods to use the orthogonal iterator. The "diaglvl" patch implements the actual levelling and clearing on top of the diagonal iterator. For convenience I also provide an all-in-one patch implementing both. Thanks for your feedback.

You can also pull them from my git repository at git://github.com/fonsinchen/openttd-cargodist.git
Each patch has its own branch. The branches are called "tileiter" and "diaglvl".
An up-to-date all-in-one patch at at http://github.com/fonsinchen/openttd-ca ... aglvl.diff
It applies to the trunk version given in http://github.com/fonsinchen/openttd-ca ... ERSION.txt

_________________
The guy on the picture is not me, it's Alonso.


Last edited by fonso on Thu Jun 10, 2010 9:18 pm, edited 5 times in total.

Top
   
PostPosted: Tue Jun 24, 2008 8:36 am 
Offline
OpenTTD Developer
OpenTTD Developer
User avatar

Joined: Mon Jun 09, 2003 6:21 pm
Posts: 4549
Location: /home/sweden
I'll try.

One argument can be that there are people that don't know of all fancy CTRL-options available in OpenTTD. If you follow the development of OpenTTD you'll learn the new CTRL behaviors one after another but if you are new to OpenTTD it can take long time to find all various nice features that are "hidden" on the CTRL-key.

I don't know if anyone have vented any such comments in the previous thread but sometimes people do vent such things on patches. But add a note that CTRL toggles diagonal operation in the tooltip of the affected buttons in the landscape toolbar and it should be enough I think. (Let's leave out the discussion about if tooltips should be shown automatically or not)


EDIT: I did my homework and read through two old thread on the subject and only found one argument. That there is another patch using CTRL but it was argued that diagonal leveling is more useful and therefor should use CTRL. viewtopic.php?p=668831#p668831

_________________
My OpenTTD contributions (AIs, Game Scripts, patches, OpenTTD Auto Updater, and some sprites)
Junctioneer (a traffic intersection simulator)


Top
   
PostPosted: Tue Jun 24, 2008 9:12 am 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
Ah, I see, thanks. Somehow I didn't realize they meant the drag&draw patch. Why does the drag&draw patch need the CTRL key anyway? Would anyone notice if drag&draw became the default behaviour for raise and lower? I mean, it's pretty much what you'd expect when keeping the mouse button pressed and moving the cursor around, isn't it? Like that only explicit levelling would actually do levelling. At the same time I wouldn't have to care about diagonal raise and lower. And as an additional benefit we'd get rid of the problem that raise can lower terrain in certain situations (and the other way round). Perhaps we should have a poll: Who is actually using area raise and lower instead of raising or lowering one tile and then using level from there?

We still need a solution for drag&draw levelling then ... any ideas? Do we need it?

_________________
The guy on the picture is not me, it's Alonso.


Top
   
PostPosted: Tue Jun 24, 2008 12:15 pm 
Offline
Tycoon
Tycoon

Joined: Sun Apr 08, 2007 4:07 am
Posts: 1884
Well, I've been trying my luck with the devs about my drag&draw terraform before, but they didn't seem too interested.. even when I tuned it down to a scedit-only version that doesn't use CTRL, after one of them proposed that to me (they seem reluctant to add stuff that uses CTRL), I got no more feedback after that.

Sorry for having not updated it for a while, been busy with university lately...

edit: okay, I have to withdraw the part about no answer.. SmatZ wrote an answer in my flyspray task 2 days ago. ;)

_________________
* @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


Top
   
PostPosted: Tue Jul 01, 2008 6:15 pm 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
A somewhat simplified version of the same patch. Does the same thing, but eliminates the if-else ugliness. The issue with consistency and CTRL usage isn't resolved due to lack of better ideas. Perhaps I should take a look at that Flyspray thing, too ...


Attachments:
diagonal_demolish_and_leveling_r13670.diff [17.32 KiB]
Downloaded 172 times

_________________
The guy on the picture is not me, it's Alonso.
Top
   
PostPosted: Tue Jul 01, 2008 6:32 pm 
Offline
OpenTTD Developer
OpenTTD Developer
User avatar

Joined: Wed Nov 07, 2007 10:44 pm
Posts: 9213
Location: Sol d
fonso wrote:
Who is actually using area raise and lower instead of raising or lowering one tile and then using level from there?

I do :). But that's indeed something I could live without, if I trade diagonal terraform tools for that :)

_________________
Image
OpenTTD: manual | online content | translations | Wanted contributions and patches
#openttdcoop: blog | wiki | public server | DevZone | NewGRF web translator
DevZone - home of the free NewGRFs: OpenSFX | OpenMSX | OpenGFX | Swedish Rails | OpenGFX+ Trains|RV|Industries|Airports|Landscape | NML


Top
   
PostPosted: Thu Jul 31, 2008 7:44 am 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
New version with some cosmetic changes.


Attachments:
diagonal_demolish_and_leveling_r13890.diff [17.35 KiB]
Downloaded 205 times

_________________
The guy on the picture is not me, it's Alonso.
Top
   
PostPosted: Thu Jul 31, 2008 5:38 pm 
Offline
Tycoon
Tycoon
User avatar

Joined: Sat Apr 24, 2004 10:43 am
Posts: 2007
Location: Venezia - Italia
planetmaker wrote:
fonso wrote:
Who is actually using area raise and lower instead of raising or lowering one tile and then using level from there?

I do :). But that's indeed something I could live without, if I trade diagonal terraform tools for that :)

I would trade both the diagonal etc tool and the raise/lower area tool for the "paint" (drag&draw) tool

_________________
ImageImage
ImageImage
ImageImage
newgrf tests, see: newgrf test topic, my patches: Wolf01's Hire Point
Italian TTD Forum
Wiki Page


Top
   
PostPosted: Wed Aug 06, 2008 3:44 pm 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
renamed some variables as requested. Fixed some indentation problems.


Attachments:
diaglvl_r14004.diff [18.14 KiB]
Downloaded 125 times

_________________
The guy on the picture is not me, it's Alonso.
Top
   
PostPosted: Thu Aug 07, 2008 9:02 pm 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
new version with iterators instead of duplicated code, flags for leveling modes and cosmetics.


Attachments:
diaglvl_r14012.diff [17.97 KiB]
Downloaded 131 times

_________________
The guy on the picture is not me, it's Alonso.
Top
   
PostPosted: Thu Aug 07, 2008 9:54 pm 
Offline
Tycoon
Tycoon

Joined: Thu Dec 20, 2007 12:49 pm
Posts: 3653
Nice patch, I hope this'll make it to trunk someday.

Some small comments about the style:

Code:
+   if (p2 & LEVEL_RAISE)
+      h++;
+   else if (p2 & LEVEL_LOWER)
+      h--;

You should use brackets if you put h++ on a different, and always for else statements. So it'll become.
Code:
+   if (p2 & LEVEL_RAISE) {
+      h++;
+   } else if (p2 & LEVEL_LOWER) {
+      h--;
+   }

You do this some more times in your patch.

Code:
DoCommandP(end_tile, start_tile, _ctrl_pressed, CcPlaySound10, CMD_CLEAR_AREA | CMD_MSG(STR_00B5_CAN_T_CLEAR_THIS_AREA));

Looks like you forgot to change _ctrl_pressed to _ctrl_pressed ? LEVEL_DIAG : 0 on this line, and for CmdClearArea in general.


Top
   
PostPosted: Fri Aug 08, 2008 3:16 am 
Offline
OpenTTD Developer
OpenTTD Developer
User avatar

Joined: Tue Apr 05, 2005 1:48 am
Posts: 1507
Location: Deep down the deepest blue
Sharp eye Yexo :)
Let say that this patch has never been closer to trunk inclusion :D

_________________
If you are not ready to work a bit for your ideas, it means they don't count much for you.
OpenTTD and Realism? Well... Here are a few thoughs on the matter.
He he he he
------------------------------------------------------------
Music from the Bloody Time Zones


Top
   
PostPosted: Fri Aug 08, 2008 8:17 am 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
Thanks Yexo.

The brackets thing was unknown to me. I know I should memorize the coding style better :(.

_________________
The guy on the picture is not me, it's Alonso.


Top
   
PostPosted: Fri Aug 08, 2008 8:39 am 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
Next version with all if-else bracketified, where needed and CmdClearArea also uses LevelMode (although I could argue about that ...).


Attachments:
diaglvl_r14018.diff [18.05 KiB]
Downloaded 160 times

_________________
The guy on the picture is not me, it's Alonso.
Top
   
PostPosted: Fri Aug 08, 2008 9:27 am 
Online
OpenTTD Developer
OpenTTD Developer

Joined: Wed Mar 30, 2005 9:43 am
Posts: 1588
I don't think LevelMode should be declared in command_type.h.

Another minor code-style thing; can you place the { after a function on a new line. There are still some missing braces around some if () statements. :)

_________________
He's like, some kind of OpenTTD developer.


Top
   
PostPosted: Fri Aug 08, 2008 12:06 pm 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
new version with (hopefully) all the brackets correct now, LevelMode divided into LevelMode and Orientation, iterating with *Iterator::end.

Edit: forgot the brackets at function start; corrected now.


Attachments:
diaglvl_r14020.diff [19.25 KiB]
Downloaded 150 times

_________________
The guy on the picture is not me, it's Alonso.
Top
   
PostPosted: Fri Aug 08, 2008 1:05 pm 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
To sum it up:

- blathijs likes it as it is now
- peter1138 finds the *Iterator::end confusing because it contains "random values"
- hylje considers the whole idea of C++ iterators braindead
- others are silent

I don't care. I'll happily make a java style iterator of it, or change it back so that you have to dereference it to get the end condition or insert a special "end" flag somewhere or even make a common base class which provides an empty "end" member or whatever. But I'm at a loss now. Please comment.

_________________
The guy on the picture is not me, it's Alonso.


Top
   
PostPosted: Fri Aug 08, 2008 5:38 pm 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
ok, now I have two alternative versions. One with java-style iterators which is very concise and clean and one with more elaborate c++-style iterators which provides an extra class InvalidIterator, so that I don't need to fill the invalid iterator with random stuff.


Attachments:
diaglvl_invit_r14026.diff [20.08 KiB]
Downloaded 110 times
diaglvl_java_r14026.diff [17.71 KiB]
Downloaded 103 times

_________________
The guy on the picture is not me, it's Alonso.
Top
   
PostPosted: Fri Aug 08, 2008 8:31 pm 
Offline
Transport Coordinator
Transport Coordinator
User avatar

Joined: Thu Feb 02, 2006 1:17 pm
Posts: 349
Location: Azewijn The netherlands
I like this idea, can i see a screenshot please?

_________________
Visit The Fake Airport Website
Image
Hobbys: being 18 years old, soccer, go karting, and transport tycoon.


Top
   
PostPosted: Sat Aug 09, 2008 10:12 am 
Offline
President
President
User avatar

Joined: Sat Oct 13, 2007 8:28 am
Posts: 945
RSpeed tycoonfreak wrote:
I like this idea, can i see a screenshot please?


Sure. Here are some. Forgive the strange screenshot windows ... I'll have to investigate that.


Attachments:
File comment: Highlight area for leveling
level_highlight.png [57.26 KiB]
Downloaded 520 times
File comment: and do the leveling
level_action.png [69.21 KiB]
Downloaded 485 times
File comment: *bang* (no highlight for demolish shown because of 3 file limit)
demolish_action.png [56.75 KiB]
Downloaded 469 times

_________________
The guy on the picture is not me, it's Alonso.
Top
   
Display posts from previous:  Sort by  
Post new topic  Reply to topic  [ 88 posts ]  Go to page 1 2 3 4 5 Next

All times are UTC


Who is online

Users browsing this forum: No registered users and 3 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB © 2000-2019 phpBB Limited

Copyright © Owen Rudge/The Transport Tycoon Forums 2001-2019.
Hosted by Zernebok Hosting.