MinecraftForge / ForgeFlower

Forge's modifications to FernFlower. Fixing various bugs/inconsistencies. Main Repo: https://github.com/MinecraftForge/FernFlower
Apache License 2.0
80 stars 44 forks source link

Fix eliminated loops removing successor #101

Closed jaskarth closed 1 year ago

jaskarth commented 3 years ago

This PR fixes eliminated loops removing their successors when they shouldn't, causing the method to fail to decompile as IfHelper expects the if statement to contain edges to it's successors. This fixes the underlying issue caused by #100 and allows the loop to be properly promoted to a do-while loop.

Diff:

diff -r -u3 -N a/net/minecraft/world/level/levelgen/SimpleRandomSource.java b/net/minecraft/world/level/levelgen/SimpleRandomSource.java
--- a/net/minecraft/world/level/levelgen/SimpleRandomSource.java    2021-07-11 15:32:58.000000000 -0400
+++ b/net/minecraft/world/level/levelgen/SimpleRandomSource.java    2021-07-11 15:38:16.000000000 -0400
@@ -85,19 +85,19 @@
          this.f_158888_ = false;
          return this.f_158887_;
       } else {
-         while(true) {
-            double d0 = 2.0D * this.nextDouble() - 1.0D;
-            double d1 = 2.0D * this.nextDouble() - 1.0D;
-            double d2 = Mth.m_144952_(d0) + Mth.m_144952_(d1);
-            if (!(d2 >= 1.0D)) {
-               if (d2 != 0.0D) {
-                  double d3 = Math.sqrt(-2.0D * Math.log(d2) / d2);
-                  this.f_158887_ = d1 * d3;
-                  this.f_158888_ = true;
-                  return d0 * d3;
-               }
-            }
-         }
+         double d0;
+         double d1;
+         double d2;
+         do {
+            d0 = 2.0D * this.nextDouble() - 1.0D;
+            d1 = 2.0D * this.nextDouble() - 1.0D;
+            d2 = Mth.m_144952_(d0) + Mth.m_144952_(d1);
+         } while(d2 >= 1.0D || d2 == 0.0D);
+
+         double d3 = Math.sqrt(-2.0D * Math.log(d2) / d2);
+         this.f_158887_ = d1 * d3;
+         this.f_158888_ = true;
+         return d0 * d3;
       }
    }
 }
zml2008 commented 2 years ago

Thanks for this PR! It's a nice little change that reduces patch size :)

Once the rebase PR is merged, would you be able to rebase this PR and provide a new diff on 1.19-pre2 (or whatever the latest -pre is at that time)?

jaskarth commented 2 years ago

Yep, sounds good! This PR needs rework anyway as there are cases where the break edges need to be removed, I'll apply that change and provide a new diff after rebasing.

jaskarth commented 1 year ago

PR has been updated, with an improved patch- with a bit more testing, some code shapes were found to rely on the last break edge being removed, so the fix was modified from commenting out the branch to ensuring the edge is a break edge first. 1.19.2 diff:

diff -r -u3 -N a/net/minecraft/world/level/levelgen/MarsagliaPolarGaussian.java b/net/minecraft/world/level/levelgen/MarsagliaPolarGaussian.java
--- a/net/minecraft/world/level/levelgen/MarsagliaPolarGaussian.java    1970-01-11 17:58:04.000000000 -0500
+++ b/net/minecraft/world/level/levelgen/MarsagliaPolarGaussian.java    1970-01-11 17:58:04.000000000 -0500
@@ -21,19 +21,19 @@
          this.f_188599_ = false;
          return this.f_188598_;
       } else {
-         while(true) {
-            double d0 = 2.0D * this.f_188597_.m_188500_() - 1.0D;
-            double d1 = 2.0D * this.f_188597_.m_188500_() - 1.0D;
-            double d2 = Mth.m_144952_(d0) + Mth.m_144952_(d1);
-            if (!(d2 >= 1.0D)) {
-               if (d2 != 0.0D) {
-                  double d3 = Math.sqrt(-2.0D * Math.log(d2) / d2);
-                  this.f_188598_ = d1 * d3;
-                  this.f_188599_ = true;
-                  return d0 * d3;
-               }
-            }
-         }
+         double d0;
+         double d1;
+         double d2;
+         do {
+            d0 = 2.0D * this.f_188597_.m_188500_() - 1.0D;
+            d1 = 2.0D * this.f_188597_.m_188500_() - 1.0D;
+            d2 = Mth.m_144952_(d0) + Mth.m_144952_(d1);
+         } while(d2 >= 1.0D || d2 == 0.0D);
+
+         double d3 = Math.sqrt(-2.0D * Math.log(d2) / d2);
+         this.f_188598_ = d1 * d3;
+         this.f_188599_ = true;
+         return d0 * d3;
       }
    }
 }