Ghosts in PICO-8 #151

balt-dev · 2024-07-25T15:49:10Z

Adds ghosts to PICO-8!
https://www.youtube.com/watch?v=OWObGCg-KDo

balt-dev · 2024-07-26T04:11:31Z

I tested this as best I could using 3 instances of Celeste hooked up to a locally hosted server. Stress tests may be in order?

…ghost settings

balt-dev · 2024-07-28T19:57:07Z

Did a lot of refactoring per some advice on discord, there is no longer a server module needed!

CelesteNet.Shared/DataTypes/DataPicoState.cs

CelesteNet.Client/Entities/PicoGhost.cs

CelesteNet.Client/Components/CelesteNetPico8Component.cs

balt-dev · 2024-08-02T19:30:57Z

oh you know what i totally forgot to test that before pushing. one second

balt-dev · 2024-09-25T04:03:28Z

any updates on this?

RedFlames · 2024-09-26T03:30:39Z

Hey, really sorry about the lack of activity 😅

Personally been having some trouble with working on CNet stuff or what to prioritize, but I can probably take a more thorough look at this again over the weekend?

This is definitely a solid PR that is 90% where it should be, but I still see some areas I wanna go over.
Like some nullable-related stuff looks quite confusing to me, e.g. in the LevelIndex property... which besides that imho should be declared before the methods of the class as well 🤔
Wondering if the location SID should be "Celeste/PICO-8" since that would fit in with vanilla and make the change in player list code unnecessary.

RedFlames

I don't even know what exactly GitHub does when you "Finish" a Review, but I'm doing it :33:

RedFlames · 2024-09-29T12:38:58Z

CelesteNet.Client/Components/CelesteNetPico8Component.cs

+using System.Reflection;
+
+namespace Celeste.Mod.CelesteNet.Client.Components;
+#nullable enable


I'm not even sure why Client is the only project that doesn't have nullable context enabled. We should look into if we can turn that on for the whole project and fix warnings, rather than just toggling it for this file alone imo.

RedFlames · 2024-09-29T12:46:16Z

CelesteNet.Client/Components/CelesteNetPico8Component.cs

+
+        CelesteNetPlayerListComponent.OnGetState += ModifyStateOfPicoPlayers;
+        CelesteNetMainComponent.OnSendState += ModifyStateInPico;
+        var client = Context?.Client;


drop the var and use the Client property of CelesteNetGameComponent?

RedFlames · 2024-09-29T12:50:17Z

CelesteNet.Client/Components/CelesteNetPico8Component.cs

+
+        CelesteNetPlayerListComponent.OnGetState -= ModifyStateOfPicoPlayers;
+        CelesteNetMainComponent.OnSendState -= ModifyStateInPico;
+        var client = Context?.Client;


(Same as line 65)

RedFlames · 2024-09-29T13:48:19Z

CelesteNet.Client/Components/CelesteNetPico8Component.cs

+    private void ModifyStateInPico(DataPlayerState state)
+    {
+        uint? id = state.Player?.ID;
+        if (id == null) return;


Just check state.Player == null, drop the local var and possibly combine the two conditional returns

RedFlames · 2024-09-29T13:50:53Z

CelesteNet.Client/Components/CelesteNetPico8Component.cs

+
+        Logger.Log(LogLevel.DBG, "PICO8-CNET", $"Modifying state!");
+
+        state.SID = $"PICO-8";


As I already said I think it'd make sense to make this "Celeste/PICO-8", and since the constant shows up in multiple places, make it a member constant variable?

RedFlames · 2024-09-29T16:04:08Z

CelesteNet.Client/Components/CelesteNetPico8Component.cs

+            var x = (float?) nodeData.Get("x") ?? float.NaN;
+            var y = (float?) nodeData.Get("y") ?? float.NaN;
+            var size = (float?) nodeData.Get("size") ?? float.NaN;


use .Get<float>()? 🤔
Unsure why VS then tells me it wouldn't even return a float? but maybe that's nullable-context jank we gotta fix...

RedFlames · 2024-09-29T16:06:28Z

CelesteNet.Client/Components/CelesteNetPico8Component.cs

+        if (_classic == null) {
+            Logger.Log(LogLevel.WRN, "PICO8-CNET", "Failed to retrieve Classic");
+            return null;
+        }


if _classic was null, InitData wouldn't have returned true and the previous conditional would've returned, right

RedFlames · 2024-09-29T16:10:40Z

CelesteNet.Client/Components/CelesteNetPlayerListComponent.cs

@@ -492,7 +492,7 @@ private string GetOrderKey(DataPlayerInfo player) {
                return "9";

            if (Client?.Data?.TryGetBoundRef(player, out DataPlayerState state) == true && !string.IsNullOrEmpty(state?.SID))
-                return $"0 {(state.SID.StartsWith("Celeste/") ? "0" : "1") + state.SID + (int) state.Mode} {player.FullName}";
+                return $"0 {(state.SID.StartsWith("Celeste/") || state.SID == "PICO-8" ? "0" : "1") + state.SID + (int) state.Mode} {player.FullName}";


Change would be avoided by using SID "Celeste/PICO-8"

RedFlames · 2024-09-29T16:14:59Z

CelesteNet.Client/Entities/GhostNameTag.cs

@@ -1,4 +1,5 @@
-using Microsoft.Xna.Framework;
+using Celeste.Pico8;


Changes in this file are superfluous? The SceneAs vs. is not seems okay although idk about the { return; } instead of newline no-braces ;)

RedFlames · 2024-09-29T16:17:31Z

CelesteNet.Client/Pico8FontHelper.cs

+    public static void Print(string text, int x, int y) {
+        Print(text, x, y, PicoWhite);
+    }
+
+    public static void Print(string text, int x, int y, Color color) {
+        var initialX = x;
+        for (var i = 0; i < text.Length; i += char.IsSurrogatePair(text, i) ? 2 : 1) {
+            var codepoint = char.ConvertToUtf32(text, i);
+            if (codepoint == 0x0A) {
+                // Newline
+                x = initialX;
+                y += 5;
+                continue;
+            }
+            var sprite = CharacterSprite((uint) codepoint);
+
+            sprite.Draw(new Vector2(x, y), Vector2.Zero, color);
+            x += 4;
+        }
+    }
+    public static void PrintOutlinedCenter(string text, int x, int y) {
+        PrintOutlinedCenter(text, x, y, PicoWhite, PicoBlack);
+    }
+
+    public static void PrintOutlinedCenter(string text, int x, int y, Color inside, Color outside) {
+        Print(text, x - (text.Length / 2 * 4) - 1, y - 1, outside);
+        Print(text, x - (text.Length / 2 * 4)    , y - 1, outside);
+        Print(text, x - (text.Length / 2 * 4) + 1, y - 1, outside);
+        Print(text, x - (text.Length / 2 * 4) - 1, y    , outside);
+        Print(text, x - (text.Length / 2 * 4) + 1, y    , outside);
+        Print(text, x - (text.Length / 2 * 4) - 1, y + 1, outside);
+        Print(text, x - (text.Length / 2 * 4)    , y + 1, outside);
+        Print(text, x - (text.Length / 2 * 4) + 1, y + 1, outside);
+        Print(text, x - (text.Length / 2 * 4)    , y    , inside);
+    }


Could give the Color parameters default values instead of the overloaded methods? 🤷

balt-dev added 9 commits July 24, 2024 20:51

Basic implementation (janky and broken)

f9953ee

Add hair, switch to DynamicData

886de98

Ghost cleanup seems to be working now

621d287

Player names and such

3a91170

Fully implement player names, player list

df0baa7

Add font files

14484c7

Remove stray class

f089cf0

Add a few locks

970fc2b

Privatize classicData

56160cf

balt-dev marked this pull request as ready for review July 26, 2024 04:10

balt-dev added 6 commits July 26, 2024 13:38

Remove a bit of unnecessary logic and remove all ghosts on disconnect

c50bd1d

Some refactoring, removing server module

9dbefc3

Fix death animation, properly check for if player in PICO-8, respect …

27c3c87

…ghost settings

Clarify reasoning of Name getter

9917c5f

Add pausing for chat in PICO-8

40c981a

Un-hardcode hair length in case another mod messes with it

16411bc

balt-dev added 2 commits July 30, 2024 22:19

CHANGE LICENSE ON FONT

b74f945

Fix null deref crash

6679216

RedFlames reviewed Aug 2, 2024

View reviewed changes

CelesteNet.Shared/DataTypes/DataPicoState.cs Outdated Show resolved Hide resolved

RedFlames reviewed Aug 2, 2024

View reviewed changes

CelesteNet.Client/Entities/PicoGhost.cs Outdated Show resolved Hide resolved

RedFlames reviewed Aug 2, 2024

View reviewed changes

CelesteNet.Client/Components/CelesteNetPico8Component.cs Outdated Show resolved Hide resolved

Apply some refactoring and JetBrains Secret Sauce TM

02003cf

balt-dev and others added 3 commits August 2, 2024 14:59

Fix hair drawing when player is hidden

25111aa

Shorten a few if checks

1bcb9e0

Merge branch '0x0ade:main' into pico8

29704f3

RedFlames requested changes Sep 29, 2024

View reviewed changes

balt-dev added 2 commits October 6, 2024 13:35

Update README.txt

3e4d12c

Add files via upload

e9372ba

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ghosts in PICO-8 #151

Ghosts in PICO-8 #151

balt-dev commented Jul 25, 2024 •

edited

Loading

balt-dev commented Jul 26, 2024

balt-dev commented Jul 28, 2024

balt-dev commented Aug 2, 2024

balt-dev commented Sep 25, 2024

RedFlames commented Sep 26, 2024

RedFlames left a comment

RedFlames Sep 29, 2024

RedFlames Sep 29, 2024

RedFlames Sep 29, 2024

RedFlames Sep 29, 2024

RedFlames Sep 29, 2024

RedFlames Sep 29, 2024

RedFlames Sep 29, 2024

RedFlames Sep 29, 2024

RedFlames Sep 29, 2024

RedFlames Sep 29, 2024


		Logger.Log(LogLevel.DBG, "PICO8-CNET", $"Modifying state!");

		state.SID = $"PICO-8";

		@@ -1,4 +1,5 @@
		using Microsoft.Xna.Framework;
		using Celeste.Pico8;

Ghosts in PICO-8 #151

Are you sure you want to change the base?

Ghosts in PICO-8 #151

Conversation

balt-dev commented Jul 25, 2024 • edited Loading

balt-dev commented Jul 26, 2024

balt-dev commented Jul 28, 2024

balt-dev commented Aug 2, 2024

balt-dev commented Sep 25, 2024

RedFlames commented Sep 26, 2024

RedFlames left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balt-dev commented Jul 25, 2024 •

edited

Loading