ConfigurationData#setValue() doesn't offer a way to set value of MapProperty
drori200 opened this issue · 5 comments
I created a MapProperty doing the following in my config:
private static Map<String, Integer> createDefaultMap() {
Map<String, Integer> map = new HashMap<>();
map.put(Ref.STR.getValue(), 10);
map.put(Ref.DEX.getValue(), 9);
map.put(Ref.CON.getValue(), 8);
map.put(Ref.WIL.getValue(), 7);
map.put(Ref.MND.getValue(), 6);
map.put(Ref.SPI.getValue(), 5);
// Organizes the Map properly
Map<String, Integer> treeMap = new TreeMap<String, Integer>(map);
return treeMap;
}
@Comment("The amount of stat points the player will get for each stat type.")
public static final MapProperty<Integer> STAT_AMOUNT = new MapProperty<>("StatAmount", createDefaultMap(),
PrimitivePropertyType.INTEGER);Now i am in the process of migrating my old config system but when i try to use the configurationData parameter from MigrationService#performMigrations there is nothing offered to set the value of the MapProperty STAT_AMOUNT
The same goes for the reader property not providing anything for getting a Map
ConfigurationData#setValue allows to pass in a value that matches the property type, so passing a map for your StatAmount will definitely work. PropertyReader reflects the YAML structure and so is more or less untyped in its return values, save for a few convenience methods I'm not too fond of.
If I understand your setup correctly, I think you have two ways to approach this. Either enumerate the old paths, retrieve them with a Property implementation and copy it over to the new path, or check generically if there is a map in the old place and take it from there. The first approach is easier to understand and less error-prone, I guess, but doesn't scale well. I'd recommend the second approach if you have a number of paths that is too much to list out.
First approach—get old paths with a local Property, if there is something, take it over:
public class StatMigrationService implements MigrationService {
/*
* Example for a YAML like:
* oldStats:
* oldStr: 3
* oldWil: 8
*
* Where oldStr and oldWil should be taken over into Ref.STR and Ref.WIL entries in the new map property.
*/
@Override
public boolean checkAndMigrate(@NotNull PropertyReader reader, @NotNull ConfigurationData configurationData) {
if (!reader.contains("oldStats")) {
return false; // Old path does not exist; nothing to migrate
}
Map<String, Integer> statValues = getCurrentStatValues(reader);
// Note: Chain statements with | and not || to make sure all are performed
boolean hasOldProperty =
setOldStatsValueIfPresent(statValues, reader, "oldStats.oldStr", Ref.STR)
| setOldStatsValueIfPresent(statValues, reader, "oldStats.oldDex", Ref.DEX)
| setOldStatsValueIfPresent(statValues, reader, "oldStats.oldCon", Ref.CON)
| setOldStatsValueIfPresent(statValues, reader, "oldStats.oldWil", Ref.WIL)
| setOldStatsValueIfPresent(statValues, reader, "oldStats.oldMnd", Ref.MND)
| setOldStatsValueIfPresent(statValues, reader, "oldStats.oldSpi", Ref.SPI);
if (hasOldProperty) {
configurationData.setValue(StatsSettingsHolder.STAT_AMOUNT, statValues);
}
return hasOldProperty;
}
// Gets the current stats under the new path, ensuring that the default value is not used
// if no values are present yet
private Map<String, Integer> getCurrentStatValues(PropertyReader reader) {
PropertyValue<Map<String, Integer>> propertyValue =
StatsSettingsHolder.STAT_AMOUNT.determineValue(reader);
if (propertyValue.isValidInResource()) {
return propertyValue.getValue();
}
return new TreeMap<>();
}
private boolean setOldStatsValueIfPresent(Map<String, Integer> statValues,
PropertyReader reader,
String oldPath, Ref newStatsReference) {
Property<Integer> oldProp = PropertyInitializer.newProperty(oldPath, -1);
PropertyValue<Integer> value = oldProp.determineValue(reader);
if (value.isValidInResource()) {
// #putIfAbsent to not have the old path override the new path's value since
// statsValues only has values that are actually in the YAML
statValues.putIfAbsent(newStatsReference.getValue(), value.getValue());
return true;
}
return false;
}
}Skeleton of the second approach:
public static class GenericMapMigrationService implements MigrationService {
/*
* Example for a YAML like:
* oldStats:
* oldStr: 3
* oldWil: 8
*
* Where oldStr and oldWil should be taken over into Ref.STR and Ref.WIL entries in the new map property.
*/
@Override
public boolean checkAndMigrate(@NotNull PropertyReader reader, @NotNull ConfigurationData configurationData) {
Object oldStatsValue = reader.getObject("oldStats");
if (oldStatsValue instanceof Map<?, ?>) {
// getCurrentStatValues same impl. as in code block above
performOldStatsMigration((Map<?, ?>) oldStatsValue,
getCurrentStatValues(reader), configurationData);
return true;
}
return false;
}
private void performOldStatsMigration(Map<?, ?> oldStatsValue,
Map<String, Integer> newStatValues,
ConfigurationData configurationData) {
boolean hasChange = false;
for (Map.Entry<?, ?> entry : oldStatsValue.entrySet()) {
Object key = entry.getKey();
Object value = entry.getValue();
if (key instanceof String && value instanceof Integer) {
// Determine new path based on key, set value to newStatValues
// newStatValues.putIfAbsent(newPath, (Integer) value);
hasChange = true;
}
}
if (hasChange) {
configurationData.setValue(StatsSettingsHolder.STAT_AMOUNT, newStatValues);
}
}
}The migration is a little more difficult than you might expect at first glance. I don't have a clear picture of how you're using this property value, but if you're absolutely expecting that every Ref entry be present in the map, I'd create a dedicated property for each stat instead of putting them all in a map. That way, you can address them individually in migrations (using moveProperty on PlainMigrationService) and retrievals and you're sure that all values are present. With the map property, you're never guaranteed that every Ref will be in it, e.g. if a user deletes on of the entries in the YAML. You'd have to separately guard against this.
The Ref class is just an Enum that holds some values, in this case Ref.STR and such is just a string with the name of the stat.
In my current case it is related to #411 where the same situation of migrating from an old config system.
To get the values for the old config file i did the following:
File oldConfigFile = new File(Ref.puDir, "PUItem.yml");
YamlFileReader oldConfig = new YamlFileReader(oldConfigFile.toPath());
To use setValue using StatsSettingsHolder.STAT_AMOUNT for it to have the old config value i'd have to transform oldConfig.getObject("PUItem(itemID).statAmounts") to a map (since there is no way to get a map from the file
So the method would look like this:
configurationData.setValue(StatsSettingsHolder.STAT_AMOUNT, (transformed to map) oldConfig.getObject("PUItem(itemID).statAmounts"))
After a short search I found this article for using GSON to turn an Object into map
https://www.baeldung.com/java-convert-object-to-map#using-gson
I decided to use this and it works great but I rather not depend on another dependency
This is my usage:
File oldConfigFile = new File(Ref.puDir, "PUItem.yml");
YamlFileReader oldConfig = new YamlFileReader(oldConfigFile.toPath());
String sectionName = "PUItem" + itemID;
if(oldConfig.contains(sectionName)) {
Gson gson = new Gson();
String json = gson.toJson(oldConfig.getObject(sectionName + ".statAmounts"));
Map<String, Integer> map = gson.fromJson(json, new TypeToken<Map<String, Integer>>() {}.getType());
System.out.print("stats " + map);
configurationData.setValue(PUConfig.STAT_AMOUNT, map);
}If your old data is in a separate file, you can use the approaches I suggested in the previous comment by creating your own YamlFileReader as you've shown in your own code:
YamlFileReader oldConfig = new YamlFileReader(oldConfigFile.toPath());YamlFileReader#getObject returns whatever is under the given path, so it's a question of checking whether there is a map there (second approach) or get each old path individually and try to convert it to an integer (first approach)