Patch: wayremover

[Dwachs]

This small patch adds functionality to double-clicking with the way remover tool. Instead of showing an error ('ways are not connected') the corresponding way is deleted on this tile.

Code Select Expand

Index: simwerkz.cc
===================================================================
--- simwerkz.cc (revision 2489)
+++ simwerkz.cc (working copy)
@@ -1650,6 +1650,11 @@
delete wkz_wayremover_bauer;
wkz_wayremover_bauer = NULL;
erster = true;
+
+ // if start = end then remove the entire way on this tile
+ if (start == gr->get_pos()) {
+ return gr->remove_everything_from_way(sp, wt, ribi_t::keine) ? NULL : "";
+ }

// get a default vehikel
fahrer_t* test_driver;


Are there any objections against putting this into trunk?

[VS]

Great idea, small and simple, but makes life easier nevertheless.

I somehow had to imagine somebody doubleclicking his railway, tile by tile and then reporting "hard to remove tracks"

[z9999]

This is bad.
Double click on bridge end will not remove bridge. Tunnel is the same.
Double click on middle of bridge also removes a part of bridge.

[Dwachs]

  my fault. but a test to exclude tunnel and bridge tiles should be easy.

Code Select Expand


Index: simwerkz.cc
===================================================================
--- simwerkz.cc (revision 2489)
+++ simwerkz.cc (working copy)
@@ -1651,30 +1651,38 @@
wkz_wayremover_bauer = NULL;
erster = true;

- // get a default vehikel
- fahrer_t* test_driver;
- if(  wt!=powerline_wt  ) {
- vehikel_besch_t remover_besch(wt, 500, vehikel_besch_t::diesel );
- test_driver = vehikelbauer_t::baue(start, sp, NULL, &remover_besch);
+ route_t verbindung;
+ bool can_delete;
+ if (start != gr->get_pos()) {
+ // get a default vehikel
+ fahrer_t* test_driver;
+ if(  wt!=powerline_wt  ) {
+ vehikel_besch_t remover_besch(wt, 500, vehikel_besch_t::diesel );
+ test_driver = vehikelbauer_t::baue(start, sp, NULL, &remover_besch);
+ }
+ else {
+ test_driver = (fahrer_t * )new electron_t();
+ }
+ can_delete = verbindung.calc_route(welt, start, gr->get_pos(), test_driver, 0);
+ delete test_driver;
+ welt->show_distance = start = koord3d::invalid;
+ DBG_MESSAGE("wkz_wayremover()", "route search returned %d", can_delete);
+
+ if(!can_delete) {
+ DBG_MESSAGE("wkz_wayremover()","no route found");
+ return "Ways not connected";
+ }
}
else {
- test_driver = (fahrer_t * )new electron_t();
+ // if start = end then remove the entire way on this tile
+ verbindung.append(start);
+ can_delete= true;
}
- route_t verbindung;
- bool can_delete = verbindung.calc_route(welt, start, gr->get_pos(), test_driver, 0);
- delete test_driver;
- welt->show_distance = start = koord3d::invalid;
- DBG_MESSAGE("wkz_wayremover()", "route search returned %d", can_delete);

- if(!can_delete) {
- DBG_MESSAGE("wkz_wayremover()","no route found");
- return "Ways not connected";
- }
-
DBG_MESSAGE("wkz_wayremover()","route with %d tile found",verbindung.get_max_n());

// found a route => check if I can delete anything on it
- for(  uint32 i=0;  can_delete  &&  i<=verbindung.get_max_n();  i++  ) {
+ for(  uint32 i=0;  can_delete  &&  i<verbindung.get_max_n();  i++  ) {
grund_t *gr=welt->lookup(verbindung.position_bei(i));
if(gr) {
if(  wt!=powerline_wt  ) {
@@ -1706,7 +1714,7 @@
}

// if successful => delete everything
- for( uint32 i=0;  can_delete  &&  i<=verbindung.get_max_n();  i++  ) {
+ for( uint32 i=0;  can_delete  &&  i<verbindung.get_max_n();  i++  ) {

grund_t *gr=welt->lookup(verbindung.position_bei(i));

@@ -1732,6 +1740,10 @@
ribi_t::ribi rem = (i>0) ? ribi_typ( verbindung.position_bei(i).get_2d(), verbindung.position_bei(i-1).get_2d() ) : 0;
ribi_t::ribi rem2 = (i<verbindung.get_max_n()) ? ribi_typ(verbindung.position_bei(i).get_2d(),verbindung.position_bei(i+1).get_2d()) : 0;
rem = ~(rem|rem2);
+ // if start=end tile then delete every direction
+ if (verbindung.get_max_n()==1) {
+ rem = 0;
+ }

if(  wt!=powerline_wt  ) {
if(!gr->get_flag(grund_t::is_kartenboden)  &&  (gr->get_typ()==grund_t::tunnelboden  ||  gr->get_typ()==grund_t::monorailboden)  &&  gr->get_weg_nr(0)->get_waytype()==wt) {

Now the patch looks much more massive, but most of the changes are due to indentation. It now uses the original remover logic but adapted to the special situation start=end.

[Fabio]

two suggestions:
-general remover tool only removes STATIONS, buildings, signals, trees, etc... but NO way.
-way remover tools as the only mean to remove a way (double click for tile-by tile removal)
this should help avoiding casual deletion and would make clear what to delete in case you click on a tile with more than an object (on a road: general removal to remove the station, way removal to remove the way (and also the station, obviously). also in a crossing, being able to use only the way-specific removal, should help to choose what to delete.

second suggestion: shortcuts.
if a way tool is selected (e.g. roads), pressing 'r' should select its respective removal tool. if other or no tool is selected, general removal. to select a specific way-removal tool (e.g. road-removal), just press the two shourcuts (e.g. s and r).

[jamespetts]

For keyboard shortcuts, there should always be a GUI alternative so as to make the full functionality obvious to new users who have not read all the documentation.

[prissi]

Not patching at all, I am afraid. Needs update.

[gerw]

It doesn't work because of my dragging patch, therefore I've updated the patch.

Code Select Expand

Index: simwerkz.cc
===================================================================
--- simwerkz.cc (revision 2514)
+++ simwerkz.cc (working copy)
@@ -1547,18 +1547,26 @@

bool wkz_wayremover_t::calc_route( route_t &verbindung, spieler_t *sp, const koord3d &start, const koord3d &end )
{
-       waytype_t wt = (waytype_t)atoi(default_param);
-       // get a default vehikel
-       fahrer_t* test_driver;
-       if(  wt!=powerline_wt  ) {
-               vehikel_besch_t remover_besch(wt, 500, vehikel_besch_t::diesel );
-               test_driver = vehikelbauer_t::baue(start, sp, NULL, &remover_besch);
+       bool can_delete;
+       if( start == end ) {
+               verbindung.clear();
+               verbindung.append( start );
+               can_delete = true;
        }
        else {
-               test_driver = (fahrer_t * )new electron_t();
+               waytype_t wt = (waytype_t)atoi(default_param);
+               // get a default vehikel
+               fahrer_t* test_driver;
+               if(  wt!=powerline_wt  ) {
+                       vehikel_besch_t remover_besch(wt, 500, vehikel_besch_t::diesel );
+                       test_driver = vehikelbauer_t::baue(start, sp, NULL, &remover_besch);
+               }
+               else {
+                       test_driver = (fahrer_t * )new electron_t();
+               }
+               can_delete = verbindung.calc_route(sp->get_welt(), start, end, test_driver, 0);
+               delete test_driver;
        }
-       bool can_delete = verbindung.calc_route(sp->get_welt(), start, end, test_driver, 0);
-       delete test_driver;
        DBG_MESSAGE("wkz_wayremover()", "route search returned %d", can_delete);
        DBG_MESSAGE("wkz_wayremover()","route with %d tile found",verbindung.get_max_n());
        return can_delete;
@@ -1622,18 +1630,24 @@
                                                if(err) {
                                                        return err;
                                                }
+                                               gr=welt->lookup(verbindung.position_bei(i));
                                        }
-                                       // do not remove asphalt from a bridge ...
-                                       continue;
+                                       else{
+                                               // do not remove asphalt from a bridge ...
+                                               continue;
+                                       }
                                }
                        }

                        // now the tricky part: delete just part of a way (or everything, if possible)
                        // calculated removing directions
-                       ribi_t::ribi rem = (i>0) ? ribi_typ( verbindung.position_bei(i).get_2d(), verbindung.position_bei(i-1).get_2d() ) : 0;
-                       ribi_t::ribi rem2 = (i<verbindung.get_max_n()) ? ribi_typ(verbindung.position_bei(i).get_2d(),verbindung.position_bei(i+1).get_2d()) : 0;
-                       rem = ~(rem|rem2);
+                       ribi_t::ribi rem = ~( verbindung.get_route().get_ribi(i) );

+                       // if start=end tile then delete every direction
+                       if( verbindung.get_max_n() == 1 ) {
+                               rem = 0;
+                       }
+
                        if(  wt!=powerline_wt  ) {
                                if(!gr->get_flag(grund_t::is_kartenboden)  &&  (gr->get_typ()==grund_t::tunnelboden  ||  gr->get_typ()==grund_t::monorailboden)  &&  gr->get_weg_nr(0)->get_waytype()==wt) {
                                        can_delete &= gr->remove_everything_from_way(sp,wt,rem);


@prissi:
Why is the last koord3d in route_t always doubled? Is this needed anywhere?

Edit + Update:
Now it also removes bridges correctly. There was a bug, if the way in front of a bridge wasn't fully removed, see situation in http://www-user.tu-chemnitz.de/~gerw/patches/bridge_remove.png

[Dwachs]

thank you!

[prissi]

The last koord3d is doubled since the route index could became larger than the last tile. To avoid crashes the last tile was doubled (before 84.x versions) I just got the code this way and left it this way. If you want to clean this up, go ahead ...

[gerw]

To avoid crashes

You mean, if somebody accesses route[i+1] without checking if i+1<max_n? That's not a good reason, so I will have a look for it.

I'm also don't like the max_n, because it's a little bit against the common C convention, that the first index is 0 and the last is count-1. Here, the last index is max_n. So I think, we should replace it by a get_count, is this OK?

[prissi]

It could be changed to a vector_tpl, when you do the renovation anyway ...

[gerw]

@prissi: can you also commit this part of my last patch:

Code Select Expand

Index: simwerkz.cc
===================================================================
--- simwerkz.cc (revision 2514)
+++ simwerkz.cc (working copy)
@@ -1622,18 +1630,24 @@
                                                if(err) {
                                                        return err;
                                                }
+                                               gr=welt->lookup(verbindung.position_bei(i));
                                        }
-                                       // do not remove asphalt from a bridge ...
-                                       continue;
+                                       else{
+                                               // do not remove asphalt from a bridge ...
+                                               continue;
+                                       }
                                }
                        }

Have a look at http://www-user.tu-chemnitz.de/~gerw/patches/bridge_remove.png. If removing the bridge this way, there will be a tile left.