Patch: wayremover

Topic: Patch: wayremover (Read 4732 times) previous topic - next topic

Patch: wayremover

May 25, 2009, 07:00:22 pm

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]

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?

Re: Patch: wayremover

Reply #1 – May 25, 2009, 07:27:41 pm

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"

Re: Patch: wayremover

Reply #2 – May 25, 2009, 07:36:57 pm

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.

Re: Patch: wayremover

Reply #3 – May 25, 2009, 07:42:34 pm

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

Code: [Select]

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 m****ive, but most of the changes are due to indentation. It now uses the original remover logic but adapted to the special situation start=end.

Re: Patch: wayremover

Reply #4 – May 27, 2009, 01:08:22 pm

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).

Re: Patch: wayremover

Reply #5 – May 27, 2009, 01:45:24 pm

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.

Re: Patch: wayremover

Reply #6 – June 11, 2009, 09:09:54 pm

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

Re: Patch: wayremover

Reply #7 – June 12, 2009, 06:46:54 am

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

Code: [Select]

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

Re: Patch: wayremover

Reply #8 – June 12, 2009, 07:43:39 am

thank you!

Re: Patch: wayremover

Reply #9 – June 12, 2009, 01:10:59 pm

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 ...

Re: Patch: wayremover

Reply #10 – June 12, 2009, 01:20:33 pm

Quote from: prissi – on June 12, 2009, 01:10:59 pm

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?

Re: Patch: wayremover

Reply #11 – June 12, 2009, 08:09:14 pm

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

Re: Patch: wayremover

Reply #12 – June 17, 2009, 09:01:34 pm

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

Code: [Select]

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.