Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hotkey Sync fixed #2

Merged
merged 3 commits into from
Mar 29, 2023
Merged

Hotkey Sync fixed #2

merged 3 commits into from
Mar 29, 2023

Conversation

JackTheFragger
Copy link

@JackTheFragger JackTheFragger commented Mar 27, 2023 •
edited
Loading

valheimPlus#801

  • fixed hotkey sync: local configured hotkeys are used, if syncHotkeyConfig is disabled
  • fixed sync for option syncHotkeyConfig: remote setting will be ignored
  • refactoring edited code for better readibility
  • removed unused property Settings in Configuration

@JackTheFragger
- local Hotkey settings will be used, if serverSyncHotkeys is disabled
6000127 
- serverSyncHotkeys will only read from local config
- refactoring for better readibility
- addex xml comments for syncsServerConfig and serverSyncHotkeys for better understanding in code
@Grantapher
Copy link
Owner

Grantapher commented Mar 28, 2023 •
edited
Loading

Can you open a duplicate PR over on the main repo (once I eventually merge it)? I think they may merge this there as well (whenever they return), and otherwise would hate for it to get lost when they do return.

Grantapher
Grantapher reviewed Mar 28, 2023
View reviewed changes
Copy link
Owner

@Grantapher Grantapher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The direction this PR goes LGTM, just a few small things.

ValheimPlus/Configurations/BaseConfig.cs Outdated Show resolved Hide resolved
ValheimPlus/RPC/VPlusConfigSync.cs Outdated Show resolved Hide resolved
ValheimPlus/Configurations/BaseConfig.cs Show resolved Hide resolved
@Grantapher Grantapher self-assigned this Mar 28, 2023
@Grantapher Grantapher mentioned this pull request Mar 28, 2023
Closed
JackTheFragger and others added 2 commits March 28, 2023 20:48
@JackTheFragger
- try-catch-finally-block in RPC_VPlusConfigSync will be thrown
782a3fc 
- all properties with option "LoadMode.LocalOnly" will be using local values when syncing
@JackTheFragger
Update BaseConfig.cs
918c8d8 
Formatting in Line 61 corrected
Grantapher
Grantapher approved these changes Mar 29, 2023
View reviewed changes
Copy link
Owner

@Grantapher Grantapher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving as is to get this out ASAP. I will probably make a minor follow-up commit.

@Grantapher Grantapher merged commit df63e8c into Grantapher:grantapher-development Mar 29, 2023
@RazeDrake RazeDrake mentioned this pull request Dec 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Grantapher Grantapher Grantapher approved these changes

Assignees

@Grantapher Grantapher

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JackTheFragger @Grantapher