Fix for cmdMove in Commands.pm

Wrote new code? Fixed a bug? Want to discuss technical stuff? Feel free to post it here.

Moderator: Moderators

Message
Author
Chontrad
Human
Human
Posts: 30
Joined: 23 Apr 2008, 10:11
Noob?: No
Location: Indonesia TANAH AIRKU
Contact:

Fix for cmdMove in Commands.pm

#1 Post by Chontrad »

I used format: move <map> <x> <y>
coz i think its easier for me to use this format. [edit: Please note, use this when u like the format only]
The fix is more compact and efficient also, as the duplicating code for entering a portal is removed.
[edit: for the duplicating code, a dev should make a change of it in SVN]

Code: Select all

sub cmdMove {
	my (undef, $args) = @_;
	my ($map, $x, $y) = ($args =~ /^(\D\w+)? ?(\d*) ?(\d*)$/);

	if ($args =~ /^(\d+)$/) {
		if ($portalsID[$1]) {
			message TF("Move into portal number %d (%s,%s)\n", 
				$1, $portals{$portalsID[$1]}{'pos'}{'x'}, $portals{$portalsID[$1]}{'pos'}{'y'});
			main::ai_route($field{name}, $portals{$portalsID[$1]}{'pos'}{'x'}, $portals{$portalsID[$1]}{'pos'}{'y'}, attackOnRoute => 1, noSitAuto => 1);
		} else {
			error "Portal $1 not exist.\n";
		}

	} elsif ($args eq "stop") {
		AI::clear(qw/move route mapRoute/);
		message T("Stopped all movement\n"), "success";

	} elsif ((!$x || !$y) && !$map) {
		error T("Syntax Error in function 'move' (Move Player)\n" .
			"Usage: move <map> &| <x> <y>\n");

	} else {
		AI::clear(qw/move route mapRoute/);
		$map = $field{name} if ($map eq "");
		if ($maps_lut{$map.'.rsw'}) {
			if ($x && $y) {
				message TF("Calculating route to: %s(%s): %s, %s\n", 
					$maps_lut{$map.'.rsw'}, $map, $x, $y), "route";
			} 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);
		}
	}
}
Last edited by Chontrad on 14 Jun 2008, 05:26, edited 1 time in total.

h4rry84
Moderators
Moderators
Posts: 234
Joined: 04 Apr 2008, 09:30
Noob?: Yes
Location: My House
Contact:

Re: Fix for cmdMove in Commands.pm

#2 Post by h4rry84 »

better post it as DIFF file

sli
Perl Monk
Perl Monk
Posts: 810
Joined: 04 Apr 2008, 17:26
Noob?: No

Re: Fix for cmdMove in Commands.pm

#3 Post by sli »

Again, diff file.
cs : ee : realist

Chontrad
Human
Human
Posts: 30
Joined: 23 Apr 2008, 10:11
Noob?: No
Location: Indonesia TANAH AIRKU
Contact:

Re: Fix for cmdMove in Commands.pm

#4 Post by Chontrad »

I don't know what is DIFF file. :roll:
Can u explain to me? Sry, n00bs :lol:


Bibian
Perl Monk
Perl Monk
Posts: 416
Joined: 04 Apr 2008, 03:08

Re: Fix for cmdMove in Commands.pm

#6 Post by Bibian »

There is a reason the move command is like: move <x> <y> &| <map>

if you do <map> first and then <x> <y> you'll have to make extra code to determine wether or not the first argument is a coord or a map...
the way it is works fine now, it has for 5 years and there's no need to change it for everyone. If you want to change it for yourself, go ahead

But im voting against this going to SVN

sli
Perl Monk
Perl Monk
Posts: 810
Joined: 04 Apr 2008, 17:26
Noob?: No

Re: Fix for cmdMove in Commands.pm

#7 Post by sli »

Same. (Had no idea that's what this code did until now. :lol: )
cs : ee : realist

Motivus
Developers
Developers
Posts: 157
Joined: 04 Apr 2008, 13:33
Noob?: Yes

Re: Fix for cmdMove in Commands.pm

#8 Post by Motivus »

Bibian wrote:There is a reason the move command is like: move <x> <y> &| <map>

if you do <map> first and then <x> <y> you'll have to make extra code to determine wether or not the first argument is a coord or a map...
the way it is works fine now, it has for 5 years and there's no need to change it for everyone. If you want to change it for yourself, go ahead

But im voting against this going to SVN
Everyone messes it up, though. Even I mess it up from time to time. It was done that way because it's easier and in a way more logical to have an optional parameter at the end. If you think about what's going on the "optional" parameter is a parameter that is always used just not always typed by the user.

It should be laid out like this to preserve current functionality and support a more user friendly format:

User command: move [map]
User command: move <x> <y> passes to Kore: move <currentMap> <x> <y>
User command: move [map] <x> <y> passes to Kore: move <map> <x> <y>
[legacy] User command: move <x> <y> [map] passes to Kore: move <map> <x> <y>

User input is one of the few places where additional code is not going to hurt performance or the user experience. The only thing that adds additional code is supporting the old format, which we don't even have to do.
Oh no.

Chontrad
Human
Human
Posts: 30
Joined: 23 Apr 2008, 10:11
Noob?: No
Location: Indonesia TANAH AIRKU
Contact:

Re: Fix for cmdMove in Commands.pm

#9 Post by Chontrad »

About the optional parameter, both <x> <y> or <map> can be an optional parameter coz only one of them is required to be written.

Okay, if u think that last code works fine for everyone, u'r wrong. Many of my friends complained why the MOVE command don't have same format as storageAuto_npc, and they messed up at least 1 time everytime they wanted to type coords and map.

Also, for the extra code... I don't think i need one. Better look at my code first before commenting. I don't think this code consumes more PC usage than the old ineffective code. This code has been tested on my machine and its working fine.

sli
Perl Monk
Perl Monk
Posts: 810
Joined: 04 Apr 2008, 17:26
Noob?: No

Re: Fix for cmdMove in Commands.pm

#10 Post by sli »

Explain how it doesn't consume more cycles. If you're moving to different coordinates within the same map, Kore will HAVE to make sure that first argument is numeric. With the current system, all it needs to do is split() and check the number of arguments as opposed to having to do that AND check the type as well.
cs : ee : realist

Post Reply