Skip to content

Commit 52fe265

Browse files
committed
Add forge entity attributes lazily to the builder
Requires less ASM hacks
1 parent 200a19a commit 52fe265

File tree

10 files changed

+223
-240
lines changed

10 files changed

+223
-240
lines changed

src/main/java/dev/su5ed/sinytra/connector/transformer/AccessWidenerTransformer.java

+19-7
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public class RemappingAccessWidenerVisitor implements AccessWidenerVisitor {
4545
private final String sourceNamespace;
4646
private final StringBuilder builder = new StringBuilder();
4747

48+
private final Map<String, AccessWidenerReader.AccessType> classAccess = new HashMap<>();
4849
private final Map<String, Map<String, AccessWidenerReader.AccessType>> classFields = new HashMap<>();
4950

5051
public RemappingAccessWidenerVisitor(String sourceNamespace) {
@@ -54,6 +55,17 @@ public RemappingAccessWidenerVisitor(String sourceNamespace) {
5455
}
5556

5657
public void finish() {
58+
// Translate class AWs
59+
this.classAccess.forEach((name, access) -> {
60+
String modifier = switch (access) {
61+
case ACCESSIBLE -> "public";
62+
case EXTENDABLE -> "public-f";
63+
default -> throw new IllegalArgumentException("Invalid access type " + access + " for class");
64+
};
65+
String mappedName = AccessWidenerTransformer.this.resolver.mapClassName(this.sourceNamespace, name).replace('/', '.');
66+
this.builder.append(modifier).append(" ").append(mappedName).append("\n");
67+
});
68+
// Translate field AWs
5769
this.classFields.forEach((owner, fields) -> fields.forEach((name, access) -> {
5870
String modifier = switch (access) {
5971
case ACCESSIBLE -> "public";
@@ -72,13 +84,11 @@ public void finish() {
7284

7385
@Override
7486
public void visitClass(String name, AccessWidenerReader.AccessType access, boolean transitive) {
75-
String modifier = switch (access) {
76-
case ACCESSIBLE -> "public";
77-
case EXTENDABLE -> "public-f";
78-
default -> throw new IllegalArgumentException("Invalid access type " + access + " for class");
79-
};
80-
String mappedName = AccessWidenerTransformer.this.resolver.mapClassName(this.sourceNamespace, name).replace('/', '.');
81-
this.builder.append(modifier).append(" ").append(mappedName).append("\n");
87+
// AcessWidener silently also access widens owners of methods that are being AW'd, but never calls visitClass for those entries
88+
// Therefore, we have to replicate this behavior ourselves in visitMethod below. In addition, we first gather all class AWs in a map
89+
// and only translate them once all AW entries have been process. This prevents conflicts in case visitMethod generates an AW entry
90+
// for a class that already has one, except with lower access.
91+
this.classAccess.compute(name, (value, existing) -> existing == null || access.ordinal() > existing.ordinal() ? access : existing);
8292
}
8393

8494
@Override
@@ -101,6 +111,8 @@ public void visitMethod(String owner, String name, String descriptor, AccessWide
101111
.append(mappedDescriptor)
102112
.append(!name.equals(mappedName) ? " # " + mappedName : "")
103113
.append("\n");
114+
// Make parent class accessible / extensible if necessary
115+
visitClass(owner, access, false);
104116
}
105117

106118
@Override

src/main/java/dev/su5ed/sinytra/connector/transformer/MixinPatchTransformer.java

+2-18
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
package dev.su5ed.sinytra.connector.transformer;
22

3+
import dev.su5ed.sinytra.connector.transformer.patch.ClassResourcesTransformer;
34
import dev.su5ed.sinytra.connector.transformer.patch.ClassTransform;
4-
import dev.su5ed.sinytra.connector.transformer.patch.MethodQualifier;
5-
import dev.su5ed.sinytra.connector.transformer.patch.ParameterToSupplierPatch;
65
import dev.su5ed.sinytra.connector.transformer.patch.Patch;
76
import net.minecraftforge.fart.api.Transformer;
87
import org.objectweb.asm.ClassReader;
@@ -169,22 +168,7 @@ public class MixinPatchTransformer implements Transformer {
169168
.build()
170169
);
171170
private static final List<ClassTransform> CLASS_TRANSFORMS = List.of(
172-
new ParameterToSupplierPatch()
173-
// Calling FabricDefaultAttributeRegistry.register usually involves passing in the result of LivingEntity.createLivingAttributes.
174-
// Forge modifies the createLivingAttributes method to include attributes which need to be registered before they can be queried (RegistryObject).
175-
// This means calling it at the time fabric mods do will fail. We replace the registration method with our own, which takes in the existing value as a supplier.
176-
// The attributes are now registered by us, just before FabricDefaultAttributeRegistry processes them.
177-
// @see dev.su5ed.sinytra.connector.mod.compat.LateEntityAttributeRegistry
178-
.add(
179-
new MethodQualifier("net/fabricmc/fabric/api/object/builder/v1/entity/FabricDefaultAttributeRegistry", "register", "(Lnet/minecraft/world/entity/EntityType;Lnet/minecraft/world/entity/ai/attributes/AttributeSupplier;)V"),
180-
new MethodQualifier("dev/su5ed/sinytra/connector/mod/compat/LateEntityAttributeRegistry", "register", "(Lnet/minecraft/world/entity/EntityType;Ljava/util/function/Supplier;)V"),
181-
Type.getObjectType("net/minecraft/world/entity/ai/attributes/AttributeSupplier")
182-
)
183-
.add(
184-
new MethodQualifier("net/fabricmc/fabric/api/object/builder/v1/entity/FabricDefaultAttributeRegistry", "register", "(Lnet/minecraft/world/entity/EntityType;Lnet/minecraft/world/entity/ai/attributes/AttributeSupplier$Builder;)V"),
185-
new MethodQualifier("dev/su5ed/sinytra/connector/mod/compat/LateEntityAttributeRegistry", "registerBuilder", "(Lnet/minecraft/world/entity/EntityType;Ljava/util/function/Supplier;)V"),
186-
Type.getObjectType("net/minecraft/world/entity/ai/attributes/AttributeSupplier$Builder")
187-
)
171+
new ClassResourcesTransformer()
188172
);
189173

190174
private final Set<String> mixins;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package dev.su5ed.sinytra.connector.transformer.patch;
2+
3+
import org.objectweb.asm.Opcodes;
4+
import org.objectweb.asm.tree.AbstractInsnNode;
5+
import org.objectweb.asm.tree.ClassNode;
6+
import org.objectweb.asm.tree.MethodInsnNode;
7+
import org.objectweb.asm.tree.MethodNode;
8+
import org.objectweb.asm.tree.analysis.Analyzer;
9+
import org.objectweb.asm.tree.analysis.AnalyzerException;
10+
import org.objectweb.asm.tree.analysis.SourceInterpreter;
11+
import org.objectweb.asm.tree.analysis.SourceValue;
12+
13+
import java.util.ArrayList;
14+
import java.util.Collection;
15+
import java.util.HashSet;
16+
import java.util.List;
17+
18+
public class ClassResourcesTransformer implements ClassTransform {
19+
20+
record Replacement(MethodInsnNode methodInsn, AbstractInsnNode paramInsn) {}
21+
22+
@Override
23+
public Result apply(ClassNode classNode) {
24+
boolean applied = false;
25+
for (MethodNode method : classNode.methods) {
26+
List<Replacement> replacements = new ArrayList<>();
27+
SourceInterpreter i = new ScanningSourceInterpreter(Opcodes.ASM9, replacements);
28+
Analyzer<SourceValue> analyzer = new Analyzer<>(i);
29+
30+
try {
31+
analyzer.analyze(method.name, method);
32+
} catch (AnalyzerException e) {
33+
throw new RuntimeException(e);
34+
}
35+
36+
for (Replacement replacement : replacements) {
37+
method.instructions.insert(replacement.paramInsn, new MethodInsnNode(Opcodes.INVOKEVIRTUAL, "java/lang/Class", "getClassLoader", "()Ljava/lang/ClassLoader;"));
38+
replacement.methodInsn.owner = "java/lang/ClassLoader";
39+
applied = true;
40+
}
41+
}
42+
return new Result(applied, false);
43+
}
44+
45+
private static class ScanningSourceInterpreter extends SourceInterpreter {
46+
private final List<Replacement> replacements;
47+
private final Collection<MethodInsnNode> seen = new HashSet<>();
48+
49+
public ScanningSourceInterpreter(int api, List<Replacement> replacements) {
50+
super(api);
51+
this.replacements = replacements;
52+
}
53+
54+
@Override
55+
public SourceValue naryOperation(AbstractInsnNode insn, List<? extends SourceValue> values) {
56+
if (insn instanceof MethodInsnNode methodInsn && !this.seen.contains(methodInsn)
57+
&& methodInsn.owner.equals("java/lang/Class")
58+
&& methodInsn.name.equals("getResourceAsStream")
59+
&& methodInsn.desc.equals("(Ljava/lang/String;)Ljava/io/InputStream;")
60+
) {
61+
SourceValue value = values.get(0);
62+
if (value.insns.size() == 1) {
63+
AbstractInsnNode sourceInsn = value.insns.iterator().next();
64+
this.replacements.add(new Replacement(methodInsn, sourceInsn));
65+
this.seen.add(methodInsn);
66+
}
67+
else {
68+
throw new IllegalStateException("Got multiple source value insns: " + value.insns);
69+
}
70+
}
71+
return super.naryOperation(insn, values);
72+
}
73+
}
74+
}

src/main/java/dev/su5ed/sinytra/connector/transformer/patch/ParameterToSupplierPatch.java

-174
This file was deleted.

src/mod/java/dev/su5ed/sinytra/connector/mod/ConnectorMod.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import dev.su5ed.sinytra.connector.loader.ConnectorExceptionHandler;
44
import dev.su5ed.sinytra.connector.mod.compat.FluidHandlerCompat;
5-
import dev.su5ed.sinytra.connector.mod.compat.LateEntityAttributeRegistry;
5+
import dev.su5ed.sinytra.connector.mod.compat.LazyEntityAttributes;
66
import net.minecraftforge.eventbus.api.EventPriority;
77
import net.minecraftforge.eventbus.api.IEventBus;
88
import net.minecraftforge.fml.ModList;
@@ -26,7 +26,7 @@ public ConnectorMod() {
2626
}
2727

2828
if (modList.isLoaded("fabric_object_builder_api_v1")) {
29-
bus.addListener(EventPriority.HIGHEST, LateEntityAttributeRegistry::onEntityAttributesCreate);
29+
bus.addListener(EventPriority.HIGHEST, LazyEntityAttributes::addMissingAttributes);
3030
}
3131
}
3232
}

0 commit comments

Comments
 (0)