From 4360535b859acb454c1692cf0942818e101df399 Mon Sep 17 00:00:00 2001 From: RedFlames Date: Mon, 31 Oct 2022 03:03:42 +0100 Subject: [PATCH 01/10] Refactor things into ResetState, RemoveAllGhosts, use RemoveGhost more --- .../Components/CelesteNetMainComponent.cs | 93 ++++++++----------- 1 file changed, 41 insertions(+), 52 deletions(-) diff --git a/CelesteNet.Client/Components/CelesteNetMainComponent.cs b/CelesteNet.Client/Components/CelesteNetMainComponent.cs index c6c19a28..9ebeb30b 100644 --- a/CelesteNet.Client/Components/CelesteNetMainComponent.cs +++ b/CelesteNet.Client/Components/CelesteNetMainComponent.cs @@ -10,6 +10,7 @@ using Monocle; using MonoMod.Cil; using MonoMod.RuntimeDetour; +using Steamworks; using System; using System.Collections.Concurrent; using System.Collections.Generic; @@ -129,15 +130,9 @@ protected override void Dispose(bool disposing) { } public void Cleanup() { - Player = null; - PlayerBody = null; - Session = null; - WasIdle = false; - WasInteractive = false; + ResetState(); - foreach (Ghost ghost in Ghosts.Values) - ghost?.RemoveSelf(); - Ghosts.Clear(); + RemoveAllGhosts(); if (IsGrabbed && Player.StateMachine.State == Player.StFrozen) Player.StateMachine.State = Player.StNormal; @@ -169,8 +164,7 @@ public void Handle(CelesteNetConnection con, DataPlayerInfo player) { return; if (string.IsNullOrEmpty(player.DisplayName)) { - ghost.RunOnUpdate(ghost => ghost.NameTag.Name = ""); - Ghosts.TryRemove(player.ID, out _); + RemoveGhost(player); LastFrames.TryRemove(player.ID, out _); Client.Data.FreeOrder(player.ID); return; @@ -179,9 +173,7 @@ public void Handle(CelesteNetConnection con, DataPlayerInfo player) { public void Handle(CelesteNetConnection con, DataChannelMove move) { if (move.Player.ID == Client.PlayerInfo.ID) { - foreach (Ghost ghost in Ghosts.Values) - ghost?.RemoveSelf(); - Ghosts.Clear(); + RemoveAllGhosts(); // The server resends all bound data anyway. foreach (DataPlayerInfo other in Client.Data.GetRefs()) { @@ -198,8 +190,7 @@ public void Handle(CelesteNetConnection con, DataChannelMove move) { ghost == null) return; - ghost.RunOnUpdate(ghost => ghost.NameTag.Name = ""); - Ghosts.TryRemove(move.Player.ID, out _); + RemoveGhost(move.Player); foreach (DataType data in Client.Data.GetBoundRefs(move.Player)) if (data.TryGet(Client.Data, out MetaPlayerPrivateState state)) @@ -225,8 +216,7 @@ public void Handle(CelesteNetConnection con, DataPlayerState state) { Session session = Session; if (session != null && (state.SID != session.Area.SID || state.Mode != session.Area.Mode || state.Level == LevelDebugMap)) { - ghost.RunOnUpdate(ghost => ghost.NameTag.Name = ""); - Ghosts.TryRemove(id, out _); + RemoveGhost(ghost.PlayerInfo); return; } @@ -244,8 +234,11 @@ public void Handle(CelesteNetConnection con, DataPlayerGraphics graphics) { } Level level = PlayerBody?.Scene as Level; - if (ghost == null && !IsGhostOutside(Session, level, graphics.Player, out _)) + if (ghost == null && !IsGhostOutside(Session, level, graphics.Player, out _)) { ghost = CreateGhost(level, graphics.Player, graphics); + if (graphics.Player.ID == uint.MaxValue) + Logger.Log(LogLevel.WRN, "META", $"Handle graphics: Created ghost for ID uint.MaxValue ({graphics.Player.ID}) - should this be happening?"); + } if (ghost != null) ghost.UpdateGraphics(graphics); @@ -273,6 +266,8 @@ public void Handle(CelesteNetConnection con, DataPlayerFrame frame) { if (!Client.Data.TryGetBoundRef(frame.Player, out DataPlayerGraphics graphics) || graphics == null) return; ghost = CreateGhost(level, frame.Player, graphics); + if (frame.Player.ID == uint.MaxValue) + Logger.Log(LogLevel.WRN, "META", $"Handle frame: Created ghost for ID uint.MaxValue ({frame.Player.ID}) - should this be happening?"); } ghost.RunOnUpdate(ghost => { @@ -609,6 +604,14 @@ protected void RemoveGhost(DataPlayerInfo info) { ghost?.RunOnUpdate(g => g.NameTag.Name = ""); } + protected void RemoveAllGhosts() { + lock (Ghosts) { + foreach (Ghost ghost in Ghosts.Values) + ghost?.RemoveSelf(); + Ghosts.Clear(); + } + } + public void UpdateIdleTag(Entity target, ref GhostEmote idleTag, bool idle) { if (Engine.Scene is not Level level) { idle = false; @@ -677,11 +680,7 @@ public override void Update(GameTime gameTime) { GrabbedBy = null; if (ready && Engine.Scene is MapEditor) { - Player = null; - PlayerBody = null; - Session = null; - WasIdle = false; - WasInteractive = false; + ResetState(); AreaKey area = (AreaKey) f_MapEditor_area.GetValue(null); if (MapEditorArea == null || MapEditorArea.Value.SID != area.SID || MapEditorArea.Value.Mode != area.Mode) { @@ -691,11 +690,7 @@ public override void Update(GameTime gameTime) { } if (Player != null && MapEditorArea == null) { - Player = null; - PlayerBody = null; - Session = null; - WasIdle = false; - WasInteractive = false; + ResetState(); SendState(); } return; @@ -732,12 +727,9 @@ public override void Update(GameTime gameTime) { } if (Player == null || Player.Scene != level) { - Player = level.Tracker.GetEntity(); - if (Player != null) { - PlayerBody = Player; - Session = level.Session; - WasIdle = false; - WasInteractive = false; + Player player = level.Tracker.GetEntity(); + if (player != null) { + ResetState(player, level.Session); StateUpdated |= true; SendGraphics(); } @@ -807,24 +799,19 @@ public void OnSetActualDepth(On.Monocle.Scene.orig_SetActualDepth orig, Scene sc public void OnLoadLevel(On.Celeste.Level.orig_LoadLevel orig, Level level, Player.IntroTypes playerIntro, bool isFromLoader = false) { orig(level, playerIntro, isFromLoader); - Session = level.Session; - WasIdle = false; - WasInteractive = false; + Player player = null; - if (Client == null) - return; + if (Client != null) + player = level.Tracker.GetEntity(); - Player = level.Tracker.GetEntity(); - PlayerBody = Player; + ResetState(player, level.Session); - SendState(); + if (Client != null) + SendState(); } public void OnExitLevel(Level level, LevelExit exit, LevelExit.Mode mode, Session session, HiresSnow snow) { - Session = null; - WasIdle = false; - WasInteractive = false; - + // the first thing Cleanup does is call ResetState. Cleanup(); SendState(); @@ -842,12 +829,7 @@ private Player OnLoadNewPlayer(On.Celeste.Level.orig_LoadNewPlayer orig, Vector2 private void OnPlayerAdded(On.Celeste.Player.orig_Added orig, Player self, Scene scene) { orig(self, scene); - Session = (scene as Level)?.Session; - WasIdle = false; - WasInteractive = false; - Player = self; - PlayerBody = self; - + ResetState(self, (scene as Level)?.Session); SendState(); SendGraphics(); @@ -918,6 +900,13 @@ private void ILTransitionRoutine(ILContext il) { #endregion + public void ResetState(Player player = null, Session ses = null) { + Player = player; + PlayerBody = player; + Session = ses; + WasIdle = false; + WasInteractive = false; + } #region Send From 54c90358759ed533bc2ae24f0eb323cb192a8bba Mon Sep 17 00:00:00 2001 From: RedFlames Date: Mon, 31 Oct 2022 03:13:22 +0100 Subject: [PATCH 02/10] VS loves adding useless 'using' statements... and added the NameTag change from #29 --- CelesteNet.Client/Components/CelesteNetMainComponent.cs | 1 - CelesteNet.Client/Entities/GhostNameTag.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/CelesteNet.Client/Components/CelesteNetMainComponent.cs b/CelesteNet.Client/Components/CelesteNetMainComponent.cs index 9ebeb30b..b2bd6b66 100644 --- a/CelesteNet.Client/Components/CelesteNetMainComponent.cs +++ b/CelesteNet.Client/Components/CelesteNetMainComponent.cs @@ -10,7 +10,6 @@ using Monocle; using MonoMod.Cil; using MonoMod.RuntimeDetour; -using Steamworks; using System; using System.Collections.Concurrent; using System.Collections.Generic; diff --git a/CelesteNet.Client/Entities/GhostNameTag.cs b/CelesteNet.Client/Entities/GhostNameTag.cs index 857f43ce..25e2dd70 100644 --- a/CelesteNet.Client/Entities/GhostNameTag.cs +++ b/CelesteNet.Client/Entities/GhostNameTag.cs @@ -45,7 +45,7 @@ public override void Render() { float scale = level.GetScreenScale(); - Vector2 pos = Tracking?.Position ?? Position; + Vector2 pos = Tracking?.BottomCenter ?? Position; pos.Y -= 16f; pos = level.WorldToScreen(pos); From 05cd0626adc7bc3bb16557511ae85727cc6314a0 Mon Sep 17 00:00:00 2001 From: RedFlames Date: Mon, 31 Oct 2022 22:46:47 +0100 Subject: [PATCH 03/10] Fix double deaths --- CelesteNet.Client/Components/CelesteNetMainComponent.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CelesteNet.Client/Components/CelesteNetMainComponent.cs b/CelesteNet.Client/Components/CelesteNetMainComponent.cs index b2bd6b66..d011c287 100644 --- a/CelesteNet.Client/Components/CelesteNetMainComponent.cs +++ b/CelesteNet.Client/Components/CelesteNetMainComponent.cs @@ -726,9 +726,9 @@ public override void Update(GameTime gameTime) { } if (Player == null || Player.Scene != level) { - Player player = level.Tracker.GetEntity(); - if (player != null) { - ResetState(player, level.Session); + Player = level.Tracker.GetEntity(); + if (Player != null) { + ResetState(Player, level.Session); StateUpdated |= true; SendGraphics(); } From e680209be629a7e3b01295cdbd35a610f5b96435 Mon Sep 17 00:00:00 2001 From: RedFlames Date: Tue, 1 Nov 2022 00:25:02 +0100 Subject: [PATCH 04/10] Don't assign to Player outside of ResetState --- CelesteNet.Client/Components/CelesteNetMainComponent.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CelesteNet.Client/Components/CelesteNetMainComponent.cs b/CelesteNet.Client/Components/CelesteNetMainComponent.cs index d011c287..e8f3fba4 100644 --- a/CelesteNet.Client/Components/CelesteNetMainComponent.cs +++ b/CelesteNet.Client/Components/CelesteNetMainComponent.cs @@ -726,9 +726,9 @@ public override void Update(GameTime gameTime) { } if (Player == null || Player.Scene != level) { - Player = level.Tracker.GetEntity(); - if (Player != null) { - ResetState(Player, level.Session); + Player player = level.Tracker.GetEntity(); + if (player != Player) { + ResetState(player, level.Session); StateUpdated |= true; SendGraphics(); } From 6f9e502e48ec27e47884a5335a35db3cdf051085 Mon Sep 17 00:00:00 2001 From: RedFlames Date: Tue, 1 Nov 2022 01:03:21 +0100 Subject: [PATCH 05/10] Make RemoveGhost return a bool if there was indeed a ghost to remove --- .../Components/CelesteNetMainComponent.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/CelesteNet.Client/Components/CelesteNetMainComponent.cs b/CelesteNet.Client/Components/CelesteNetMainComponent.cs index e8f3fba4..a977e76c 100644 --- a/CelesteNet.Client/Components/CelesteNetMainComponent.cs +++ b/CelesteNet.Client/Components/CelesteNetMainComponent.cs @@ -185,12 +185,9 @@ public void Handle(CelesteNetConnection con, DataChannelMove move) { } } else { - if (!Ghosts.TryGetValue(move.Player.ID, out Ghost ghost) || - ghost == null) + if (!RemoveGhost(move.Player)) return; - RemoveGhost(move.Player); - foreach (DataType data in Client.Data.GetBoundRefs(move.Player)) if (data.TryGet(Client.Data, out MetaPlayerPrivateState state)) Client.Data.FreeBoundRef(data); @@ -598,9 +595,11 @@ protected Ghost CreateGhost(Level level, DataPlayerInfo player, DataPlayerGraphi return ghost; } - protected void RemoveGhost(DataPlayerInfo info) { - Ghosts.TryRemove(info.ID, out Ghost ghost); + protected bool RemoveGhost(DataPlayerInfo info) { + if (!Ghosts.TryRemove(info.ID, out Ghost ghost)) + return false; ghost?.RunOnUpdate(g => g.NameTag.Name = ""); + return true; } protected void RemoveAllGhosts() { From a3b716b94827a167912acd53a89856bd62b3102e Mon Sep 17 00:00:00 2001 From: RedFlames Date: Tue, 1 Nov 2022 01:05:58 +0100 Subject: [PATCH 06/10] Revert argument name change here --- CelesteNet.Client/Components/CelesteNetMainComponent.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CelesteNet.Client/Components/CelesteNetMainComponent.cs b/CelesteNet.Client/Components/CelesteNetMainComponent.cs index a977e76c..9e7145a2 100644 --- a/CelesteNet.Client/Components/CelesteNetMainComponent.cs +++ b/CelesteNet.Client/Components/CelesteNetMainComponent.cs @@ -595,8 +595,8 @@ protected Ghost CreateGhost(Level level, DataPlayerInfo player, DataPlayerGraphi return ghost; } - protected bool RemoveGhost(DataPlayerInfo info) { - if (!Ghosts.TryRemove(info.ID, out Ghost ghost)) + protected bool RemoveGhost(DataPlayerInfo player) { + if (!Ghosts.TryRemove(player.ID, out Ghost ghost)) return false; ghost?.RunOnUpdate(g => g.NameTag.Name = ""); return true; From 44be2829ce79ced6098b718c7cbf8b00cbb4058c Mon Sep 17 00:00:00 2001 From: RedFlames Date: Tue, 1 Nov 2022 01:43:00 +0100 Subject: [PATCH 07/10] Add changes from PR #29 onto this refactor branch --- .../Components/CelesteNetMainComponent.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/CelesteNet.Client/Components/CelesteNetMainComponent.cs b/CelesteNet.Client/Components/CelesteNetMainComponent.cs index 9e7145a2..98f92f37 100644 --- a/CelesteNet.Client/Components/CelesteNetMainComponent.cs +++ b/CelesteNet.Client/Components/CelesteNetMainComponent.cs @@ -587,7 +587,6 @@ protected Ghost CreateGhost(Level level, DataPlayerInfo player, DataPlayerGraphi UnsupportedSpriteModes.Add(graphics.SpriteMode); RunOnMainThread(() => { level.Add(ghost); - level.OnEndOfFrame += () => ghost.Active = true; ghost.UpdateGraphics(graphics); }); ghost.UpdateGraphics(graphics); @@ -673,6 +672,10 @@ public EventInstance PlayAudio(Ghost ghost, string sound, Vector2? at, string pa public override void Update(GameTime gameTime) { base.Update(gameTime); + foreach (Ghost g in Ghosts.Values) + if (g != null) + g.Active = true; + bool ready = Client != null && Client.IsReady && Client.PlayerInfo != null; if (Engine.Scene is not Level level || !ready) { GrabbedBy = null; @@ -831,8 +834,10 @@ private void OnPlayerAdded(On.Celeste.Player.orig_Added orig, Player self, Scene SendState(); SendGraphics(); - foreach (DataPlayerFrame frame in LastFrames.Values.ToArray()) - Handle(null, frame); + scene.OnEndOfFrame += () => { + foreach (DataPlayerFrame frame in LastFrames.Values.ToArray()) + Handle(null, frame); + }; } private PlayerDeadBody OnPlayerDie(On.Celeste.Player.orig_Die orig, Player self, Vector2 direction, bool evenIfInvincible, bool registerDeathInStats) { @@ -899,6 +904,9 @@ private void ILTransitionRoutine(ILContext il) { #endregion public void ResetState(Player player = null, Session ses = null) { + // Clear ghosts if the scene changed + if (player != null && player?.Scene != Player?.Scene) + RemoveAllGhosts(); Player = player; PlayerBody = player; Session = ses; From de9118ad4b9b5376d59384185e6650b2729045c7 Mon Sep 17 00:00:00 2001 From: RedFlames Date: Tue, 1 Nov 2022 02:49:36 +0100 Subject: [PATCH 08/10] Introduce flag for keeping ghosts on reset --- CelesteNet.Client/Components/CelesteNetMainComponent.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CelesteNet.Client/Components/CelesteNetMainComponent.cs b/CelesteNet.Client/Components/CelesteNetMainComponent.cs index 98f92f37..e158e5ad 100644 --- a/CelesteNet.Client/Components/CelesteNetMainComponent.cs +++ b/CelesteNet.Client/Components/CelesteNetMainComponent.cs @@ -730,7 +730,7 @@ public override void Update(GameTime gameTime) { if (Player == null || Player.Scene != level) { Player player = level.Tracker.GetEntity(); if (player != Player) { - ResetState(player, level.Session); + ResetState(player, level.Session, keepGhosts: Player != null && Player.Scene == null); StateUpdated |= true; SendGraphics(); } @@ -903,9 +903,9 @@ private void ILTransitionRoutine(ILContext il) { #endregion - public void ResetState(Player player = null, Session ses = null) { + public void ResetState(Player player = null, Session ses = null, bool keepGhosts = false) { // Clear ghosts if the scene changed - if (player != null && player?.Scene != Player?.Scene) + if (!keepGhosts && player?.Scene != Player?.Scene) RemoveAllGhosts(); Player = player; PlayerBody = player; From b855093ee4bfa3b3e08bdb9cf93980978b2687bd Mon Sep 17 00:00:00 2001 From: RedFlames Date: Tue, 1 Nov 2022 05:10:22 +0100 Subject: [PATCH 09/10] Prevent Ghosts from disappearing during death sequence - can't set PlayerBody null there... --- CelesteNet.Client/Components/CelesteNetMainComponent.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CelesteNet.Client/Components/CelesteNetMainComponent.cs b/CelesteNet.Client/Components/CelesteNetMainComponent.cs index e158e5ad..2f284965 100644 --- a/CelesteNet.Client/Components/CelesteNetMainComponent.cs +++ b/CelesteNet.Client/Components/CelesteNetMainComponent.cs @@ -730,7 +730,7 @@ public override void Update(GameTime gameTime) { if (Player == null || Player.Scene != level) { Player player = level.Tracker.GetEntity(); if (player != Player) { - ResetState(player, level.Session, keepGhosts: Player != null && Player.Scene == null); + ResetState(player, level.Session, keepPB: true); StateUpdated |= true; SendGraphics(); } @@ -903,12 +903,13 @@ private void ILTransitionRoutine(ILContext il) { #endregion - public void ResetState(Player player = null, Session ses = null, bool keepGhosts = false) { + public void ResetState(Player player = null, Session ses = null, bool keepPB = false, bool keepGhosts = false) { // Clear ghosts if the scene changed if (!keepGhosts && player?.Scene != Player?.Scene) RemoveAllGhosts(); Player = player; - PlayerBody = player; + if(!keepPB) + PlayerBody = player; Session = ses; WasIdle = false; WasInteractive = false; From 6a4ae7a59ec344b85ed6fba6d2d205eb9ea7e22e Mon Sep 17 00:00:00 2001 From: RedFlames Date: Tue, 1 Nov 2022 19:39:46 +0100 Subject: [PATCH 10/10] Move the respawn logic checks for the keepGhosts & PlayerBody conditionals into ResetState --- .../Components/CelesteNetMainComponent.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CelesteNet.Client/Components/CelesteNetMainComponent.cs b/CelesteNet.Client/Components/CelesteNetMainComponent.cs index 2f284965..52d57257 100644 --- a/CelesteNet.Client/Components/CelesteNetMainComponent.cs +++ b/CelesteNet.Client/Components/CelesteNetMainComponent.cs @@ -730,7 +730,7 @@ public override void Update(GameTime gameTime) { if (Player == null || Player.Scene != level) { Player player = level.Tracker.GetEntity(); if (player != Player) { - ResetState(player, level.Session, keepPB: true); + ResetState(player, level.Session); StateUpdated |= true; SendGraphics(); } @@ -903,12 +903,14 @@ private void ILTransitionRoutine(ILContext il) { #endregion - public void ResetState(Player player = null, Session ses = null, bool keepPB = false, bool keepGhosts = false) { - // Clear ghosts if the scene changed + public void ResetState(Player player = null, Session ses = null, bool keepGhosts = false) { + // Clear ghosts if the scene changed, but not on respawns + keepGhosts |= player == null && Player != null && Player.Scene == null; if (!keepGhosts && player?.Scene != Player?.Scene) RemoveAllGhosts(); Player = player; - if(!keepPB) + // when keeping Ghosts we also need to keep PlayerBody apparently, because some Ghost logic uses PlayerBody.Scene as Level + if(!keepGhosts) PlayerBody = player; Session = ses; WasIdle = false;