Implement buddy warping by lwcasgc · Pull Request #12 · yungcomputerchair/RustyFusion

lwcasgc · 2025-11-08T06:36:06Z

when teleporting across shards, it obtains the position of the target buddy, sets the player position to the buddy position and then lastly sends the P_FE2CL_REP_PC_BUDDY_WARP_OTHER_SHARD_SUCC packet for the client to swap shards

yungcomputerchair

Solid work; I'm pretty sure this won't work if they're in different channels right now but that should be an easy fix. I'm also not sure if P_FE2CL_REP_PC_BUDDY_WARP_SAME_SHARD_SUCC needs to be in the mix, I'll get back to you on that

yungcomputerchair · 2025-11-08T06:44:36Z

+    }
+
+    let buddy_id = res.unwrap();
+    let buddy: &rusty_fusion::entity::Player = state.get_player(buddy_id)?;


Can omit the type

Suggested change

let buddy: &rusty_fusion::entity::Player = state.get_player(buddy_id)?;

let buddy = state.get_player(buddy_id)?;

yungcomputerchair · 2025-11-08T06:45:31Z

+    if buddy.instance_id.map_num != 0 {
+        let response = sP_FE2CL_REP_PC_BUDDY_WARP_FAIL {
+            iBuddyPCUID: buddy_uid,
+            iErrorCode: 1, // TODO: define error codes


I'll reach out to you offline about how to define error codes based on the client code

yungcomputerchair · 2025-11-08T06:49:08Z

+    log_if_failed(
+        clients
+            .get_self()
+            .send_packet(P_FE2CL_REP_PC_GOTO_SUCC, &resp),


this probably works fine but I'm wondering if P_FE2CL_REP_PC_BUDDY_WARP_SAME_SHARD_SUCC is relevant to us at all

yungcomputerchair · 2025-11-08T06:53:55Z

+        log_if_failed(from_shard.send_packet(P_LS2FE_REP_BUDDY_WARP_FAIL, &fail_resp));
+
+        return Ok(());


Since communicating back to the shard is pretty integral to the handling of this packet, I would treat the failure to send the packet as an error:

Suggested change

log_if_failed(from_shard.send_packet(P_LS2FE_REP_BUDDY_WARP_FAIL, &fail_resp));

return Ok(());

return from_shard.send_packet(P_LS2FE_REP_BUDDY_WARP_FAIL, &fail_resp);

yungcomputerchair · 2025-11-08T06:54:19Z

+    log_if_failed(from_shard.send_packet(P_LS2FE_REP_BUDDY_WARP_SUCC, &resp));
+
+    Ok(())


see above

Suggested change

log_if_failed(from_shard.send_packet(P_LS2FE_REP_BUDDY_WARP_SUCC, &resp));

Ok(())

from_shard.send_packet(P_LS2FE_REP_BUDDY_WARP_SUCC, &resp)

yungcomputerchair · 2025-11-08T06:58:01Z

+        z: buddy_z,
+    });
+
+    player.instance_id = rusty_fusion::chunk::InstanceID::default();


see above, make sure to factor in channel ID here

yungcomputerchair · 2025-11-08T07:02:48Z

+    player.instance_id = rusty_fusion::chunk::InstanceID::default();
+
+    state.entity_map.update(
+        rusty_fusion::entity::EntityID::Player(pc_id),


code style nit: import the struct and then do

Suggested change

rusty_fusion::entity::EntityID::Player(pc_id),

EntityID::Player(pc_id),

yungcomputerchair · 2025-11-08T07:03:16Z

+        Some(clients),
+    );
+
+    let warp_pkt: sP_FE2CL_REP_PC_BUDDY_WARP_OTHER_SHARD_SUCC =


code style nit: omit the type

Suggested change

let warp_pkt: sP_FE2CL_REP_PC_BUDDY_WARP_OTHER_SHARD_SUCC =

let warp_pkt =

yungcomputerchair · 2025-11-08T07:07:44Z

+
+    if buddy_metadata.map_num != 0 {
+        log(
+            Severity::Warning,


this can probably be less important

Suggested change

Severity::Warning,

Severity::Info,

yungcomputerchair · 2025-11-08T07:10:55Z

+            iChannelNum: if buddy_channel > 1 {
+                buddy_channel as i32
+            } else {
+                0
+            },


am I missing something or can you just do:

Suggested change

iChannelNum: if buddy_channel > 1 {

buddy_channel as i32

} else {

0

},

iChannelNum: buddy_channel as i32,

yungcomputerchair

On top of the comments I left, I'm pretty confused on the packet flow here.
What's the purpose of the P_FE2LS_REQ_BUDDY_WARP packet? I don't see it being sent anywhere. I see the shard sending P_FE2LS_REQ_BUDDY_WARP_VALIDATION when the buddy is not in the same shard, but that's it.

I think the intuitive flow & naming here here looks more like:

client sends CL2FE_REQ_BUDDY_WARP to shard 1
shard 1 sends FE2LS_REQ_BUDDY_WARP to login
login sends LS2FE_REQ_BUDDY_WARP to shard 2
shard 2 sends FE2LS_REP_BUDDY_WARP_SUCC/FAIL to login
login sends LS2FE_REP_BUDDY_WARP_SUCC/FAIL to shard 1
shard 1 sends FE2CL_REP_BUDDY_WARP_OTHER_SHARD_SUCC/FAIL to client

yungcomputerchair · 2025-11-09T20:16:44Z

+    let mut invalid_warp = |error_code: i32, msg: String| -> FFResult<()> {
+        let response = sP_FE2CL_REP_PC_BUDDY_WARP_FAIL {
+            iBuddyPCUID: buddy_uid,
+            iErrorCode: error_code, // "Warp failure!\nUh oh! %s is in a location that you can't warp to! Try again later."
+        };
+        log_if_failed(
+            clients
+                .get_self()
+                .send_packet(P_FE2CL_REP_PC_BUDDY_WARP_FAIL, &response),
+        );
+        Err(FFError::build(Severity::Info, msg))
+    };


Love this. Need more usage of closures like this in the codebase. Awesome

yungcomputerchair · 2025-11-09T20:18:31Z

+    let pc_id = client.get_player_id()?;
+    let player = state.get_player(pc_id)?;
+    let player_payzone_flag = player.flags.payzone_flag;
+    let player_skyway_state = player.skyway_ride.clone();


I would strongly prefer if we didn't clone the skyway ride state here; I don't think it's necessary. You can probably do something like this instead:

Suggested change

let player_skyway_state = player.skyway_ride.clone();

let is_player_on_skyway = player.skyway_ride.is_some();

yungcomputerchair · 2025-11-09T20:24:24Z

+    let buddy_id = res.unwrap();
+    let buddy = state.get_player(buddy_id)?;
+    let buddy_payzone_flag = buddy.flags.payzone_flag;
+    let buddy_skyway_state = buddy.skyway_ride.clone();


see above comment on cloning

yungcomputerchair · 2025-11-09T20:25:33Z

+
+    if player_skyway_state.is_some() {
+        return invalid_warp(
+            3,


it would be great if you defined these error codes in error.rs

yungcomputerchair · 2025-11-09T20:29:43Z

+    player.instance_id = InstanceID {
+        map_num: ID_OVERWORLD,
+        channel_num: if player.instance_id.channel_num != buddy_instance.channel_num {
+            buddy_instance.channel_num
+        } else {
+            0
+        },
+        instance_num: buddy_instance.instance_num,
+    };


I don't think we want to set the channel number to 0. Should just copy the instance ID directly from the buddy.
We only want this to be 0 in the response packet, so the client interprets it as "same channel". But as for the actual instance ID, channel 0 is invalid.

Suggested change

player.instance_id = InstanceID {

map_num: ID_OVERWORLD,

channel_num: if player.instance_id.channel_num != buddy_instance.channel_num {

buddy_instance.channel_num

} else {

0

},

instance_num: buddy_instance.instance_num,

};

player.instance_id = buddy_instance;

yungcomputerchair · 2025-11-09T20:35:00Z

+        iWarpReceiverX: receiver_pos.x,
+        iWarpReceiverY: receiver_pos.y,
+        iWarpReceiverZ: receiver_pos.z,
+        iWarpReceiverShardID: config_get().shard.shard_id.get(),


redundant field

yungcomputerchair · 2025-11-09T20:35:58Z

+    log_if_failed(
+        clients
+            .get_login_server()
+            .unwrap()
+            .send_packet(P_FE2LS_REP_BUDDY_WARP_VALIDATION_SUCC, &response),
+    );
+
+    Ok(())


I would treat the failure to send this packet back as an error:

Suggested change

log_if_failed(

clients

.get_login_server()

.unwrap()

.send_packet(P_FE2LS_REP_BUDDY_WARP_VALIDATION_SUCC, &response),

);

Ok(())

clients

.get_login_server()

.unwrap()

.send_packet(P_FE2LS_REP_BUDDY_WARP_VALIDATION_SUCC, &response)

yungcomputerchair · 2025-11-09T20:39:07Z

+    player.instance_id = InstanceID {
+        map_num: ID_OVERWORLD,
+        channel_num: if receiver_channel_num as u8 != player_channel_num {
+            receiver_channel_num as u8
+        } else {
+            0
+        },
+        instance_num: if receiver_instance_num > 0 {
+            Some(receiver_instance_num as u32)
+        } else {
+            None
+        },
+    };


No need to set the instance ID since it's not preserved in the database anyway

yungcomputerchair · 2025-11-09T20:39:31Z

+    let receiver_z = pkt.iWarpReceiverZ;
+    let receiver_shard_id = pkt.iWarpReceiverShardID;
+    let receiver_channel_num = pkt.iWarpReceiverChannelNum;
+    let _receiver_map_num = pkt.iWarpReceiverMapNum;


Is this field unused? If so should remove it from the packet

yungcomputerchair · 2025-11-09T20:43:18Z

+    // has to be sent to prevent teleporting to "unknown" map
+    let resp = sP_FE2CL_REP_PC_GOTO_SUCC {
+        iX: receiver_x,
+        iY: receiver_y,
+        iZ: receiver_z,
+    };


I really don't understand why we need to do this...
I would think that since the client is connecting to the other shard through the normal login flow, it should pick up the position from the DB. Maybe flush the player to the DB with db_run_sync() and see if this is still needed?

… saving)

yungcomputerchair

This code is extremely clean and most of my (very few) comments are nits.
The only reason I'm flagging this for changes is that the buddy warp timer does not seem to be set serverside upon a successful warp, and the check for it is implemented incorrectly.
If you could do that (bonus points if you address the rest of the comment) then I will approve :)

yungcomputerchair · 2025-11-12T06:07:14Z

+
+    if buddy_is_on_skyway {
+        return invalid_warp(format!(
+            "Player {} is currently on a skyway ride",


Suggested change

"Player {} is currently on a skyway ride",

"Buddy {} is currently on a skyway ride",

yungcomputerchair · 2025-11-12T06:09:30Z

+
+    if buddy_is_on_skyway {
+        return invalid_warp(format!(
+            "Player {} is currently on a skyway ride",


Suggested change

"Player {} is currently on a skyway ride",

"Buddy {} is currently on a skyway ride",

yungcomputerchair · 2025-11-12T06:12:31Z

+
+            let other_shard_succ_pkt = sP_FE2CL_REP_PC_BUDDY_WARP_OTHER_SHARD_SUCC {
+                iBuddyPCUID: pkt.iBuddyPCUID,
+                iChannelNum: 0,


Should leave a comment here explaining why we always send back 0 in this case

yungcomputerchair · 2025-11-12T06:19:57Z

+    }
+
+    pub fn is_warp_on_cooldown(&self) -> bool {
+        self.buddy_warp_time > 0


This is a timestamp of the last buddy warp, so really this will be something like self.buddy_warp_time > 0 && self.buddy_warp_time - util::get_timestamp_sec() < BUDDYWARP_INTERVAL.
Or even better, if you could make self.buddy_warp_time an Option<usize> which is initially None but then gets set to Some(timestamp) on warp.
Then this function can look like:

Suggested change

self.buddy_warp_time > 0

self.buddy_warp_time.is_some_and(|ts| util::get_timestamp() - ts < BUDDYWARP_INTERVAL)

(BUDDYWARP_INTERVAL can be imported from defines.rs)

Even better yet if you could rename the field to something less confusing like last_buddy_warp_timestamp

lwcasgc · 2025-11-12T13:04:37Z

            });
-            let player_saved = player.clone();
-
-            log_if_failed(db_run_sync(move |db| db.save_player(&player_saved)));


removed db save, the position bug wasn’t caused by not saving to the database. the real issue was that instancenum was being copied from the buddy making it fall into the condition of some, causing the save_player_internal condition to read the prewarp position upon saving instead of the actual player position.

It's fine to remove it from the same-shard warp case, but we still need it for the cross-shard.
The reason is because there could potentially be a race condition: the processing of the client's disconnect might not finish before the client connects to the other shard.
Flushing the player to DB before we send the success packet guarantees that the position will be correct before the client even thinks about connecting to the other shard.

yungcomputerchair

Basically there but I'm too worried about the cross-shard race condition. Bring back the DB flush for the cross-shard case and we're good

yungcomputerchair · 2025-11-12T17:07:41Z

+            iBuddyWarpTime: self.last_buddy_warp_timestamp.map_or(0, |ts| {
+                let elapsed = util::get_timestamp_sec(SystemTime::now()).saturating_sub(ts);
+                if elapsed < BUDDYWARP_INTERVAL {
+                    // client expects the absolute timestamp when the cooldown ends
+                    (ts + BUDDYWARP_INTERVAL) as i32
+                } else {
+                    0
+                }
+            }),


no need to do this calculation since the client does its own math

Suggested change

iBuddyWarpTime: self.last_buddy_warp_timestamp.map_or(0, |ts| {

let elapsed = util::get_timestamp_sec(SystemTime::now()).saturating_sub(ts);

if elapsed < BUDDYWARP_INTERVAL {

// client expects the absolute timestamp when the cooldown ends

(ts + BUDDYWARP_INTERVAL) as i32

} else {

0

}

}),

iBuddyWarpTime: self.last_buddy_warp_timestamp.unwrap_or(0),

yungcomputerchair · 2025-11-12T17:11:00Z

            });
-            let player_saved = player.clone();
-
-            log_if_failed(db_run_sync(move |db| db.save_player(&player_saved)));


It's fine to remove it from the same-shard warp case, but we still need it for the cross-shard.
The reason is because there could potentially be a race condition: the processing of the client's disconnect might not finish before the client connects to the other shard.
Flushing the player to DB before we send the success packet guarantees that the position will be correct before the client even thinks about connecting to the other shard.

yungcomputerchair

Great work 👍

yungcomputerchair requested changes Nov 8, 2025

View reviewed changes

lwcasgc force-pushed the buddies branch 2 times, most recently from 8eb76c9 to 0c4caa7 Compare November 9, 2025 15:46

lwcasgc marked this pull request as draft November 9, 2025 15:48

lwcasgc marked this pull request as ready for review November 9, 2025 18:27

lwcasgc requested a review from yungcomputerchair November 9, 2025 18:28

lwcasgc marked this pull request as draft November 9, 2025 18:29

lwcasgc marked this pull request as ready for review November 9, 2025 18:45

yungcomputerchair requested changes Nov 9, 2025

View reviewed changes

lwcasgc added 3 commits November 9, 2025 22:06

Refactor same-shard buddy warp

f93e017

Correct order to validate player first

749cd92

Buddy warp code refactor (missing load balance avoidance and position…

636d540

… saving)

lwcasgc force-pushed the buddies branch from 173b0c4 to 636d540 Compare November 10, 2025 12:09

lwcasgc added 2 commits November 10, 2025 12:48

Logging process of channel requests

feead17

Fix the pending channel obtainment, fix the saving of player position

655fd5f

lwcasgc requested a review from yungcomputerchair November 12, 2025 01:14

yungcomputerchair requested changes Nov 12, 2025

View reviewed changes

Improving warp cooldown check

d36b241

lwcasgc commented Nov 12, 2025

View reviewed changes

lwcasgc requested a review from yungcomputerchair November 12, 2025 13:07

yungcomputerchair requested changes Nov 12, 2025

View reviewed changes

lwcasgc added 2 commits November 12, 2025 19:10

Bringing back db save for cross-shard warps

973b6da

Simplifying warp cooldown checking

3f14059

lwcasgc requested a review from yungcomputerchair November 12, 2025 23:19

yungcomputerchair approved these changes Nov 12, 2025

View reviewed changes

yungcomputerchair merged commit 65b5ee5 into yungcomputerchair:main Nov 12, 2025
3 checks passed

	let buddy: &rusty_fusion::entity::Player = state.get_player(buddy_id)?;
	let buddy = state.get_player(buddy_id)?;

		log_if_failed(from_shard.send_packet(P_LS2FE_REP_BUDDY_WARP_FAIL, &fail_resp));

		return Ok(());

		log_if_failed(from_shard.send_packet(P_LS2FE_REP_BUDDY_WARP_SUCC, &resp));

		Ok(())

	rusty_fusion::entity::EntityID::Player(pc_id),
	EntityID::Player(pc_id),

	let warp_pkt: sP_FE2CL_REP_PC_BUDDY_WARP_OTHER_SHARD_SUCC =
	let warp_pkt =

	let player_skyway_state = player.skyway_ride.clone();
	let is_player_on_skyway = player.skyway_ride.is_some();

	"Player {} is currently on a skyway ride",
	"Buddy {} is currently on a skyway ride",

	self.buddy_warp_time > 0
	self.buddy_warp_time.is_some_and(\|ts\| util::get_timestamp() - ts < BUDDYWARP_INTERVAL)

Conversation

lwcasgc commented Nov 8, 2025

Uh oh!

yungcomputerchair left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yungcomputerchair left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yungcomputerchair Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yungcomputerchair left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yungcomputerchair Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lwcasgc Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yungcomputerchair left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yungcomputerchair left a comment

yungcomputerchair Nov 9, 2025 •

edited

Loading

yungcomputerchair Nov 12, 2025 •

edited

Loading

lwcasgc Nov 12, 2025 •

edited

Loading