Noita-Together / noita-together

Play alone together
MIT License
44 stars 13 forks source link

Feature/emotes πŸ‘β€οΈπŸ‘‹πŸ‘ˆπŸ‘ #118

Closed infinitesunrise closed 7 months ago

infinitesunrise commented 8 months ago

This is a rough, working integration of the 2023111800 version of noita-emotes into noita-together. Still some things keeping this from being ready for main at time of pull:

And some notes:

tehkab commented 7 months ago

Just getting to this, sorry...

Tested in beta branch, getting a lua error, looks like the keybind setting is missing?

Lua error: [string "mods/noita-together/settings.lua"]:69: bad argument #1 to 'gmatch' (string expected, got nil)

I tried copypasting from the standalone mod and made this change which appeared to work, but you're more familiar with how that works so maybe you want to do it differently?

I should also note that after generating the setting, the lua error disappears and the keybind works without this code but it's still missing in the mod settings. So the problem is half if noita-together.NT_EMOTE_BINDING doesn't yet exist (which it wont for any first-time user), and half that its unchangable if it does exist?

diff --git a/noita_mod/core/settings.lua b/noita_mod/core/settings.lua
index 7fc98c1..d174da8 100755
--- a/noita_mod/core/settings.lua
+++ b/noita_mod/core/settings.lua
@@ -47,6 +47,21 @@ local old_binding = ModSettingGet("noita-together.NT_EMOTE_BINDING")

 function ModSettingsUpdate( init_scope )
        local old_version = mod_settings_get_version( mod_id )
+
+       local settings = {}
+
+    local binding = {
+        key = "noita-together.NT_EMOTE_BINDING",
+        default = "key_code,20,mouse_code,joystick_code"
+    }
+    table.insert(settings, binding)
+
+    for _, setting in ipairs(settings) do
+        if (ModSettingGet(setting.key) == nil) then
+            ModSettingSet(setting.key, setting.default)
+        end
+    end
+
        mod_settings_update( mod_id, mod_settings, init_scope )
 end
infinitesunrise commented 7 months ago

I did some digging into recognized settings table options, turns out there's a "hidden" boolean you can add that does exactly what it sounds like it does. So with that, I was able to add the emote keybind setting in with the rest of the settings, while my dynamic keybind rendering is still intact.

I'm actually not sure if my "old_binding" variable is even needed now, but I left it in there with an added default fallback as it doesn't hurt.

infinitesunrise commented 7 months ago

I've added another setting to this PR to turn display of emotes / skin swaps / emotes menu on and off, as it's possible that some players might wish to play NT without added visual antics from the player ghosts. Works dynamically in-game: The emotes system entities remain present on the player and player ghosts so that they can listen for this setting changing and respond to it.

If a remote player was actively emoting when the setting is enabled, the local player won't see them start emoting at that moment as emote changes are only transmitted when they're started or stopped. But the local player will see the next emote change sent by the remote player. Same for skin color swaps. As for when the setting is disabled, the emote system entities will note that change dynamically and end any active emotes / switch skins back to default purple at that moment.

Github is claiming there's a merge conflict but I think it's just overwhelmed by settings.lua being altered so radically- You may want to verify this yourself before merging. I've made sure to preserve the new NT Fridge option that was just added to the next branch earlier this week and updated some errant spaces to tabs.

SkyeOfBreeze commented 7 months ago

Hey @infinitesunrise! Thanks!

Here are some issues @tehkab and I found:

Also, there is one more conflict in the settings to be resolved. I changed a setting name

infinitesunrise commented 7 months ago

That is some dang good QA, thank you.

As for the cape colors not being restored when the emote system is disabled and then re-enabled during gameplay, that is expected behavior due to NT's model of acting on received payloads but not storing the resulting state change anywhere. The skins will update if changed after the setting is re-enabled. AFAIK this is true for other ghost data, like position and wands... And bank changes too, I think? I figure users will either leave the setting on or off so it should rarely if ever be an issue. If it's truly bothersome, I can look into using NT's stringstore or a variable component on the ghost's emote entities to save data, but IMO that might be better saved for a later feature addition if requested as that's all new logic to work out.