wegbauer_t::weg

wegbauer_t::weg_search

May 22, 2009, 07:21:45 am

In r2487, lines 137+138 are logically identical and line 139 doesn't do, what the comment implies. I think these lines should read

Code: [Select]

( best->get_topspeed() <      speed_limit       &&      speed_limit      <= test->get_topspeed()) || // test is faster than speed_limit
( best->get_topspeed() <  test->get_topspeed()  &&  test->get_topspeed() <=     speed_limit  )    || // closer to desired speed (from the low end)
(     speed_limit      <= test->get_topspeed()  &&  test->get_topspeed() <  best->get_topspeed()) || // closer to desired speed (from the top end)

In r2487, if no way fulfills the timeline, the first way found is returned. This is also changed in the patch.

Patch:

Code: [Select]

Index: bauer/wegbauer.cc
===================================================================
--- bauer/wegbauer.cc   (revision 2487)
+++ bauer/wegbauer.cc   (working copy)
@@ -120,26 +120,33 @@
 }
 
 
-/**
+/*
  * Finds a way with a given speed limit for a given waytype
+ * It finds:
+ *  - the slowest way, as fast as speed limit
+ *  - if no way faster than speed limit, the fastest way.
+ * The timeline is also respected.
  * @author prissi
  */
 const weg_besch_t* wegbauer_t::weg_search(const waytype_t wtyp, const uint32 speed_limit, const uint16 time, const weg_t::system_type system_type)
 {
        const weg_besch_t* best = NULL;
+       bool best_allowed = false; // Does the best way fulfill the timeline?
        for(  stringhashtable_iterator_tpl<const weg_besch_t*> iter(alle_wegtypen); iter.next();  ) {
                const weg_besch_t* const test = iter.get_current_value();
                if(  ((test->get_wtyp()==wtyp  &&
                             (test->get_styp()==system_type  ||  system_type==weg_t::type_all))  ||  (test->get_wtyp()==track_wt  &&  test->get_styp()==weg_t::type_tram  &&  wtyp==tram_wt))
                             &&  test->get_cursor()->get_bild_nr(1)!=IMG_LEER  ) {
-                       if(  best==NULL  ||  time==0  ||  (test->get_intro_year_month()<=time  &&  time<test->get_retire_year_month())) {
+                       bool test_allowed = test->get_intro_year_month()<=time  &&  time<test->get_retire_year_month();
+                       if(  !best_allowed  ||  time==0  ||  test_allowed  ) {
                                if(  best==NULL  ||
-                                               (best->get_topspeed() < speed_limit  &&  test->get_topspeed() <= speed_limit  &&  best->get_topspeed() < test->get_topspeed())  ||      // not yet there but this is ...
-                                               (test->get_topspeed() <=  speed_limit  &&  best->get_topspeed() < test->get_topspeed()) ||      // closer to desired speed (from the low end)
-                                               (best->get_topspeed() > speed_limit  &&  test->get_topspeed() < best->get_topspeed())  ||       // closer to desired speed (from the top end)
-                                               (time!=0  &&  (best->get_intro_year_month()>time  ||  time>=best->get_retire_year_month()))     // current choice is acutally not really allowed, timewise
+                                               ( best->get_topspeed() <      speed_limit       &&      speed_limit      <= test->get_topspeed()) || // test is faster than speed_limit
+                                               ( best->get_topspeed() <  test->get_topspeed()  &&  test->get_topspeed() <=     speed_limit  )    || // closer to desired speed (from the low end)
+                                               (     speed_limit      <= test->get_topspeed()  &&  test->get_topspeed() <  best->get_topspeed()) || // closer to desired speed (from the top end)
+                                               (time!=0  &&  !best_allowed  &&  test_allowed)                                                       // current choice is acutally not really allowed, timewise
                                        ) {
                                        best = test;
+                                       best_allowed = test_allowed;
                                }
                        }
                }

Re: wegbauer_t::weg_search

Reply #1 – June 26, 2009, 06:55:15 am

In my opinion, the commit in r2545 was wrong.

Code: [Select]

						(     speed_limit      <  best->get_topspeed()  &&  test->get_topspeed() <   best->get_topspeed()) || // respects speed_limit better

will select test, albeit it's slower than speed_limit.

Re: wegbauer_t::weg_search

Reply #2 – June 26, 2009, 07:22:47 am

as prissi pm'ed me, speed_limit is a hard limit, so this condition select test when its nearer to feasibility.

Re: wegbauer_t::weg_search

Reply #3 – June 26, 2009, 07:33:36 am

But what is the sense of this hard limit??

Re: wegbauer_t::weg_search

Reply #4 – June 26, 2009, 12:13:41 pm

No cityroads with more than 50km/h which are then downgraded by city AI to 50kmh.

Re: wegbauer_t::weg_search

Reply #5 – June 26, 2009, 12:31:44 pm

Which AI downgrade the streets?

Re: wegbauer_t::weg_search

Reply #6 – June 26, 2009, 01:27:33 pm

The citybuilder uses this routine to find a cityroad, when non is defined or the current one is not yet available.

Re: wegbauer_t::weg_search

Reply #7 – June 27, 2009, 12:25:34 pm

And what is the problem, if a city builds a road with more than 50 km/h?

Re: wegbauer_t::weg_search

Reply #8 – June 27, 2009, 07:39:55 pm

It has the speed of 50kmh nevertheless, just confusing players.

Re: wegbauer_t::weg_search

Reply #9 – June 28, 2009, 01:32:01 pm

But then it would be better, if weg_search retruns a way with a speed of at least the speedlimit, if one exists. If an AI wants to build a way for a vehicle, it should be faster than the vehicle, shouldn't it?

Re: wegbauer_t::weg_search

Reply #10 – June 28, 2009, 08:20:26 pm

Maybe just a routine for a way less or same and one for the cehapest. This would keep the logic much cleaner?

Re: wegbauer_t::weg_search

Reply #11 – June 29, 2009, 06:35:49 am

Quote from: prissi – on June 28, 2009, 08:20:26 pm

Maybe just a routine for a way less or same and one for the cehapest. This would keep the logic much cleaner?

This sounds reasonable. Maybe, we could just add a parameter?