Skip to content

Rewrite the FAWE configuration system - #3703

Open
MattBDev wants to merge 23 commits into
fix/v3/worldedit-config-loadingfrom
refactor/v3/settings-records
Open

MattBDev wants to merge 23 commits into
fix/v3/worldedit-config-loadingfrom
refactor/v3/settings-records

Conversation

@MattBDev

@MattBDev MattBDev commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Description

FAWE's config.yml is now loaded into read-only Java records. Each setting's default is written next to the setting, and code changes a setting with a method reference such as Settings.History::compressionLevel.

Removed API

Before After
Settings.settings().QUEUE.PARALLEL_THREADS Settings.settings().queue().parallelThreads()
Settings.IMP Settings.settings()
Settings.settings().EXPERIMENTAL.PERSISTENT_BRUSHES = false Settings.override(Settings.Experimental::persistentBrushes, false)
Settings.settings().reload(File) Settings.set(Settings.load(Path))
Settings.settings().save(File) settings.save(Path)
Settings.settings().LIMITS.get("default") Settings.settings().defaultLimits()
Settings.settings().PROTOCOL_SUPPORT_FIX FaweCache.INSTANCE.setProtocolSupportFix(boolean)
Config and its annotations @Comment, @Default

Performance

The new code loads, reloads and saves faster, works out limits more than twice as fast, and allocates much less memory. Reading a setting costs the same.

Benchmark Old New Speed-up Old alloc New alloc
getLimit, player in no group, stock file 225 ± 21 ns 85 ± 15 ns 2.64× 640 B 264 B
getLimit, player in two groups, 8 groups 1.14 ± 0.10 µs 473 ± 23 ns 2.40× 2 KB 752 B
Six setting reads 2.1 ± 0.1 ns 2.0 ± 0.1 ns 1.03× 0 B 0 B
Load, stock file 1.28 ± 0.25 ms 0.62 ± 0.05 ms 2.07× 1.31 MB 515 KB
Reload, stock file 1.74 ± 0.10 ms 0.67 ± 0.02 ms 2.60× 1.42 MB 583 KB
Forced save, stock file 0.61 ± 0.04 ms 0.45 ± 0.04 ms 1.35× 96 KB 56 KB

Settings becomes a tree of records: each nested record is a section of
config.yml and each component an option, with its comment in @comment
and its default in @default. ConfigMapper binds a parsed file to the
records and writes them back, so key names no longer come from field
names by reflection. config.yml keeps its keys, order, comments and
defaults.

- Settings.override and Settings#with change an option through an
  accessor reference, so a wrong value type is a compile error.
- Reloading builds a new tree and swaps one reference.
- Strings are escaped when written, an unreadable file is not
  overwritten, and an unchanged file is not rewritten.
- LimitResolver prepares each limit group once per loaded
  configuration instead of on every getLimit call.
- PROTOCOL_SUPPORT_FIX moves to FaweCache#setProtocolSupportFix, since
  a record cannot hold a runtime flag.

Config and ConfigOptComputation are removed and every use of the old
Settings fields is migrated.
@MattBDev
MattBDev added this pull request to stack #3704 October 21, 2026 04:54
Settings becomes a tree of records: each nested record is a section of
config.yml and each component an option, with its comment in @comment
and its default in @default. ConfigMapper binds a parsed file to the
records and writes them back, so key names no longer come from field
names by reflection. config.yml keeps its keys, order, comments and
defaults.

- Settings.override and Settings#with change an option through an
  accessor reference, so a wrong value type is a compile error.
- Reloading builds a new tree and swaps one reference.
- Strings are escaped when written, an unreadable file is not
  overwritten, and an unchanged file is not rewritten.
- LimitResolver prepares each limit group once per loaded
  configuration instead of on every getLimit call.
- PROTOCOL_SUPPORT_FIX moves to FaweCache#setProtocolSupportFix, since
  a record cannot hold a runtime flag.

Config and ConfigOptComputation are removed and every use of the old
Settings fields is migrated.
@MattBDev
MattBDev force-pushed the refactor/v3/settings-records branch from 6ef49ee to 4adf724 Compare October 21, 2026 04:55
@MattBDev
MattBDev requested a balanced review from Copilot October 21, 2026 04:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It is an explicitly Work-In-Progress draft that rewrites the core configuration system and mechanically touches ~70 call sites, which warrants human review before approval.

3 open issues
What changed in this PR

This PR replaces FAWE's legacy reflection-based Config.java configuration system with an immutable, record-based Settings API. Configuration sections become nested Java records whose components are annotated with @Comment/@Default, and a new mapping layer (ConfigMapper, ConfigYaml, ConfigTree, LimitResolver) handles record↔YAML serialization, migration of legacy keys, and limit-group resolution. Roughly 70 call sites are mechanically migrated from uppercase field access (Settings.settings().QUEUE.PARALLEL_THREADS) to accessor methods (Settings.settings().queue().parallelThreads()). The PR is explicitly a Work-In-Progress draft.

Changes:

  • Introduce an immutable record-based Settings model plus a new serialization/migration framework and comprehensive unit tests.
  • Rewrite Fawe.setupConfigs() to load/save via Settings.load/save with a BuildInfo, and move the FaweCache.protocolSupportFix flag off the global settings object.
  • Migrate ~70 call sites to the new accessor API and delete Config.java/ConfigOptComputation.java.
File Description
.../​configuration/​Settings.java New immutable record config model, load/save/migrate/defaults and BuildInfo (context only; not in diff).
.../​configuration/​ConfigMapper.java Reflection-based record↔YAML mapping, coercion, and defaults.
.../​configuration/​ConfigYaml.java Hand-rolled UTF-8 YAML reader/writer with quoting/escaping.
.../​configuration/​ConfigTree.java Dotted-path tree used by migration.
.../​configuration/​LimitResolver.java Merges limit groups into a resolved FaweLimit ("most generous" semantics).
.../​configuration/​{Option,Default,Comment}.java Annotations/metadata driving option paths, defaults and comments.
.../​core/​Fawe.java Rewritten setupConfigs() using Path, Settings.load/save, and BuildInfo.
.../​core/​FaweCache.java New protocolSupportFix field + setter replacing the old global setting.
.../​test/​.../​SettingsTest.java, ConfigMapperTest.java New unit tests covering save/load, migration, limits and fuzzed text round-tripping.
.../​test/​resources/​.../​config.yml Expected-output fixture mirroring the @Comment/@Default text; carries over several comment typos.
~70 migrated call sites Mechanical switch from uppercase fields to accessor methods.

Review effort: Balanced

@MattBDev
MattBDev marked this pull request as ready for review October 23, 2026 00:35
@MattBDev
MattBDev requested a review from a team as a code owner October 23, 2026 00:35
@MattBDev
MattBDev marked this pull request as draft October 23, 2026 00:35
…rate schematic limits into default

Also makes protocolSupportFix volatile and sets the remaining @SInCE tags to 3.0.0.
…form from the saved config

Also finishes the option to setting rename in javadoc and replaces the assert with Entry.requireType().
…ngs-records

Brings in main via v3, including 26.3 support. Config.java stays deleted
and Settings.java keeps the records version: ConfigYaml already writes no
space after a list key and the comment strings carry no trailing spaces,
which is what #3675 and #3676 fixed in the old classes. The 26.3 adapter
gets the same record accessors as the other adapters.
- Remove the unused experimental.anvil-queue-mode setting and a duplicated
  history comment line; existing files have the key reported and removed.
- Use var for entry loops and fix minor javadoc in the configuration classes.
- Remove savedSettingsLoadBackUnchanged and missingFileGivesDefaults, which
  repeated anyTextIsReadBackAsWritten and aFileWithNothingToReadGivesTheFallback.
- Make the allowed-image-hosts case differ from its default so it can fail,
  and drop input lines no assertion checked.
- Add YAML 1.1 boolean words to the anyTextIsReadBackAsWritten samples.
Copilot AI and others added 2 commits October 28, 2026 20:51
Co-authored-by: MattBDev <4009945+MattBDev@users.noreply.github.com>
Co-authored-by: MattBDev <4009945+MattBDev@users.noreply.github.com>
@MattBDev

Copy link
Copy Markdown
Contributor Author

Marking the PR as ready for review but I still have to update the PR body. Should be done Saturday. I think I have a minor tweak or two to I have to push as well.

@MattBDev
MattBDev marked this pull request as ready for review November 1, 2026 18:43
@MattBDev
MattBDev requested a balanced review from Copilot November 1, 2026 18:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

An unresolved critical finding remains: a typo in the config.yml comment (clipboad.on-disk instead of clipboard.use-disk) that must be mirrored in the Settings.java @Comment or the defaultsAreWrittenAsExpected test will fail.

1 open issue
3 resolved since last review

Review effort: Balanced

# Allows NBT stored in a clipboard to be written to disk
# - Requires clipboard.use-disk to be enabled
save-clipboard-nbt-to-disk: true
# Apply a file lock on the clipboard file (only relevant if clipboad.on-disk is enabled)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed the typo in the authoritative Settings.java comment and synced the expected generated config.yml output to clipboard.use-disk.

Co-authored-by: MattBDev <4009945+MattBDev@users.noreply.github.com>
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.

3 participants