A possible fix for the position bug when attacking (Netcode)

[Tero]

This is such a minor tweak that I don't even really think it's worth creating a topic for, but my testers reported that this makes the game feel significantly smoother.

 

This change attempts to address a desync issue when attacking.  When clicking on a monster that is hostile and is also moving towards you, sometimes your character will fail to begin attacking because the client thinks it is within range to attack, but the character is actually too far away to attack on the server side.  It's especially common with monsters that move fast.  With this tweak, this situation should no longer happen.

 

The change is in unit.cpp:

 

Find this comment:

// Player tries to attack but target is too far, notify client

 

Add this line immediately afterwards:

clif_fixpos(src);  // synchronize the player's position with the client

 

Don't change anything else.  The finished code block should look like this.

if(sd && !check_distance_client_bl(src,target,range)) {
		// Player tries to attack but target is too far, notify client
		clif_fixpos(src);  // synchronize the player's position with the client
		clif_movetoattack(sd,target);		
		return 1;	

 

Then just recompile the code and test it out.  Everyone I had test it said it felt instantly noticeable.  This does increase the number of packets being sent so this may cause a very minor increase to network traffic, but based on the number of packets that are typically sent back and forth I don't think the effect is very significant.

[Tero]

So it turns out there's actually another version of this glitch where sometimes your character will fail to attack because they're at an extremely precise range where they are out of range to attack, but too close to move closer and then attack.  This seemingly requires some degree of client desync and is rare, but it annoyed me enough that I spent many hours tracking down the exact problem.

 

Find this in unit.cpp: 

if(src->type == BL_PC && ud->walktimer != INVALID_TIMER && (!battle_check_range(src, target, range-1) || ignore_range)) {

(it occurs twice in the file)

In both instances, replace it with this:

if(src->type == BL_PC && (!battle_check_range(src, target, range) || ignore_range)) {
	if (ud->walktimer == INVALID_TIMER) {
		unit_walktoxy(src, target->x, target->y, 8);
	}

Do not touch anything below this code, leave everything about the stepskills untouched.

 

 

If your rathena is old, the ignore range part is not present so you are instead looking for this:

if(src->type == BL_PC && ud->walktimer != INVALID_TIMER && !battle_check_range(src, target, range)) {

Which you replace with this:

if(src->type == BL_PC && !battle_check_range(src, target, range)) {
	if (ud->walktimer == INVALID_TIMER) {
		unit_walktoxy(src, target->x, target->y, 8);
	}

 

 

The problem is that if the user is too close for the client to execute a move to attack instruction, but not close enough to attack, the walktimer will not be initialized, but the range check will fail.  It should execute a stepskill in this situation, but it does not because it requires a valid walktimer to execute a stepskill.  This handles this boundary case by forcing the user to approach the target, as the client would do automatically under all other circumstances.  This is maybe not the most graceful solution (maybe they should just move one tile closer to the target?) but it works and the problem can no longer occur.

Edited October 24, 2023 by Tero

[eleriaqueen]

I have do admit I was quite a bit skeptical when I saw your fixes consisted of such a small amount of code, but they make a huge difference !  Thank you !

Edited December 22, 2023 by eleriaqueen

[Gidz Cross]

On 10/24/2023 at 2:52 AM, Tero said:

If your rathena is old, the ignore range part is not present so you are instead looking for this:

if(src->type == BL_PC && ud->walktimer != INVALID_TIMER && !battle_check_range(src, target, range)) {

Which you replace with this:

if(src->type == BL_PC && !battle_check_range(src, target, range)) {
	if (ud->walktimer == INVALID_TIMER) {
		unit_walktoxy(src, target->x, target->y, 8);
	}

My rA is much older than this i guess. I dont have this
 

Quote

if(src->type == BL_PC && ud->walktimer != INVALID_TIMER && !battle_check_range(src, target, range)) {

But i got this instead
 

	// Remember the skill request from the client while walking to the next cell
	if(src->type == BL_PC && ud->walktimer != INVALID_TIMER && !battle_check_range(src, target, range-1)) {
		ud->stepaction = true;
		ud->target_to = target_id;
		ud->stepskill_id = skill_id;
		ud->stepskill_lv = skill_lv;
		return 0; // Attacking will be handled by unit_walktoxy_timer in this case
	}

and

	// Remember the skill request from the client while walking to the next cell
	if(src->type == BL_PC && ud->walktimer != INVALID_TIMER && !battle_check_range(src, &bl, range-1)) {
		struct map_data *md = &map[src->m];
		// Convert coordinates to target_to so we can use it as target later
		ud->stepaction = true;
		ud->target_to = (skill_x + skill_y*md->xs);
		ud->stepskill_id = skill_id;
		ud->stepskill_lv = skill_lv;
		return 0; // Attacking will be handled by unit_walktoxy_timer in this case
	}

and finally

	// Remember the attack request from the client while walking to the next cell
	if(src->type == BL_PC && ud->walktimer != INVALID_TIMER && !battle_check_range(src, target, range-1)) {
		ud->stepaction = true;
		ud->target_to = ud->target;
		ud->stepskill_id = 0;
		ud->stepskill_lv = 0;
		return 0; // Attacking will be handled by unit_walktoxy_timer in this case
	}

Thanks!

[Tero]

 

Oh, it's possible the change from range -1 to range might have been something else I tweaked at some point, because it looks like the version you have is the same as mine.  In any case, you'd just have this.

// Remember the skill request from the client while walking to the next cell
if(src->type == BL_PC && !battle_check_range(src, target, range)) {
	if (ud->walktimer == INVALID_TIMER) {
		unit_walktoxy(src, target->x, target->y, 8);
	}
	ud->stepaction = true;
	ud->target_to = target_id;
	ud->stepskill_id = skill_id;
	ud->stepskill_lv = skill_lv;
	return 0; // Attacking will be handled by unit_walktoxy_timer in this case
}

 

For the second block, you can actually leave that one.  That is for ground-targetted skills, which cannot experience this issue.

 

The third block looks like this.

// Remember the attack request from the client while walking to the next cell
if(src->type == BL_PC && !battle_check_range(src, target, range)) {
	if (ud->walktimer == INVALID_TIMER) {
		unit_walktoxy(src, target->x, target->y, 8);
	}
	ud->stepaction = true;
	ud->target_to = ud->target;
	ud->stepskill_id = 0;
	ud->stepskill_lv = 0;
	return 0; // Attacking will be handled by unit_walktoxy_timer in this case
}

Edited January 8, 2024 by Tero