Explicitly set default facing direction of other players

[tsdko]

Both server and client code assume the default is 0 (f is sent only if the client's facing value is other than 0), but the liblcf default is actually 2 (down).

Fixes an issue where a player facing north (0) would show up as facing south (2) after logging out or switching to private mode.

[Desdaemon]

Would changing server and client to use south as the default direction make more sense in this case?

[tsdko]

I can change it. This was one of my first ideas but I figured going with the principle of least surprise was safer, especially considering how 0 was already assumed to be the default on both sides. 2 is a pretty unexpected value to see as the default if you don't already have all the context. One small benefit of using south instead of north is not having to send that one extra packet for every player facing south, which for maps populated with frequent idlers could save a tiny (probably not very significant) bit of bandwidth per join. (there was a sentence about keeping the explicit assignment for stability against upstream changes here but it feels like overkill considering these defaults appear to be derived from how RPG_RT interprets the save data with these fields missing (example 1 (different field)))

[tsdko]

I've thought about this some more and I feel my earlier thought about not including explicit default assigment was shortsighted. A recent liblcf pull request is an example of a change it wouldn't have covered (making a certain field's default value engine-version-dependent).

I can still go ahead with it, the code below works; I am still unsure which one is better.

--- a/src/game_playerother.h
+++ b/src/game_playerother.h
@@ -12,8 +12,11 @@ using Game_PlayerBase = Game_CharacterDataStorage<lcf::rpg::SavePartyLocation>;
  */
 class Game_PlayerOther : public Game_PlayerBase {
 	public:
+		static constexpr int DEFAULT_FACING = Down;
+
 		Game_PlayerOther(int id) : Game_CharacterDataStorage(PlayerOther), id(id)
 		{
+			SetFacing(DEFAULT_FACING);
 			SetDirection(lcf::rpg::EventPage::Direction_down);
 			SetMoveSpeed(4);
 			SetAnimationType(lcf::rpg::EventPage::AnimType_non_continuous);
--- a/src/multiplayer/game_multiplayer.cpp
+++ b/src/multiplayer/game_multiplayer.cpp
@@ -558,7 +558,7 @@ void Game_Multiplayer::SendBasicData() {
 	connection.SendPacketAsync<C::SpeedPacket>(player->GetMoveSpeed());
 	connection.SendPacketAsync<C::SpritePacket>(player->GetSpriteName(),
 				player->GetSpriteIndex());
-	if (player->GetFacing() > 0) {
+	if (player->GetFacing() != Game_PlayerOther::DEFAULT_FACING) {
 		connection.SendPacketAsync<C::FacingPacket>(player->GetFacing());
 	}
 	connection.SendPacketAsync<C::HiddenPacket>(player->IsSpriteHidden());

(all current instances of SetFacing with a direct value use the enum defined in Game_Character, not EventPage; values for the basic 4 directions are the same in both)

--- a/server/client.go
+++ b/server/client.go
@@ -33,6 +33,8 @@ const (
 	maxMessageSize = 4096
 
 	maxPictures = 1000
+
+	defaultFacing = 2 // down
 )
 
 type Picture struct {
@@ -303,7 +305,7 @@ func (c *RoomClient) disconnect() {
 func (c *RoomClient) reset() {
 	c.x = -1
 	c.y = -1
-	c.facing = 0
+	c.facing = defaultFacing
 	c.speed = 0
 
 	c.flash = [5]int{}
--- a/server/room.go
+++ b/server/room.go
@@ -341,7 +341,7 @@ func (c *RoomClient) getPlayerData(client *RoomClient) {
 	if client.x != -1 {
 		c.outbox <- buildMsg("m", client.session.id, client.x, client.y)
 	}
-	if client.facing != 0 {
+	if client.facing != defaultFacing {
 		c.outbox <- buildMsg("f", client.session.id, client.facing)
 	}
 	if client.speed != 0 {

During testing I've noticed there's another facing-related server bug that resulted in (kind of) accidentally correct behavior prior to ynoproject/ynoserver@651597c getting merged, will try to send a patch for it tomorrow but the south-default version of this PR will start triggering it for south-facing players instead of north-facing players.

[tsdko]

Updated to use south as the default.