Remove Mods.Common bridge, split Integration, and fix doc drift - #6
Open
CTristan wants to merge 6 commits into
Open
Remove Mods.Common bridge, split Integration, and fix doc drift#6CTristan wants to merge 6 commits into
CTristan wants to merge 6 commits into
Conversation
Deletes LmmConfigurationProvider (the IConfigProvider implementation that bridged Mods.Common's ModConfig/IConfigEntry to LmmConfigFile) and the provider-wiring block in Harmony_Patch that attached the provider to every registered ModConfig. Upstream Mods.Common has also dropped these config abstractions: mod settings are not a concern of a general reflection-facade library. A source generator shipped from this repo will reintroduce the optional-dependency integration without requiring mods to carry an extra runtime DLL. Also drops the Mods.Common PackageReference from both csprojs (no longer used), the Common .dll staging step from the release workflow, and adds a local ExcludeFromCodeCoverageAttribute polyfill in place of the one that previously came transitively from Common's ILRepack.
Ships build-time glue as one NuGet reference with PrivateAssets="all" so mods integrate with ConfigurationManager without a runtime dependency on ConfigurationManager.dll. The source generator emits reflection-based interop directly into the consuming mod's own assembly, so the mod ships one DLL and still runs when ConfigurationManager is absent (bindings fall back to an in-memory store). Also adds Audience & Language guidance to CLAUDE.md to shape user-facing text (README, error messages, release notes) for native Korean ESL readers and first-time modders, and extends the release workflow to pack and upload the nupkg alongside the existing zip asset.
Wires up samples/SampleMod/ as a complete, buildable reference for the Integration package's optional-dependency pattern, and rewrites README / copilot-instructions to target ESL first-time mod authors. Adds the sample to the solution so CI enforces the net35 compile-check the csproj promises.
The source-generator package and its tests now live at open-lobotomy/LobotomyCorporation.Mods.ConfigurationManager.Integration. This repo retains only the LGPL-3.0 runtime mod DLL and its tests. Side effects: - Removes LICENSES.md and LICENSE-MIT; single-license repo. - Removes the samples/ directory (migrated to Integration repo). - Dual-credits BepInEx contributors + Open Lobotomy in Directory.Build.props. - Marks the runtime csproj IsPackable=false (defense against accidental pack). - Trims release.yml to the zip installer path; NuGet pack/push moved to Integration repo's release.yml. - README loses the Integration-authoring walkthrough; adds a Related link back to the Integration repo for mod authors.
…ssions Align ConfigurationManagerAttributes with upstream BepInEx.ConfigurationManager by using public fields (read via Type.GetFields) instead of auto-properties, so a plugin's copy-paste template copied from either source works without modification. Update SettingEntryBase.SetFromAttributes to match. Add ApiVersion = 1 constant on LmmConfigRegistration so the LobotomyCorporation.Mods.ConfigurationManager.Integration source generator can probe at runtime and refuse to register against a mismatched contract instead of silently corrupting bindings. Fold two near-identical Register overloads into one. Move all inline analyzer suppressions (S1104, CS0649, CA1054, IDE0130) to path-scoped sections in .editorconfig. Each section carries a comment explaining why the rule is wrong for that specific file, so reviewers see every exception and its rationale in one place rather than chasing #pragmas and [SuppressMessage] attributes through source. CA1054 widens from per-member to per-file in CommonHelpers.cs; the scope note in .editorconfig flags that tradeoff.
- Add LmmConfigRegistration.Register<T> string overload so README "individual settings" example compiles and the Register API matches LmmConfigFile.Bind (which already has both overloads). - Correct README and CLAUDE.md notes about the ConfigurationManagerAttributes template: the fork uses public fields, matching upstream, so copy-as-is works from either source. Previous text told authors to convert to auto-properties, which would silently break attribute matching. - Trim CLAUDE.md Integration/SampleMod audience guidance now that both live in the separate open-lobotomy/LobotomyCorporation.Mods.ConfigurationManager.Integration repo, and update the auto-scan path to the real location. - Drop Microsoft.CodeAnalysis.* and Verify.* PackageVersion entries from Directory.Packages.props — no project references them since the Integration split.
There was a problem hiding this comment.
Pull request overview
This PR updates ConfigurationManager after the Integration repo split by removing the deprecated Mods.Common bridge, aligning the author-facing attribute/template contract with upstream, and cleaning up related docs/build metadata.
Changes:
- Remove the
LobotomyCorporation.Mods.Commonconfiguration bridge (runtime + tests) and stop shipping/copying its DLL in release packaging. - Switch
ConfigurationManagerAttributesand reflection plumbing to public fields (GetFields) and centralize analyzer suppressions in.editorconfig. - Fix documentation drift and add a
LmmConfigRegistration.Register<T>overload that accepts a plain description string.
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Rewrites audience/installation/author guidance; updates attribute-template guidance and adds links/license section. |
| LobCorp.ConfigurationManager/Utilities/CommonHelpers.cs | Removes per-member CA1054 suppression (now handled via .editorconfig). |
| LobCorp.ConfigurationManager/Polyfills/ExcludeFromCodeCoverageAttribute.cs | Adds a net35 polyfill for ExcludeFromCodeCoverageAttribute. |
| LobCorp.ConfigurationManager/LobCorp.ConfigurationManager.csproj | Removes Mods.Common dependency; marks project non-packable. |
| LobCorp.ConfigurationManager/Implementations/SettingEntryBase.cs | Changes attribute tag-copying to read public fields via reflection. |
| LobCorp.ConfigurationManager/Implementations/LmmConfigurationProvider.cs | Removes Mods.Common bridge implementation. |
| LobCorp.ConfigurationManager/Harmony_Patch.cs | Removes provider wiring to Mods.Common ModConfig. |
| LobCorp.ConfigurationManager/ConfigurationManagerAttributes.cs | Converts template from auto-properties to public fields; updates docs accordingly. |
| LobCorp.ConfigurationManager/Config/LmmConfigRegistration.cs | Adds ApiVersion constant + string-description Register<T> overload; updates doc comments. |
| LobCorp.ConfigurationManager.Test/ModTests/.../LmmSettingEntryTests.cs | Updates foreign-attributes stub + test name to field shape. |
| LobCorp.ConfigurationManager.Test/ModTests/.../LmmConfigurationProviderTests.cs | Removes tests for the deleted Mods.Common bridge. |
| LobCorp.ConfigurationManager.Test/LobCorp.ConfigurationManager.Test.csproj | Removes Mods.Common references; updates comment about coverage polyfill interaction. |
| Directory.Packages.props | Removes centrally managed version for Mods.Common. |
| Directory.Build.props | Updates Authors/Copyright metadata. |
| .gitignore | Adds *.lscache. |
| .github/workflows/release.yml | Uses github.actor + GITHUB_TOKEN; stops copying Mods.Common DLL into release staging. |
| .github/copilot-instructions.md | Updates repo docs re: config path + Integration repo ownership + attributes field contract. |
| .editorconfig | Adds file-scoped analyzer suppressions and rationale comments. |
| .DS_Store | Adds a macOS metadata file (should not be committed). |
Comment on lines
222
to
+256
| @@ -236,32 +236,31 @@ var propertyPair in _myProperties.Join( | |||
| { | |||
| try | |||
| { | |||
| var val = propertyPair.other.GetValue(attrib, null); | |||
| var val = memberPair.other.GetValue(attrib); | |||
| if (val != null) | |||
| { | |||
| if ( | |||
| propertyPair.my.PropertyType | |||
| != propertyPair.other.PropertyType | |||
| memberPair.my.PropertyType != memberPair.other.FieldType | |||
| && typeof(Delegate).IsAssignableFrom( | |||
| propertyPair.my.PropertyType | |||
| memberPair.my.PropertyType | |||
| ) | |||
| ) | |||
| { | |||
| val = Delegate.CreateDelegate( | |||
| propertyPair.my.PropertyType, | |||
| memberPair.my.PropertyType, | |||
| ((Delegate)val).Target, | |||
| ((Delegate)val).Method | |||
| ); | |||
| } | |||
|
|
|||
| propertyPair.my.SetValue(this, val, null); | |||
| memberPair.my.SetValue(this, val, null); | |||
There was a problem hiding this comment.
SetFromAttributes now only reads ConfigurationManagerAttributes values via public fields (GetFields). That will stop honoring tags from mods built against earlier versions of this fork that used the public auto-property template (those tags will have properties but no fields). To avoid breaking existing mods, consider supporting both shapes (read fields first, then fall back to properties) when copying values.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch rebuilds the optional-dependency story for mod authors. Upstream changes landed in sequence:
LobotomyCorporation.Mods.Commonconfig bridge (no longer needed)Integrationsource-generator package, then split it into its own repo (open-lobotomy/LobotomyCorporation.Mods.ConfigurationManager.Integration)ConfigurationManagerAttributestemplate to public fields (matching upstream) and centralize analyzer suppressionsThe final commit on the branch is a post-split cleanup pass that fixes documentation drift and one API inconsistency surfaced by a review:
LmmConfigRegistration.Register<T>string overload — the README example passed a string where the existingRegisteroverload requiredLmmConfigDescription, so it wouldn't compile. Added a string overload matching the pair already present onLmmConfigFile.Bind.Microsoft.CodeAnalysis.*andVerify.*fromDirectory.Packages.props; no project references them after the Integration split.Test plan
dotnet buildclean (0 warnings, 0 errors)dotnet test— 176/176 pass