Additional unit tests for the patcher + many bugfixes

build.gradle

@@ -223,7 +223,7 @@ configure(project(":annotations")){
 }
 
 //compile with java 8 compatibility for everything except the annotation project
-configure(subprojects - project(":annotations")){
+configure(subprojects - project(":annotations") - project(":tests")){
     tasks.withType(JavaCompile){
         options.compilerArgs.addAll(['--release', '8'])
     }
@@ -400,6 +400,12 @@ project(":tests"){
         testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:5.7.1"
     }
 
+    tasks.withType(JavaCompile){
+        targetCompatibility = JavaVersion.VERSION_17
+        sourceCompatibility = JavaVersion.VERSION_17
+        options.compilerArgs.addAll(['--release', '17'])
+    }
+
     test{
         //fork every test so mods don't interact with each other
         forkEvery = 1

core/src/mindustry/mod/ContentParser.java

@@ -105,7 +105,7 @@ public class ContentParser{
                 if(result != null) return result;
                 throw new IllegalArgumentException("Unknown status effect: '" + data.asString() + "'");
             }
-            StatusEffect effect = new StatusEffect(currentMod.name + "-" + data.getString("name"));
+            StatusEffect effect = new StatusEffect((currentMod == null ? null : currentMod.name + "-") + data.getString("name"));
             effect.minfo.mod = currentMod;
             readFields(effect, data);
             return effect;
@@ -310,7 +310,9 @@ public class ContentParser{
             data.remove("type");
             var weapon = make(oc);
             readFields(weapon, data);
-            weapon.name = currentMod.name + "-" + weapon.name;
+            if(currentMod != null){
+                weapon.name = currentMod.name + "-" + weapon.name;
+            }
             return weapon;
         });
         put(Consume.class, (type, data) -> {
@@ -448,11 +450,15 @@ public class ContentParser{
                 case "remove" -> {
                     String[] values = child.isString() ? new String[]{child.asString()} : child.asStringArray();
                     for(String type : values){
-                        Class<?> consumeType = resolve("Consume" + Strings.capitalize(type), Consume.class);
+                        if(type.equals("all")){
-                        if(consumeType != Consume.class){
+                            block.removeConsumers(b -> true);
-                            block.removeConsumers(b -> consumeType.isAssignableFrom(b.getClass()));
                         }else{
-                            Log.warn("Unknown consumer type '@' (Class: @) in consume: remove.", type, "Consume" + Strings.capitalize(type));
+                            Class<?> consumeType = resolve("Consume" + Strings.capitalize(type), Consume.class);
+                            if(consumeType != Consume.class){
+                                block.removeConsumers(b -> consumeType.isAssignableFrom(b.getClass()));
+                            }else{
+                                Log.warn("Unknown consumer type '@' (Class: @) in consume: remove.", type, "Consume" + Strings.capitalize(type));
+                            }
                         }
                     }
                 }
@@ -552,7 +558,6 @@ public class ContentParser{
             }
 
             currentContent = unit;
-            //TODO test this!
             read(() -> {
                 //add reconstructor type
                 if(value.has("requirements")){
@@ -765,7 +770,7 @@ public class ContentParser{
 
     private <T extends Content> T find(ContentType type, String name){
         Content c = Vars.content.getByName(type, name);
-        if(c == null) c = Vars.content.getByName(type, currentMod.name + "-" + name);
+        if(c == null && currentMod != null) c = Vars.content.getByName(type, currentMod.name + "-" + name);
         if(c == null) throw new IllegalArgumentException("No " + type + " found with name '" + name + "'");
         return (T)c;
     }
@@ -941,7 +946,7 @@ public class ContentParser{
 
     private <T extends MappableContent> T locate(ContentType type, String name){
         T first = Vars.content.getByName(type, name); //try vanilla replacement
-        return first != null ? first : Vars.content.getByName(type, currentMod.name + "-" + name);
+        return first != null ? first : Vars.content.getByName(type, currentMod == null ? name : currentMod.name + "-" + name);
     }
 
     private <T extends MappableContent> T locateAny(String name){
@@ -1277,7 +1282,7 @@ public class ContentParser{
         }
 
         if(def != null){
-            Log.warn("[@] No type '" + base + "' found, defaulting to type '" + def.getSimpleName() + "'", currentContent == null ? currentMod.name : "");
+            Log.warn("[@] No type '" + base + "' found, defaulting to type '" + def.getSimpleName() + "'", currentContent == null && currentMod != null ? currentMod.name : "");
             return def;
         }
         throw new IllegalArgumentException("Type not found: " + base);

core/src/mindustry/mod/ContentPatcher.java

@@ -93,25 +93,22 @@ public class ContentPatcher{
         usedpatches.clear();
     }
 
+    void visit(Object object){
+        if(object instanceof Content c && usedpatches.add(c)){
+            after(c::afterPatch);
+        }
+    }
+
     void assign(Object object, String field, Object value, @Nullable FieldData metadata, @Nullable Object parentObject, @Nullable String parentField) throws Exception{
         if(field == null || field.isEmpty()) return;
 
-        char prefix = 0;
-
-        //fetch modifier (+ or -) and concat it to the end, turning `+array` into `array.+`
-        if(field.charAt(0) == '+'){
-            prefix = field.charAt(0);
-            field = field.substring(1);
-        }else if(field.endsWith(".+")){
-            prefix = field.charAt(field.length() - 1);
-            field = field.substring(0, field.length() - 2);
-        }
-
         //field.field2.field3 nested syntax
         if(field.indexOf('.') != -1){
             //resolve the field chain until the final field is reached
             String[] path = field.split("\\.");
             for(int i = 0; i < path.length - 1; i++){
+                parentObject = object;
+                parentField = path[i];
                 Object[] result = resolve(object, path[i], metadata);
                 if(result == null){
                     warn("Failed to resolve @.@", object, path[i]);
@@ -119,13 +116,15 @@ public class ContentPatcher{
                 }
                 object = result[0];
                 metadata = (FieldData)result[1];
+
+                if(i < path.length - 2){
+                    visit(object);
+                }
             }
             field = path[path.length - 1];
         }
 
-        if(object instanceof Content c && usedpatches.add(c)){
+        visit(object);
-            after(c::afterPatch);
-        }
 
         if(object == root){
             if(value instanceof JsonValue jval && jval.isObject()){
@@ -142,17 +141,20 @@ public class ContentPatcher{
             }
         }else if(object instanceof Seq<?> || object.getClass().isArray()){ //TODO
 
-            if(prefix == '+'){
+            if(field.equals("+")){
+                var meta = new FieldData(metadata.elementType, null, null);
                 //handle array addition syntax
                 if(object instanceof Seq s){
                     modifiedField(parentObject, parentField, s.copy());
 
-                    assignValue(object, field, metadata, () -> null, val -> s.add(val), value, false);
+                    assignValue(object, field, meta, () -> null, s::add, value, false);
                 }else{
                     modifiedField(parentObject, parentField, copyArray(object));
 
                     var fobj = object;
-                    assignValue(parentObject, parentField, metadata, () -> null, val -> {
+                    var fpo = parentObject;
+                    var fpf = parentField;
+                    assignValue(parentObject, parentField, meta, () -> null, val -> {
                         try{
                             //create copy array, put the new object in the last slot, and assign the parent's field to it
                             int len = Array.getLength(fobj);
@@ -160,7 +162,7 @@ public class ContentPatcher{
                             Array.set(copy, len - 1, val);
                             System.arraycopy(fobj, 0, copy, 0, len);
 
-                            assign(parentObject, parentField, copy, null, null, null);
+                            assign(fpo, fpf, copy, null, null, null);
                         }catch(Exception e){
                             throw new RuntimeException(e);
                         }
@@ -190,7 +192,7 @@ public class ContentPatcher{
                     assignValue(object, field, metadata, () -> Array.get(fobj, i), val -> Array.set(fobj, i, val), value, false);
                 }
             }
-        }else if(object instanceof ObjectSet set && prefix == '+'){
+        }else if(object instanceof ObjectSet set && field.equals("+")){
             modifiedField(parentObject, parentField, set.copy());
 
             assignValue(object, field, metadata, () -> null, val -> set.add(val), value, false);
@@ -269,7 +271,7 @@ public class ContentPatcher{
                 try{
                     setter.get(json.readValue(metadata.type, metadata.elementType, jsv));
                 }catch(Throwable e){
-                    warn("Failed to read value @.@ = @: @ (type = @ elementType = @)", object, field, value, e.getMessage(), metadata.type, metadata.elementType);
+                    warn("Failed to read value @.@ = @: @ (type = @ elementType = @)\n@", object, field, value, e.getMessage(), metadata.type, metadata.elementType, Strings.getStackTrace(e));
                 }
             }else{
                 //assign each field manually

core/src/mindustry/type/UnitType.java

@@ -1228,6 +1228,12 @@ public class UnitType extends UnlockableContent implements Senseable{
         }
     }
 
+    @Override
+    public void afterPatch(){
+        super.afterPatch();
+        totalRequirements = cachedRequirements = firstRequirements = null;
+    }
+
     /** @return the time required to build this unit, as a value that takes into account reconstructors */
     public float getBuildTime(){
         getTotalRequirements();

core/src/mindustry/world/blocks/power/ConsumeGenerator.java

@@ -70,6 +70,15 @@ public class ConsumeGenerator extends PowerGenerator{
         super.init();
     }
 
+    @Override
+    public void afterPatch(){
+        super.afterPatch();
+
+        filterItem = findConsumer(c -> c instanceof ConsumeItemFilter);
+        filterLiquid = findConsumer(c -> c instanceof ConsumeLiquidFilter);
+        if(filterItem instanceof ConsumeItemEfficiency eff) eff.itemDurationMultipliers = itemDurationMultipliers;
+    }
+
     @Override
     public void setStats(){
         stats.timePeriod = itemDuration;

core/src/mindustry/world/blocks/units/Reconstructor.java

@@ -119,8 +119,19 @@ public class Reconstructor extends UnitBlock{
 
     @Override
     public void init(){
-        capacities = new int[Vars.content.items().size];
+        initCapacities();
+        super.init();
+    }
 
+    @Override
+    public void afterPatch(){
+        initCapacities();
+        super.afterPatch();
+    }
+
+    public void initCapacities(){
+        capacities = new int[Vars.content.items().size];
+        itemCapacity = 10;
         ConsumeItems cons = findConsumer(c -> c instanceof ConsumeItems);
         if(cons != null){
             for(ItemStack stack : cons.items){
@@ -130,8 +141,6 @@ public class Reconstructor extends UnitBlock{
         }
 
         consumeBuilder.each(c -> c.multiplier = b -> state.rules.unitCost(b.team));
-
-        super.init();
     }
 
     public void addUpgrade(UnitType from, UnitType to){

core/src/mindustry/world/blocks/units/UnitFactory.java

@@ -80,7 +80,19 @@ public class UnitFactory extends UnitBlock{
 
     @Override
     public void init(){
+        initCapacities();
+        super.init();
+    }
+
+    @Override
+    public void afterPatch(){
+        initCapacities();
+        super.afterPatch();
+    }
+
+    public void initCapacities(){
         capacities = new int[Vars.content.items().size];
+        itemCapacity = 10; //unit factories can't control their own capacity externally, setting the value does nothing
         for(UnitPlan plan : plans){
             for(ItemStack stack : plan.requirements){
                 capacities[stack.item.id] = Math.max(capacities[stack.item.id], stack.amount * 2);
@@ -89,8 +101,6 @@ public class UnitFactory extends UnitBlock{
         }
 
         consumeBuilder.each(c -> c.multiplier = b -> state.rules.unitCost(b.team));
-
-        super.init();
     }
 
     @Override

core/src/mindustry/world/consumers/ConsumeLiquid.java

@@ -22,7 +22,6 @@ public class ConsumeLiquid extends ConsumeLiquidBase{
         this(null, 0f);
     }
 
-
     @Override
     public void apply(Block block){
         super.apply(block);

tests/src/test/java/PatcherTests.java

@@ -0,0 +1,93 @@
+import arc.struct.*;
+import mindustry.*;
+import mindustry.content.*;
+import mindustry.entities.bullet.*;
+import mindustry.type.*;
+import mindustry.world.blocks.units.*;
+import mindustry.world.meta.*;
+import org.junit.jupiter.api.*;
+import org.junit.jupiter.params.*;
+import org.junit.jupiter.params.provider.*;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+public class PatcherTests{
+
+    @BeforeAll
+    static void init(){
+        ApplicationTests.launchApplication(false);
+    }
+
+    @AfterEach
+    void resetAfter(){
+        Vars.logic.reset();
+    }
+
+    @BeforeEach
+    void resetBefore(){
+        Vars.logic.reset();
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"""
+    block.ground-factory.plans.+: {
+        unit: flare
+        requirements: [surge-alloy/10]
+        time: 100
+    }
+    """,
+    """
+    block: {
+        ground-factory: {
+            plans.+: {
+                unit: flare
+                requirements: [surge-alloy/10]
+                time: 100
+            }
+        }
+    }
+    """
+    })
+    void unitFactoryPlans(String value) throws Exception{
+        Vars.state.patcher.apply(Seq.with(value));
+
+        var plan = ((UnitFactory)Blocks.groundFactory).plans.find(u -> u.unit == UnitTypes.flare);
+        assertNotNull(plan, "A plan for flares must have been added.");
+        assertEquals(UnitTypes.flare, plan.unit);
+        assertArrayEquals(new ItemStack[]{new ItemStack(Items.surgeAlloy, 10)}, plan.requirements);
+        assertEquals(100f, plan.time);
+
+        Vars.state.patcher.unapply();
+
+        plan = ((UnitFactory)Blocks.groundFactory).plans.find(u -> u.unit == UnitTypes.flare);
+
+        assertNull(plan);
+    }
+
+    @Test
+    void testUnitWeapons() throws Exception{
+        UnitTypes.dagger.checkStats();
+        UnitTypes.dagger.stats.add(Stat.charge, 999);
+        assertNotNull(UnitTypes.dagger.stats.toMap().get(StatCat.general).get(Stat.charge));
+
+        Vars.state.patcher.apply(Seq.with("""
+        unit.dagger.weapons.+: {
+            name: navanax-weapon
+            bullet: {
+                type: LightningBulletType
+                lightningLength: 999
+            }
+        }
+        """));
+
+        assertEquals(3, UnitTypes.dagger.weapons.size);
+        assertEquals("navanax-weapon", UnitTypes.dagger.weapons.get(2).name);
+        assertEquals(LightningBulletType.class, UnitTypes.dagger.weapons.get(2).bullet.getClass());
+        assertEquals(999, UnitTypes.dagger.weapons.get(2).bullet.lightningLength);
+
+        Vars.logic.reset();
+
+        UnitTypes.dagger.checkStats();
+        assertNull(UnitTypes.dagger.stats.toMap().get(StatCat.general).get(Stat.charge));
+    }
+}