team-abnormals / blueprint

Library that implements the framework of all Abnormals mods
https://www.curseforge.com/minecraft/mc-mods/blueprint
Other
111 stars 34 forks source link

Added a config condition system #74

Closed flowerfugue closed 3 years ago

flowerfugue commented 3 years ago

This was something i'd quickly hacked up for a few specific config classes, but i thought it would be good to generalise it. It lets you register a condition seraliser which automatically maps name IDs to ConfigValue<Boolean> fields given instances of config classes to pull from.

registerBooleanConfigCondition takes in a mod id, some config objects (meaning stuff like ACConfig.COMMON), and a boolean for whether it should convert the field/class names to snake case, and generates a bunch of allowed arguments for "type": "[modid]:config":

The reasaon I added a thing to convert it to snake case is because it was inconsistent with the rest of the json :P There might be a few edge cases with what it should count as separate words - i decided that strings of capital letters or numbers like ID or 098 should stay in the same word, but a change from a number to a capital letter or a capital letter to a number means it should split there (e.g. 0B -> 0_b, aD8 -> ad_8). However, i've never seen a config field with this type of name so maybe it isn't a problem.

I did think about trying to make it support non-boolean config values too by adding a way to parse simple number expressions or whatever, but that would get very complicated - you should probably just write a custom condition at that point. I guess you could argue that automatically made config conditions are overkill in the first place, but I think the use case of making an advancement or a crafting recipe or something similar depend on a boolean config is common enough for this to be useful.

SmellyModder commented 3 years ago

A good name for the annotation could be @ConfigKey

flowerfugue commented 3 years ago

Hmm, I agree about the annotations - it's better to allow the programmer control over what the config value serialises as to avoid conflicts and to properly abstract it to the user. Also, what exactly did you mean by this?

Ideally, there should be a way to make conditions for any config value.

If you mean you should be able to serialise non-boolean values I agree, but I'm not really sure how to implement that with annotations - you can't create object types so the closest thing would be a Class<Predicate> or something.

flowerfugue commented 3 years ago

Right, so that's finally done. There's still a few things i'm unsure of about this. Due to the 'generics hell' I mentioned, I ended up making IConfigPredicate#test just take any config value instead of one matching a type parameter, and throw an exception if it's the wrong type, the same applying for IConfigPredicateSerializer#write. I'm not sure exactly how to solve this with generics considering that the serializer map is of completely unknown type, I guess you could do some reflection stuff with getting the type but that might not count as capture of ?. Not sure.

Also, I'm not sure if this works since I don't know when IConditionSerializer#write gets called:

@Override
public void write(JsonObject json, ConfigValueCondition value) {
    json.addProperty("value", value.valueID);
    json.add("predicates", new JsonArray());
    JsonArray predicates = JSONUtils.getJsonArray(json, "predicates");
    for (IConfigPredicate predicate : value.predicates.keySet()) { //TODO might not work
        CONFIG_PREDICATE_SERIALIZERS.get(predicate.getID()).write(predicates.getAsJsonObject(), predicate);
        JSONUtils.getJsonObject(predicates.getAsJsonObject(), predicate.getID().toString()).addProperty("inverted", value.predicates.get(predicate));
    }
    if (value.inverted) json.addProperty("inverted", true);
}

Similarly, I ended up writing a pretty hardcoded write method for EqualsPredicate since addProperty only accepts string, boolean, character or number parameteres, but I feel like using add which just takes a JsonObject should allow that to be generalised. Ideally though, if a number gets deserialized, it should serialize back into a number instead of turning into a string or something. Perhaps that could be an else block, except I don't know how you create a generic JsonObject:

@Override
public void write(JsonObject json, IConfigPredicate value) {
    if (!(value instanceof EqualsPredicate)) throw new IllegalArgumentException("Incompatible predicate type");
    Object object = ((EqualsPredicate) value).value;
    if (object instanceof String) {
        json.addProperty("value", (String) object);
    } else if (object instanceof Number) {
        json.addProperty("value", (Number) object);
    } else if (object instanceof Boolean) {
        json.addProperty("value", (Boolean) object);
    } else if (object instanceof Character) {
        json.addProperty("value", (Character) object);
    } else {
        json.add("value", /*however you create a json object from an object*/);
    }
}
flowerfugue commented 3 years ago

Oh I forgot to ask, are the ConfigKey values I chose okay and do there need to be more default predicates?

flowerfugue commented 3 years ago

Documentation for the predicates will come soon. As far as I know, the only code change needed is to make the equals predicate actually work - since there doesn't seem to be any kind of generic json object getting, I think I'll just need to hardcode all the primitive/String types

SmellyModder commented 3 years ago

Even GSON hardcodes the JSON primitive types. There isn't really a great way around it.

flowerfugue commented 3 years ago

Ok, the equals predicate is finally done. I'll start on the documentation in a few days, once I've finished some schoolwork.

flowerfugue commented 3 years ago

I think I've documented everything now.

flowerfugue commented 3 years ago

I've made the requested changes now.

flowerfugue commented 3 years ago

import is gone 🦀 🦀 🦀

flowerfugue commented 3 years ago

TRUE