fix field/method merging

unimined · Oct 25, 2024 · 27049c8 · 27049c8
1 parent 415253a
commit 27049c8
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 88 deletions.
diff --git a/src/commonMain/kotlin/xyz/wagyourtail/unimined/mapping/tree/node/LazyResolvables.kt b/src/commonMain/kotlin/xyz/wagyourtail/unimined/mapping/tree/node/LazyResolvables.kt
@@ -16,13 +16,13 @@ class LazyResolvables<T: BaseVisitor<T>, U>(val mappings: AbstractMappingTree, v
             resolved.clear()
             for (element in unresolved) {
                 var resolvedElement: U? = null
-                for (existing in resolved) {
-                    resolvedElement = element.merge(existing)
-                    if (resolvedElement != null) {
-                        if (resolvedElement != existing) {
-                            resolved.add(resolvedElement)
+                for (existing in resolved.toList()) {
+                    val res = element.merge(existing) as U?
+                    if (res != null) {
+                        if (res != existing) {
+                            resolved.add(res)
                         }
-                        break
+                        resolvedElement = res
                     }
                 }
                 if (resolvedElement == null) {

diff --git a/.../kotlin/xyz/wagyourtail/unimined/mapping/tree/node/_class/member/FieldMethodResolvable.kt b/.../kotlin/xyz/wagyourtail/unimined/mapping/tree/node/_class/member/FieldMethodResolvable.kt
@@ -59,7 +59,7 @@ abstract class FieldMethodResolvable<T: FieldMethodResolvable<T, U>, U: MemberVi
 
                     // warn if a method name is getting implicitly overridden
                     if (notMatched) {
-                        LOGGER.warn {
+                        LOGGER.info {
                             """
                                 Joining different names, second will take priority
                                 $element
@@ -77,54 +77,25 @@ abstract class FieldMethodResolvable<T: FieldMethodResolvable<T, U>, U: MemberVi
                     null
                 }
             } else {
-                // merge, but also create new unmerged one
-
-                // warn if a method name is getting implicitly overridden
-                if (notMatched) {
-                    LOGGER.warn {
-                        """
-                            Joining different names, second will take priority
-                            $element
-                            $this
-                        """.trimIndent()
-                    }
-                }
-
-                val new = create(parent as ClassNode)
-                new.setNames(names)
-                element.setNames(names)
-                element.doMerge(new)
-                doMerge(new)
-                new
+                element.setNames(names.filter { it.key !in element.names })
+                setNames(element.names.filter { it.key !in names })
+                doMerge(element)
+                // return null when desc/nodesc match is found
+                null
             }
         } else {
             if (descs.isNotEmpty()) {
-                // merge, but also create a new one with descs
-
-                // warn if a method name is getting implicitly overridden
-                if (notMatched) {
-                    LOGGER.warn {
-                        """
-                            Joining different names, second will take priority
-                            $element
-                            $this
-                        """.trimIndent()
-                    }
-                }
-
-                val new = create(parent as ClassNode)
-                new.setNames(element.names)
-                new.setNames(names)
-                new.setDescriptors(descs)
-                element.doMerge(new)
-                doMerge(new)
-                new
+                element.setNames(names.filter { it.key !in element.names })
+                setNames(element.names.filter { it.key !in names })
+                element.doMerge(this as T)
+                // return null when desc/nodesc match is found
+                null
             } else {
                 // merge
 
                 // warn if a method name is getting implicitly overridden
                 if (notMatched) {
-                    LOGGER.warn {
+                    LOGGER.info {
                         """
                             Joining different names, second will take priority
                             $element

diff --git a/src/commonMain/kotlin/xyz/wagyourtail/unimined/mapping/tree/node/_class/member/MethodNode.kt b/src/commonMain/kotlin/xyz/wagyourtail/unimined/mapping/tree/node/_class/member/MethodNode.kt
@@ -108,45 +108,35 @@ class MethodNode(parent: ClassNode) : FieldMethodResolvable<MethodNode, MethodVi
         if (merged == null && (name == "<init>" || name == "<clinit>") && name == element.names.values.first()) {
 
             // test desc
-            if (descs.isNotEmpty() && element.descs.isNotEmpty()) {
-                val descNs = descs.keys.first()
-                return if (hasDescriptor()) {
-                    if (element.hasDescriptor()) {
-                        if (getMethodDesc(descNs) == element.getMethodDesc(descNs)) {
-                            // merge
-                            element.setNames(names)
-                            doMerge(element)
-                            element
-                        } else {
-                            // dont merge
-                            null
-                        }
-                    } else {
-                        // merge but also create new unmerged one
-                        val new = create(parent as ClassNode)
-                        new.setNames(names)
-                        element.setNames(names)
-                        doMerge(element)
-                        doMerge(new)
-                        new
-                    }
-                } else {
-                    if (element.hasDescriptor()) {
-                        // merge, but also create a new one without descs
-                        val new = create(parent as ClassNode)
-                        new.setNames(element.names)
-                        new.setNames(names)
-                        new.setDescriptors(descs)
-                        element.setNames(names)
-                        doMerge(element)
-                        doMerge(new)
-                        new
-                    } else {
+            return if (element.hasDescriptor()) {
+                if (hasDescriptor()) {
+                    val descNs = descs.keys.first()
+                    if (getMethodDesc(descNs) == element.getMethodDesc(descNs)) {
                         // merge
                         element.setNames(names)
                         doMerge(element)
                         element
+                    } else {
+                        // dont merge
+                        null
                     }
+                } else {
+                    element.setNames(names.filter { it.key !in element.names })
+                    setNames(element.names.filter { it.key !in names })
+                    doMerge(element)
+                    null
+                }
+            } else {
+                if (element.hasDescriptor()) {
+                    element.setNames(names.filter { it.key !in element.names })
+                    setNames(element.names.filter { it.key !in names })
+                    element.doMerge(this)
+                    null
+                } else {
+                    // merge
+                    element.setNames(names)
+                    doMerge(element)
+                    element
                 }
             }
         }

diff --git a/src/commonTest/kotlin/xyz/wagyourtail/unimined/mapping/test/formats/umf/UMFReadWriteTest.kt b/src/commonTest/kotlin/xyz/wagyourtail/unimined/mapping/test/formats/umf/UMFReadWriteTest.kt
@@ -43,16 +43,23 @@ umf	1	0
 intermediary	named	extra
 c	net/minecraft/class_310	net/minecraft/client/MinecraftClient	_
 	f	field_1724	_	_
-	m	method_1507;()V	testMethod	_
+	f	c	field_71512_c	
+	f	c;Ljava/util/Map;	_	c;Ljava/util/Map;
+	f	c;Ljava/util/Map;	_	field_3;Ljava/util/Map;
+	m	method_1507	testMethod	_
 		p	_	0	_	_	p1
-	m	_	testMethod;()V	methodNameExtra
+	m	_	testMethod;()V	ignored
 		p	_	0	_	this	_
 		v	1	_	_	lv1	lv1Extra
+	m	_	testMethod;()V	methodNameExtra
 	m	__	dontMerge;()V	dontMerge2
-	m	<init>;(Leqm;JLeub;Ljava/lang/String;Ljava/lang/String;)V
+	m	<init>;(Leqm;JLeub;Ljava/lang/String;Ljava/lang/String;)V	_	_
 		p	0	_	p1	_	_
-	m	<init>;(Leqm;JLeub;Ljava/lang/String;Ljava/lang/String;)V	<init>	<init>
-		p	0	1	_	_	_
+	m	<init>;(Leqm;JLeub;Ljava/lang/String;Ljava/lang/String;)V	_	<init>
+		p	_	2	p2	_	_
+	m	<init>	<init>	<init>
+		p	0	1	_	_	test
+		p	1	2	_	_
         """.trimIndent().trimEnd()
 
         val mappings = Buffer().use { input ->
@@ -63,16 +70,25 @@ c	net/minecraft/class_310	net/minecraft/client/MinecraftClient	_
             mappings.accept(UMFWriter.write(output, false))
             output.readUtf8()
         }
-        val testOutput = """umf	1	0
+        val testOutput = """
+umf	1	0
 intermediary	named	extra
 c	net/minecraft/class_310	net/minecraft/client/MinecraftClient	_
 	f	field_1724	_	_
+	f	c	field_71512_c	c
+	f	c;Ljava/util/Map;	field_71512_c;Ljava/util/Map;	field_3;Ljava/util/Map;
+	m	method_1507	testMethod	ignored
+		p	_	0	_	_	p1
 	m	method_1507;()V	testMethod;()V	methodNameExtra;()V
 		p	_	0	_	this	p1
 		v	1	_	_	lv1	lv1Extra
 	m	_;()V	dontMerge;()V	dontMerge2;()V
 	m	<init>;(Leqm;JLeub;Ljava/lang/String;Ljava/lang/String;)V	<init>;(Leqm;JLeub;Ljava/lang/String;Ljava/lang/String;)V	<init>;(Leqm;JLeub;Ljava/lang/String;Ljava/lang/String;)V
-		p	0	1	p1	_	_
+		p	0	1	p1	_	test
+		p	1	2	p2	_	_
+	m	<init>	<init>	<init>
+		p	0	1	_	_	test
+		p	1	2	_	_	_
         """.trimIndent().replace(' ', '\t').trimEnd()
 
         assertEquals(testOutput, output.trimEnd())