Skip to content

Split ClientConfig off of ServerConfig and synchronize upon joining Server#130

Open
skorpnok wants to merge 1 commit into
jayugg:masterfrom
skorpnok:ServerClientConfigRework
Open

Split ClientConfig off of ServerConfig and synchronize upon joining Server#130
skorpnok wants to merge 1 commit into
jayugg:masterfrom
skorpnok:ServerClientConfigRework

Conversation

@skorpnok

@skorpnok skorpnok commented Jun 2, 2026

Copy link
Copy Markdown

Synchronize runtime ServerConfig with Server when joining.
fix some behaviours getting added twice.

Fixes #129 and probably other desync related bugs. Also fixes some of the double tooltip errors.

When no client config is found it tries to load the client values from the old shared SmithingPlus.json to create the new client config.

Summary by CodeRabbit

  • New Features

    • Added separate client-side configuration file for UI preferences and display settings.
    • Introduced new configuration options for client-specific behavior (repaired count visibility, repair smith name display, recipe visualization, and more).
  • Refactor

    • Reorganized configuration system to separate server and client settings into distinct configuration files.
    • Migrated UI-related settings from server configuration to dedicated client configuration.

Synchronize runtime ServerConfig with Server when joining
fix some behaviours getting added twice
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR separates client-side and server-side configuration into distinct structures. A new ClientConfig class holds display settings (repaired counts, recipe voxels, anvil columns); ServerConfig retains only server mechanics. ConfigLoader manages separate config files with fallback loading. Core initializes both configs at startup with side-dependent persistence. Client behaviors now read display settings from CConfig instead of Config. The problematic mold units serialization patches are removed, fixing multiplayer synchronization issues.

Changes

Config Architecture Separation

Layer / File(s) Summary
Config Schema Definition
SmithingPlus/Config/ClientConfig.cs, SmithingPlus/Config/ServerConfig.cs
ClientConfig introduces eight new client-side display and behavior properties (ShowRepairedCount, ShowBrokenCount, ShowRepairSmithName, AnvilShowRecipeVoxels, RememberHammerToolMode, ShowWorkableTemperature, HandbookExtraInfo, AnvilRecipeSelectionColumns). ServerConfig removes these same properties, retaining only server-side configuration.
Serialization Infrastructure
SmithingPlus/Util/TreeAttributeSerialize.cs
Generic TreeAttributeSerializer<T> utility enables reflection-based serialization of typed objects to/from Vintagestory ITreeAttribute, handling bool, int, float, and string properties with conditional deserialization checking for attribute presence.
Config File Management
SmithingPlus/Config/ConfigLoader.cs
ConfigLoader now manages separate SmithingPlus.json (server) and SmithingPlusClient.json (client) config files, attempts fallback loading from legacy filename when client file is missing, creates new configs with logging when files don't exist, and clears both Config and CConfig on disposal.
Config Initialization and Access
SmithingPlus/Core.cs
Introduces LocalConfig and CConfig static properties, changes Config to a runtime-settable property, adds OnlyEnableClientside flag, and implements side-dependent Start() logic: server copies LocalConfig into Config and persists to world tree attributes; client deserializes Config from world tree attributes.
Config Usage in Core and Behaviors
SmithingPlus/Core.cs, SmithingPlus/ClientTweaks/AnvilRecipeSelectorPatch.cs, SmithingPlus/ToolRecovery/CollectibleBehaviorBrokenToolHead.cs, SmithingPlus/ToolRecovery/CollectibleBehaviorRepairableTool.cs
Core updates entity behavior registration to read LocalConfig.ArrowsDropBits, refactors asset finalization to explicitly gate client-only workable-temperature behaviors using CConfig, and restructures patch enabling. Client patches and behaviors switch from Core.Config to Core.CConfig for display settings (anvil columns, broken/repaired counts, repair smith name).
Tool Mold Persistence Cleanup
SmithingPlus/CastingTweaks/ToolMoldUnitsPatch.cs
Removes ToTreeAttributes_Postfix and FromTreeAttributes_Postfix that previously serialized requiredUnits to tree attributes, resolving multiplayer synchronization issues where mold units displayed as 0/0 after chunk reloads due to conflicting client/server serialization.

Sequence Diagram(s)

sequenceDiagram
  participant Server
  participant Client
  participant ConfigLoader
  participant WorldState
  
  Server->>ConfigLoader: Load ServerConfig from SmithingPlus.json
  Client->>ConfigLoader: Load ClientConfig from SmithingPlusClient.json
  Server->>Server: Core.Start copies LocalConfig to Config
  Server->>WorldState: Serialize Config to world tree attributes
  Client->>WorldState: Deserialize Config from world tree attributes
  Client->>Client: Use CConfig for display, Config for mechanics
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit hops through config files with glee,
Splitting client from server—now synced they'll be!
No more zeros in molds when worlds reload,
Each side carries its own configuration load. 🐰✨

🚥 Pre-merge checks | ✅ 3 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Changes are primarily focused on config separation and synchronization; however, removal of ToTreeAttributes_Postfix and FromTreeAttributes_Postfix patches may be out of scope for the stated objective. Clarify why the ToolMoldUnitsPatch serialization methods were removed and whether this aligns with the issue #129 fix for mold unit desynchronization.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No description field was provided in the pull request context to evaluate. Provide a detailed pull request description explaining the motivation and implementation approach for config separation.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: splitting ClientConfig from ServerConfig and synchronizing it on server join.
Linked Issues check ✅ Passed The PR addresses issue #129 by separating client and server configs and ensuring synchronization, which should prevent desynchronization of mold unit display in multiplayer.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
SmithingPlus/Util/TreeAttributeSerialize.cs (1)

12-13: ⚡ Quick win

Guard against indexers and read-only/write-only properties.

GetProperties() returns every public property. Today the only non-{get;set;} member of ServerConfig is the computed string[] GetToolRepairForgettableAttributes, which is skipped only because its type isn't matched. If anyone later adds a computed bool/int/float/string property (e.g. a derived flag), FromTreeAttributes will call SetValue on a get-only property and throw at mod load. Filtering on CanRead/CanWrite (and indexer params) makes this reusable utility safe.

♻️ Proposed guard
-        foreach (PropertyInfo fi in typeof(T).GetProperties())
-        {
+        foreach (PropertyInfo fi in typeof(T).GetProperties())
+        {
+            if (!fi.CanRead || !fi.CanWrite || fi.GetIndexParameters().Length > 0) continue;
             var type = fi.PropertyType;

Apply the same guard in both ToTreeAttributes and FromTreeAttributes.

Also applies to: 30-31

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SmithingPlus/Util/TreeAttributeSerialize.cs` around lines 12 - 13, When
iterating properties in ToTreeAttributes and FromTreeAttributes, guard against
indexers and read-only/write-only properties by only processing properties where
PropertyInfo.CanRead and PropertyInfo.CanWrite are true and
PropertyInfo.GetIndexParameters().Length == 0; update the foreach over
typeof(T).GetProperties() in both methods (and any other similar loops) to skip
properties that fail these checks so SetValue/GetValue is never called on
indexers or get-only/write-only properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@SmithingPlus/Util/TreeAttributeSerialize.cs`:
- Around line 12-13: When iterating properties in ToTreeAttributes and
FromTreeAttributes, guard against indexers and read-only/write-only properties
by only processing properties where PropertyInfo.CanRead and
PropertyInfo.CanWrite are true and PropertyInfo.GetIndexParameters().Length ==
0; update the foreach over typeof(T).GetProperties() in both methods (and any
other similar loops) to skip properties that fail these checks so
SetValue/GetValue is never called on indexers or get-only/write-only properties.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8e6ec8be-7cb3-40d5-a41b-320696a18baf

📥 Commits

Reviewing files that changed from the base of the PR and between 241b9f2 and e4dc2bd.

📒 Files selected for processing (9)
  • SmithingPlus/CastingTweaks/ToolMoldUnitsPatch.cs
  • SmithingPlus/ClientTweaks/AnvilRecipeSelectorPatch.cs
  • SmithingPlus/Config/ClientConfig.cs
  • SmithingPlus/Config/ConfigLoader.cs
  • SmithingPlus/Config/ServerConfig.cs
  • SmithingPlus/Core.cs
  • SmithingPlus/ToolRecovery/CollectibleBehaviorBrokenToolHead.cs
  • SmithingPlus/ToolRecovery/CollectibleBehaviorRepairableTool.cs
  • SmithingPlus/Util/TreeAttributeSerialize.cs
💤 Files with no reviewable changes (1)
  • SmithingPlus/CastingTweaks/ToolMoldUnitsPatch.cs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dynamic Mold units 0/0 in multiplayer

1 participant