valheimPlus / ValheimPlus

A HarmonyX Mod aimed at improving the gameplay and quality of life of the game Valheim.
http://valheim.plus
GNU Affero General Public License v3.0
967 stars 236 forks source link

local defined Keys will not be loaded when connecting to a dedicated server #801

Open Grantapher opened 1 year ago

Grantapher commented 1 year ago

I have a bug, that local defined Keys will not be loaded when connecting to a dedicated server. Insted only the default values will be used. I have a fix for it, but can't commit it to your fork (branch: origin\grantapher-development). The button is disabled in VS2022.

I've changed the code in BaseConfig.cs from

                if (prop.PropertyType == typeof(KeyCode) && !RPC.VPlusConfigSync.isConnecting)
                {
                    prop.SetValue(this, data.GetKeyCode(keyName, (KeyCode)existingValue), null);
                    continue;
                }

to

                if (prop.PropertyType == typeof(KeyCode))
                {
                    prop.SetValue(this, data.GetKeyCode(keyName, (KeyCode)existingValue), null);
                    continue;
                }

This is necessary to use FreePlacementRotation without colliding with other keys (e.g. F is used for activateForsaken Powers. This is very frustrating.


Originally posted by @JackTheFragger in https://github.com/valheimPlus/ValheimPlus/issues/793#issuecomment-1483860769

Grantapher commented 1 year ago

Based on commit 149e08e it seems that the code you deleted exists to prevent the server from overriding keys defined in each player's local config. Are you certain you have Server.serverSyncHotkeys set to false? Are you certain you overwrote the key configuration in your local config?

JackTheFragger commented 1 year ago

My server config has the same key definitions. But they will never be read. I've tried key sync enabled and disabled. The properly name 'isConnecting' doedn't say, that it's the KeySync option. I will test it.

JackTheFragger commented 1 year ago

The Code is bullshit, because:

The Sync will read the full config and replaces the local config with the server one. If HotKeySync is disabled, the new config will take only the default values, because they will not been overwritten with the local defined... That's why your local config will never run

JackTheFragger commented 1 year ago

I've added my tested changes. Now it will peform correctyl. And: now server sync settings will be used for decisions, not local settings.

Grantapher commented 1 year ago

What exactly are you trying to get the functionality to be? The server shouldn't overwrite the player's own choices for the hotkeys unless the player opts in, which I believe is what the code originally aimed for.

And although it does defy the "server syncs config" stuff, this seems like it should be an exception.

JackTheFragger commented 1 year ago

ServerSync has to be locked by the server for avoiding "cheating". The HotkeySync-Option is something, I've never understood. Who is using it? And why? The option will be overwritten with first sync and will stay there until quitting the game. So it should defined by the server for deterministic.

JackTheFragger commented 1 year ago

If local defining of server hotkey sync is required, code has to be changed, too, to avoid server overwriting this option.

Please answer, what you think about it.

Grantapher commented 1 year ago
; Changes whether the server will force it's config on clients that connect. Only affects servers.
; WE HEAVILY RECOMMEND TO NEVER DISABLE THIS!
serverSyncsConfig=true

serverSyncsConfig does say it only affects servers, so agree that the client value shouldn't have an effect there.

; If false allows you to keep your own defined hotkeys instead of synchronising the ones declared for the server. 
; Sections need to be enabled in your local configuration to load hotkeys. 
; This is a client side setting and not affected by server settings.
serverSyncHotkeys=false

serverSyncHotkeys says the opposite, only client side configuration affects it, so we should stay with that. It makes sense that players should be able to customize hotkeys.

JackTheFragger commented 1 year ago

Thanks for clarification. I will change the code.

JackTheFragger commented 1 year ago

I've edited my code with refactorings for better readibility and fix for overwriting local value for syncHotkeyConfig (only local will be used and remote ignored).