Skip to content

Commit

Permalink
Config cleanup, part 2 (#178)
Browse files Browse the repository at this point in the history 
* Rewrite config read/write logic to be more straightforward

* Remove config tracking from the `ModContainer` itself
  • Loading branch information
@Technici4n
Technici4n authored Jul 10, 2024
1 parent af00d44 commit cd7eafe
Show file tree
Hide file tree
Showing 7 changed files with 147 additions and 116 deletions.
15 changes: 2 additions & 13 deletions loader/src/main/java/net/neoforged/fml/ModContainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import static net.neoforged.fml.Logging.LOADING;

import java.util.EnumMap;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Optional;
Expand All @@ -21,7 +20,6 @@
import net.neoforged.fml.config.ModConfig;
import net.neoforged.fml.event.IModBusEvent;
import net.neoforged.fml.event.lifecycle.FMLConstructModEvent;
import net.neoforged.fml.loading.FMLPaths;
import net.neoforged.neoforgespi.language.IModInfo;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand All @@ -48,7 +46,6 @@ public abstract class ModContainer {
protected final String namespace;
protected final IModInfo modInfo;
protected final Map<Class<? extends IExtensionPoint>, Supplier<?>> extensionPoints = new IdentityHashMap<>();
protected final EnumMap<ModConfig.Type, ModConfig> configs = new EnumMap<>(ModConfig.Type.class);

public ModContainer(IModInfo info) {
this.modId = info.getModId();
Expand Down Expand Up @@ -95,14 +92,6 @@ public <T extends IExtensionPoint> void registerExtensionPoint(Class<T> point, S
extensionPoints.put(point, extension);
}

private void addConfig(final ModConfig modConfig) {
configs.put(modConfig.getType(), modConfig);

if (modConfig.getType() == ModConfig.Type.STARTUP) {
ConfigTracker.INSTANCE.openConfig(modConfig, FMLPaths.CONFIGDIR.get(), null);
}
}

/**
* Adds a {@link ModConfig} with the given type and spec. An empty config spec will be ignored and a debug line will
* be logged.
Expand All @@ -117,7 +106,7 @@ public void registerConfig(ModConfig.Type type, IConfigSpec configSpec) {
return;
}

addConfig(ConfigTracker.INSTANCE.registerConfig(type, configSpec, this));
ConfigTracker.INSTANCE.registerConfig(type, configSpec, this);
}

/**
Expand All @@ -134,7 +123,7 @@ public void registerConfig(ModConfig.Type type, IConfigSpec configSpec, String f
return;
}

addConfig(ConfigTracker.INSTANCE.registerConfig(type, configSpec, this, fileName));
ConfigTracker.INSTANCE.registerConfig(type, configSpec, this, fileName);
}

/**
Expand Down
146 changes: 85 additions & 61 deletions loader/src/main/java/net/neoforged/fml/config/ConfigTracker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,35 @@
package net.neoforged.fml.config;

import com.electronwill.nightconfig.core.CommentedConfig;
import com.electronwill.nightconfig.core.ConfigFormat;
import com.electronwill.nightconfig.core.file.CommentedFileConfig;
import com.electronwill.nightconfig.core.InMemoryCommentedFormat;
import com.electronwill.nightconfig.core.UnmodifiableCommentedConfig;
import com.electronwill.nightconfig.core.concurrent.ConcurrentCommentedConfig;
import com.electronwill.nightconfig.core.concurrent.SynchronizedConfig;
import com.electronwill.nightconfig.core.file.FileWatcher;
import com.electronwill.nightconfig.core.io.ParsingException;
import com.electronwill.nightconfig.core.io.ParsingMode;
import com.electronwill.nightconfig.core.io.WritingMode;
import com.electronwill.nightconfig.toml.TomlFormat;
import com.electronwill.nightconfig.toml.TomlParser;
import com.electronwill.nightconfig.toml.TomlWriter;
import com.mojang.logging.LogUtils;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumMap;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Function;
import net.neoforged.fml.ModContainer;
import net.neoforged.fml.event.config.ModConfigEvent;
import net.neoforged.fml.loading.FMLConfig;
Expand Down Expand Up @@ -80,6 +88,11 @@ public ModConfig registerConfig(ModConfig.Type type, IConfigSpec spec, ModContai
var lock = locksByMod.computeIfAbsent(container.getModId(), m -> new ReentrantLock());
var modConfig = new ModConfig(type, spec, container, fileName, lock);
trackConfig(modConfig);

if (modConfig.getType() == ModConfig.Type.STARTUP) {
openConfig(modConfig, FMLPaths.CONFIGDIR.get(), null);
}

return modConfig;
}

Expand Down Expand Up @@ -113,29 +126,19 @@ public void unloadConfigs(ModConfig.Type type) {
this.configSets.get(type).forEach(ConfigTracker::closeConfig);
}

public static void openConfig(ModConfig config, Path configBasePath, @Nullable Path configOverrideBasePath) {
static void openConfig(ModConfig config, Path configBasePath, @Nullable Path configOverrideBasePath) {
LOGGER.trace(CONFIG, "Loading config file type {} at {} for {}", config.getType(), config.getFileName(), config.getModId());
if (config.config != null) {
LOGGER.warn("Opening a config that was already loaded with value {} at path {}", config.config, config.getFileName());
if (config.loadedConfig != null) {
LOGGER.warn("Opening a config that was already loaded with value {} at path {}", config.loadedConfig, config.getFileName());
}
var basePath = resolveBasePath(config, configBasePath, configOverrideBasePath);
var configPath = basePath.resolve(config.getFileName());
var configData = CommentedFileConfig.builder(configPath)
.sync()
.preserveInsertionOrder()
.autosave()
.onFileNotFound((newfile, configFormat) -> setupConfigFile(config, newfile, configFormat))
.writingMode(WritingMode.REPLACE_ATOMIC)
.build();
LOGGER.debug(CONFIG, "Built TOML config for {}", configPath);

loadConfig(config, configData);

loadConfig(config, configPath, ModConfigEvent.Loading::new);
LOGGER.debug(CONFIG, "Loaded TOML config file {}", configPath);
config.config = configData;
config.postConfigEvent(ModConfigEvent.Loading::new);

if (!FMLConfig.getBoolConfigValue(FMLConfig.ConfigValue.DISABLE_CONFIG_WATCHER)) {
FileWatcher.defaultInstance().addWatch(configPath, new ConfigWatcher(config, configData, Thread.currentThread().getContextClassLoader()));
FileWatcher.defaultInstance().addWatch(configPath, new ConfigWatcher(config, configPath, Thread.currentThread().getContextClassLoader()));
LOGGER.debug(CONFIG, "Watching TOML config file {} for changes", configPath);
}
}
Expand All @@ -151,102 +154,123 @@ private static Path resolveBasePath(ModConfig config, Path configBasePath, @Null
return configBasePath;
}

static void loadConfig(ModConfig modConfig, CommentedFileConfig config) {
static void loadConfig(ModConfig modConfig, Path path, Function<ModConfig, ModConfigEvent> eventConstructor) {
CommentedConfig config;

try {
config.load();
// Load existing config
config = readConfig(path);

if (!modConfig.getSpec().isCorrect(config)) {
LOGGER.warn(CONFIG, "Configuration file {} is not correct. Correcting", config.getFile().getAbsolutePath());
backUpConfig(config);
LOGGER.warn(CONFIG, "Configuration file {} is not correct. Correcting", path);
backUpConfig(path);
modConfig.getSpec().correct(config);
config.save();
writeConfig(path, config);
}
} catch (NoSuchFileException ignored) {
// Config file does not exist yet
try {
setupConfigFile(modConfig, path);
config = readConfig(path);
} catch (IOException | ParsingException ex) {
throw new RuntimeException("Failed to create default config file " + modConfig.getFileName() + " of type " + modConfig.getType() + " for modid " + modConfig.getModId(), ex);
}
} catch (ParsingException ex) {
LOGGER.warn(CONFIG, "Failed to parse {}: {}. Attempting to recreate", modConfig.getFileName(), ex);
} catch (IOException | ParsingException ex) {
// Failed to read existing file
LOGGER.warn(CONFIG, "Failed to load config {}: {}. Attempting to recreate", modConfig.getFileName(), ex);
try {
backUpConfig(config);
Files.delete(config.getNioPath());
backUpConfig(path);
Files.delete(path);

setupConfigFile(modConfig, config.getNioPath(), config.configFormat());
config.load();
setupConfigFile(modConfig, path);
config = readConfig(path);
} catch (Throwable t) {
ex.addSuppressed(t);

throw new RuntimeException("Failed to recreate config file " + modConfig.getFileName() + " of type " + modConfig.getType() + " for modid " + modConfig.getModId(), ex);
}
}
modConfig.getSpec().acceptConfig(config);

modConfig.setConfig(new LoadedConfig(config, path, modConfig), eventConstructor);
}

public static void acceptSyncedConfig(ModConfig modConfig, byte[] bytes) {
if (modConfig.config != null) {
LOGGER.warn("Overwriting non-null config {} at path {} with synced config", modConfig.config, modConfig.getFileName());
if (modConfig.loadedConfig != null) {
LOGGER.warn("Overwriting non-null config {} at path {} with synced config", modConfig.loadedConfig, modConfig.getFileName());
}
modConfig.config = TomlFormat.instance().createParser().parse(new ByteArrayInputStream(bytes));
// TODO: do we want to do any validation? (what do we do if it fails?)
modConfig.getSpec().acceptConfig(modConfig.config);
modConfig.postConfigEvent(ModConfigEvent.Reloading::new); // TODO: should maybe be Loading on the first load?
var newConfig = new SynchronizedConfig(InMemoryCommentedFormat.defaultInstance(), LinkedHashMap::new);
newConfig.bulkCommentedUpdate(view -> {
TomlFormat.instance().createParser().parse(new ByteArrayInputStream(bytes), view, ParsingMode.REPLACE);
});
// TODO: do we want to do any validation? (what do we do if acceptConfig fails?)
modConfig.setConfig(new LoadedConfig(newConfig, null, modConfig), ModConfigEvent.Reloading::new); // TODO: should maybe be Loading on the first load?
}

public void loadDefaultServerConfigs() {
configSets.get(ModConfig.Type.SERVER).forEach(modConfig -> {
if (modConfig.config != null) {
LOGGER.warn("Overwriting non-null config {} at path {} with default server config", modConfig.config, modConfig.getFileName());
if (modConfig.loadedConfig != null) {
LOGGER.warn("Overwriting non-null config {} at path {} with default server config", modConfig.loadedConfig, modConfig.getFileName());
}
modConfig.config = createDefaultConfig(modConfig.getSpec());
modConfig.getSpec().acceptConfig(modConfig.config);
modConfig.postConfigEvent(ModConfigEvent.Loading::new);

modConfig.setConfig(new LoadedConfig(createDefaultConfig(modConfig.getSpec()), null, modConfig), ModConfigEvent.Loading::new);
});
}

private static CommentedConfig createDefaultConfig(IConfigSpec spec) {
var commentedConfig = CommentedConfig.inMemory();
spec.correct(commentedConfig);
var commentedConfig = new SynchronizedConfig(InMemoryCommentedFormat.defaultInstance(), LinkedHashMap::new);
commentedConfig.bulkCommentedUpdate(spec::correct);
return commentedConfig;
}

private static void closeConfig(ModConfig config) {
if (config.config != null) {
if (config.config instanceof CommentedFileConfig) {
if (config.loadedConfig != null) {
if (config.loadedConfig.path() != null) {
LOGGER.trace(CONFIG, "Closing config file type {} at {} for {}", config.getType(), config.getFileName(), config.getModId());
unload(config);
config.config = null;
config.getSpec().acceptConfig(null);
config.postConfigEvent(ModConfigEvent.Unloading::new);
unload(config.loadedConfig.path());
config.setConfig(null, ModConfigEvent.Unloading::new);
} else {
LOGGER.warn(CONFIG, "Closing non-file config {} at path {}", config.config, config.getFileName());
LOGGER.warn(CONFIG, "Closing non-file config {} at path {}", config.loadedConfig, config.getFileName());
}
}
}

private static void unload(ModConfig config) {
private static void unload(Path path) {
if (FMLConfig.getBoolConfigValue(FMLConfig.ConfigValue.DISABLE_CONFIG_WATCHER))
return;
Path configPath = config.getFullPath();
try {
FileWatcher.defaultInstance().removeWatch(configPath);
FileWatcher.defaultInstance().removeWatch(path);
} catch (RuntimeException e) {
LOGGER.error("Failed to remove config {} from tracker!", configPath, e);
LOGGER.error("Failed to remove config {} from tracker!", path, e);
}
}

private static boolean setupConfigFile(ModConfig modConfig, Path file, ConfigFormat<?> conf) throws IOException {
private static void setupConfigFile(ModConfig modConfig, Path file) throws IOException {
Files.createDirectories(file.getParent());
Path p = defaultConfigPath.resolve(modConfig.getFileName());
if (Files.exists(p)) {
LOGGER.info(CONFIG, "Loading default config file from path {}", p);
Files.copy(p, file);
} else {
conf.createWriter().write(createDefaultConfig(modConfig.getSpec()), file, WritingMode.REPLACE_ATOMIC);
writeConfig(file, createDefaultConfig(modConfig.getSpec()));
}
return true;
}

private static void backUpConfig(CommentedFileConfig commentedFileConfig) {
backUpConfig(commentedFileConfig, 5); //TODO: Think of a way for mods to set their own preference (include a sanity check as well, no disk stuffing)
private static ConcurrentCommentedConfig readConfig(Path path) throws IOException, ParsingException {
try (var reader = Files.newBufferedReader(path)) {
var config = new SynchronizedConfig(TomlFormat.instance(), LinkedHashMap::new);
config.bulkCommentedUpdate(view -> {
new TomlParser().parse(reader, view, ParsingMode.REPLACE);
});
return config;
}
}

static void writeConfig(Path file, UnmodifiableCommentedConfig config) {
new TomlWriter().write(config, file, WritingMode.REPLACE_ATOMIC);
}

private static void backUpConfig(CommentedFileConfig commentedFileConfig, int maxBackups) {
backUpConfig(commentedFileConfig.getNioPath(), maxBackups);
private static void backUpConfig(Path commentedFileConfig) {
backUpConfig(commentedFileConfig, 5); //TODO: Think of a way for mods to set their own preference (include a sanity check as well, no disk stuffing)
}

private static void backUpConfig(Path commentedFileConfig, int maxBackups) {
Expand Down
14 changes: 5 additions & 9 deletions loader/src/main/java/net/neoforged/fml/config/ConfigWatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,21 @@

package net.neoforged.fml.config;

import com.electronwill.nightconfig.core.file.CommentedFileConfig;
import com.mojang.logging.LogUtils;
import java.nio.file.Path;
import net.neoforged.fml.event.config.ModConfigEvent;
import org.slf4j.Logger;

class ConfigWatcher implements Runnable {
private static final Logger LOGGER = LogUtils.getLogger();

private final ModConfig modConfig;
private final CommentedFileConfig commentedFileConfig;
private final Path path;
private final ClassLoader realClassLoader;

ConfigWatcher(ModConfig modConfig, CommentedFileConfig commentedFileConfig, ClassLoader classLoader) {
ConfigWatcher(ModConfig modConfig, Path path, ClassLoader classLoader) {
this.modConfig = modConfig;
this.commentedFileConfig = commentedFileConfig;
this.path = path;
this.realClassLoader = classLoader;
}

Expand All @@ -32,11 +32,7 @@ public void run() {
modConfig.lock.lock();
try {
LOGGER.debug(ConfigTracker.CONFIG, "Config file {} changed, re-loading", modConfig.getFileName());
if (this.modConfig.config != this.commentedFileConfig) {
LOGGER.warn(ConfigTracker.CONFIG, "Config file {} has a mismatched loaded config. Expected {} but was {}.", modConfig.getFileName(), commentedFileConfig, modConfig.config);
}
ConfigTracker.loadConfig(this.modConfig, this.commentedFileConfig);
this.modConfig.postConfigEvent(ModConfigEvent.Reloading::new);
ConfigTracker.loadConfig(this.modConfig, this.path, ModConfigEvent.Reloading::new);
} finally {
modConfig.lock.unlock();
}
Expand Down
20 changes: 17 additions & 3 deletions loader/src/main/java/net/neoforged/fml/config/IConfigSpec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
* <h3>Thread-safety</h3>
* <p>The {@link Config} objects themselves are thread-safe, but mod config code should not be assumed to be thread-safe in general.
* FML will guard event dispatches behind a lock when necessary.
* A spec can safely mutate {@code Config} objects, but should let FML fire events,
* for example using {@link ModConfig#onConfigChanged()}.
* A spec can safely mutate {@code Config} objects, but should let FML fire events and saving the file by calling {@link ILoadedConfig#save()}.
*/
public interface IConfigSpec {
/**
Expand Down Expand Up @@ -53,5 +52,20 @@ public interface IConfigSpec {
* This is called on loading and on reloading.
* The config is guaranteed to be valid according to {@link #isCorrect}.
*/
void acceptConfig(@Nullable CommentedConfig config);
void acceptConfig(@Nullable ILoadedConfig config);

sealed interface ILoadedConfig permits LoadedConfig {
/**
* Accesses the current config.
*
* <p>The config locks internally, and is therefore safe to
* read/write across multiple threads without additional synchronization.
*/
CommentedConfig config();

/**
* Saves the current value of the {@link #config} and dispatches a config reloading event.
*/
void save();
}
}
Loading

2 comments on commit cd7eafe

@cpw
Copy link
Member

@cpw cpw commented on cd7eafe Jul 12, 2024

Choose a reason for hiding this comment

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

Why?

@TelepathicGrunt
Copy link
Contributor

Choose a reason for hiding this comment

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

@cpw it was part of this PR and was reviewed and looks at by a few people
#178

is there a specific concern you have with the code changes?

Please sign in to comment.