Adds support for <account id> param for getcharid(). Deprecates charid2rid.

[bWolfie]

Pull Request Prelude

• I have followed proper Hercules code styling.
• I have read and understood the contribution guidelines before making this PR.
• I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.

Changes Proposed

• Adds support for account id to getcharid().
  I.e.
  getcharid(<type>{, "<character name>"})
getcharid(<type>{, <account id>})

• Deprecates charid2rid() in favour of getcharid(CHAR_ID_ACCOUNT, <char id>).

Issues addressed:
N/A

[HerculesWSAPI]

This change is 

[AnnieRuru]

*charid2rid is still useful to retrieve the value from SQL table or item's char id field
^--- EDITED ---

1. from SQL
   select char_id from char where online = 1;
   well yes can use select account_id ...
   but charid2rid return 0 when player is offline, similar to isloggedin return 0 when player is offline

2. signed item
   

   Hercules/doc/script_commands.txt

   Lines 5006 to 5017 in 52360f0

   	// First, let's get an ID of a character who's name will be on the
   	// item. Only an existing character's name may be there.
   	// Let's assume our character is 'Adam' and find his ID.
   	.@charid = getcharid(CHAR_ID_CHAR, "Adam");
   	// Now we split the character ID number into two portions with a
   	// binary shift operation. If you don't understand what this does,
   	// just copy it.
   	.@card3 = .@charid & 65535;
   	.@card4 = .@charid >> 16;

   
   this only return char id

[bWolfie]

@AnnieRuru I don't understand what you're saying. This PR does not reduce functionality of getcharid(), only improve it.
.@charid = getcharid(CHAR_ID_CHAR, "Adam"); still returns the same value.

getcharid() already returns 0 if player is offline.
charid2rid() returns account id anyway, so may as well merge into the superior command.

getcharid

	if (sd == NULL) {
		script_pushint(st, 0); //return 0, according docs
		return true;
	}

charid2rid

	if (sd == NULL) {
		script_pushint(st, 0);
		return true;
	}

[AnnieRuru]

what I am saying is just don't deprecate the *charid2rid, because I have use them personally
nothing wrong with getcharid implementation =)

oops, edit that post, I word it wrongly again

getcharid() already returns 0 if player is offline.

yes, getcharid() return 0, but the signed item only return char id WITHOUT account ID
so when you need to check the account ID from char ID, if you deprecate that command, then how are you going to use it ?

getcharid(CHAR_ID_ACCOUNT, <char_id>) ????
of course doesn't work, because that field you have already insert with account ID

EDITING ---- OK I WRITE A SAMPLE SCRIPT -----

[AnnieRuru]

prontera,155,185,5	script	sfshdfk	1_F_MARIA,{
	getnameditem 1201, getcharid(0);
//	Use my getitemname2 function, I only write a small part of it
	getinventorylist;
	for ( .@i = 0; .@i < @inventorylist_count; ++.@i ) {
		if ( @inventorylist_card1[.@i] == 255 || @inventorylist_card1[.@i] == 254 ) {
			.@charid = @inventorylist_card3[.@i];
			// now I have to convert this char ID into name 
			.@account_id = charid2rid(.@charid);
			if ( .@account_id )
				dispbottom rid2name(.@account_id) +"'s "+ getitemname( @inventorylist_id[.@i] ) +"[Online]";
			// now tell me how are you going to convert char ID into name IF charid2rid is deprecated ???
			else {
				query_sql "select name from `char` where char_id = "+ .@charid, .@name$ +"[Offline]";
				dispbottom .@name$ +"'s "+ getitemname( @inventorylist_id[.@i] );
			}
		}
	}
	end;
}

EDIT: --> .@account_id = getcharid(3, .@charid);
OH, you merge them into 1 single command

[dastgirp]

what I am saying is just don't deprecate the *charid2rid, because I have use them personally
nothing wrong with getcharid implementation =)

oops, edit that post, I word it wrongly again

getcharid() already returns 0 if player is offline.

yes, getcharid() return 0, but the signed item only return char id WITHOUT account ID
so when you need to check the account ID from char ID, if you deprecate that command, then how are you going to use it ?

getcharid(CHAR_ID_ACCOUNT, <char_id>) ????
of course doesn't work, because that field you have already insert with account ID

EDITING ---- OK I WRITE A SAMPLE SCRIPT -----

@AnnieRuru, @EyesOfAHawk has modified the getcharid, so that CHAR_ID_ACCOUNT will check for char_id and not for account id as 2nd argument

Although the PR seems fine, but let @Asheraf or @MishimaHaruna review whether it is good idea to merge these 2 functionality

[AnnieRuru]

ok I misread that part, approve it
*charid2rid is very very seldom use,
most of the time I actually want to change from char_id into name
so I have to use *charid2rid + *rid2name together
I actually wish there is a *charid2name script command for it

actually yeah, need discussion, because getcharid(CHAR_ID_NAME, <char_id>) doesn't work
<-- that field already reserve for <account_id>
(that's the reason why I misunderstood this pull request =/ sorry about that)

[MishimaHaruna]

The part about getcharid(CHAR_ID_ACCOUNT, <char id>) seems confusing to me, I feel that having hidden special cases (the same command takes different IDs as argument depending on what you request to return) makes the scripts less readable. What does everyone else think?

[AnnieRuru]

hmm, rathena has this PR rathena/rathena#3924
also looks confusing to me ....

I actually wish there is a *charid2name script command for it

I rather have a script command that is clearly state what it is doing,
just like writing an essay which everyone understand
yeah a script command with hidden parameter make the script less readable ...

EDIT: exactly the point, it confused me/makes me misunderstood in previous posts

[Kenpachi2k13]

@MishimaHaruna

What does everyone else think?

I do agree with you. In my opinion this special case unnecessary. If someone is able to pass a character ID to retrieve the corresponding account ID, he should also be able to pass the character's name instead.