Fix for cmdMove in Commands.pm

[Alexander]

sli: the current code uses regex so no type checking required

Why not have the best of both worlds?

Someone should write a better error message though

Code: Select all

sub cmdMove {
   my (undef, $args) = @_;
   my ($arg1, $arg2, $arg3, $arg4) = $args =~ /^(\D+?)?\s*(\d+)?\s*(\d+)?\s*(\S+)?$/;
   my $map = $arg1 ? $arg1 : $arg4;

   if ( ($arg1 && $arg4) || ($arg2 eq "" && !$map) || ($map && $arg2 ne "" && $arg3 eq "") ) {
      error T("Syntax Error in function 'move' (Move Player)\n" .
         "Usage: move <x> <y> &| <map> or <map> &| <x> <y>\n");
   } elsif (!$map && $arg3 eq "") {
      if ($portalsID[$arg2]) {
         message TF("Move into portal number %s (%s,%s)\n", 
            $arg2, $portals{$portalsID[$arg2]}{'pos'}{'x'}, $portals{$portalsID[$arg2]}{'pos'}{'y'});
         main::ai_route($field{name}, $portals{$portalsID[$arg2]}{'pos'}{'x'}, $portals{$portalsID[$arg2]}{'pos'}{'y'}, attackOnRoute => 1, noSitAuto => 1);
      } else {
         error T("No portals exist.\n");
      }
   } elsif ($map eq "stop") {
      AI::clear(qw/move route mapRoute/);
      message T("Stopped all movement\n"), "success";
   } else {
      AI::clear(qw/move route mapRoute/);
      $map = $field{name} if ($map eq "");
      if ($maps_lut{"${map}.rsw"}) {
         my ($x, $y);
         if ($arg2 ne "" && $arg3 ne "") {
            message TF("Calculating route to: %s(%s): %s, %s\n", 
               $maps_lut{$map.'.rsw'}, $map, $arg2, $arg3), "route";
            $x = $arg2;
            $y = $arg3;
         } else {
            message TF("Calculating route to: %s(%s)\n", 
               $maps_lut{$map.'.rsw'}, $map), "route";
         }
         main::ai_route($map, $x, $y,
            attackOnRoute => 1,
            noSitAuto => 1,
            notifyUponArrival => 1);
      } else {
         error TF("Map %s does not exist\n", $map);
      }
   }
}

[hakore]

Its true that some people do mistake using <map> <x> <y> for the move command since most other config options require such sequence. A little tolerance for this won't hurt. Afterall, the move command itself needs some cleaning up.

I don't prefer using too many "optional" entities in regexes though. They may not always work the way you intend them to.

Modified move command and updated SVN. Now supports standard "map x y" parameter sequence.

Thanks Alexander.

[Alexander]

The only difference is the logic is played outside of the regex

[hakore]

Just fixed the regex and some stricter parameter error handling. Updated SVN again.

Alexander, next time, I suggest you post a diff file instead of just code, like what the others are asking from you in the first place. Thanks.

Also, the code you posted cannot properly see the error when the user typed in "move payon prontera". It will simply ignore the second word "prontera" which is not right. Just some minor things though.

[sli]

sli: the current code uses regex so no type checking required

Type checking is still required because you have to match different things for strings and numbers ([a-zA-Z] vs [0-9]{,3}), you'd have to see which of those matches a given argument for the command.

[Chontrad]

Alexander: Sry, but your code is wrong.

The first (\D+?) will only match for non-digit isn't it? But u know, the map contains digits and characters. (ex: pay_fild04)

Why don't any of u ever care of my first code? It's also written in RegEx and I don't play another logic outside that.
I don't know if it uses more PC usage (I don't understand about O + Log(O) thing) But i think the RegEx should be:

/^(\D\S+?)?\s*(\d+)?\s*(\d+)?\s*(\S+)?$/
or
/^(\D\w+?)?\s*(\d+)?\s*(\d+)?\s*(\S+)?$/


Why? coz It will match only if the first arguments is started with non-digit, the later words can be anything except spaces. (The logic is, MAP NAME always starts with non-digit)

Okay, I'm still a n00b in programming, but please respect my work. (sry, a little pissed off)

[Motivus]

Alexander: Sry, but your code is wrong.

The first (\D+?) will only match for non-digit isn't it? But u know, the map contains digits and characters. (ex: pay_fild04)

Why don't any of u ever care of my first code? It's also written in RegEx and I don't play another logic outside that.
I don't know if it uses more PC usage (I don't understand about O + Log(O) thing) But i think the RegEx should be:

/^(\D\S+?)?\s*(\d+)?\s*(\d+)?\s*(\S+)?$/
or
/^(\D\w+?)?\s*(\d+)?\s*(\d+)?\s*(\S+)?$/


Why? coz It will match only if the first arguments is started with non-digit, the later words can be anything except spaces. (The logic is, MAP NAME always starts with non-digit)

Okay, I'm still a n00b in programming, but please respect my work. (sry, a little pissed off)

Map names can start with digits. From future updates:

Code: Select all

1@cata.rsw#īŸÄÞ#
2@cata.rsw#ºÀÀÎµÈ ½ÅÀü#
1@tower.rsw#¿£µé·¹½º Ÿ¿ö#
2@tower.rsw#¿£µé·¹½º Ÿ¿ö#
3@tower.rsw#¿£µé·¹½º Ÿ¿ö#
4@tower.rsw#¿£µé·¹½º Ÿ¿ö#
5@tower.rsw#¿£µé·¹½º Ÿ¿ö#
6@tower.rsw#¿£µé·¹½º Ÿ¿ö(¾Ë ¼ö ¾ø´Â °÷)#

[Alexander]

hakore already put this into svn...

Didn't know maps started with digits though

[hakore]

@Chontrad

Sorry, but your code dismisses the "move <x> <y> [<map>]" command syntax which is not what we want.

And don't feel bad. When collaborating with other developers, expect that your original code will not always be picked unaltered. Aim therefore to improve, and let your coding skills evolve with every input from others.

The recently modified move command in SVN does not check for the map name's format or type since we are aware such names can be anything under the sun. The code therefore looks only at parameters with lesser type variation or no type variation at all, which are the coordinates (always positive integers), to determine how to interpret the move command.