From f742aba80d3ef70aecd8eda30a506cd10ecbd46c Mon Sep 17 00:00:00 2001
From: Graham <gpe@openrs2.dev>
Date: Sun, 25 Aug 2019 09:52:56 +0100
Subject: [PATCH] Replace simple if condition swapping heuristic with a smarter
 one

This allows us to change expressions like `!a && !b` to `a || b`. It
isn't perfect, as we will now flip bitwise flag checks (which was one of
the main motivations for the old heuristic).
---
 .../deob/ast/transform/IfElseTransformer.java    |  8 ++++----
 .../dev/openrs2/deob/ast/util/ExprUtils.java     | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/deob-ast/src/main/java/dev/openrs2/deob/ast/transform/IfElseTransformer.java b/deob-ast/src/main/java/dev/openrs2/deob/ast/transform/IfElseTransformer.java
index 4c673402..6a02d1bb 100644
--- a/deob-ast/src/main/java/dev/openrs2/deob/ast/transform/IfElseTransformer.java
+++ b/deob-ast/src/main/java/dev/openrs2/deob/ast/transform/IfElseTransformer.java
@@ -54,11 +54,11 @@ public final class IfElseTransformer extends Transformer {
 				}
 
 				/*
-				 * Prefer if (a) over if (!a). We don't swap != as it makes
-				 * checking bitwise flags look worse.
+				 * Prefer fewer NOTs in the if condition.
 				 */
-				if (ExprUtils.isNot(condition)) {
-					stmt.setCondition(ExprUtils.not(condition));
+				var notCondition = ExprUtils.not(condition);
+				if (ExprUtils.countNots(notCondition) < ExprUtils.countNots(condition)) {
+					stmt.setCondition(notCondition);
 					stmt.setThenStmt(elseStmt.clone());
 					stmt.setElseStmt(thenStmt.clone());
 				}
diff --git a/deob-ast/src/main/java/dev/openrs2/deob/ast/util/ExprUtils.java b/deob-ast/src/main/java/dev/openrs2/deob/ast/util/ExprUtils.java
index c346bde5..10650924 100644
--- a/deob-ast/src/main/java/dev/openrs2/deob/ast/util/ExprUtils.java
+++ b/deob-ast/src/main/java/dev/openrs2/deob/ast/util/ExprUtils.java
@@ -71,6 +71,22 @@ public final class ExprUtils {
 		return new UnaryExpr(expr.clone(), UnaryExpr.Operator.LOGICAL_COMPLEMENT);
 	}
 
+	public static int countNots(Expression expr) {
+		int count = 0;
+
+		if (expr.isUnaryExpr() && expr.asUnaryExpr().getOperator() == UnaryExpr.Operator.LOGICAL_COMPLEMENT) {
+			count++;
+		} else if (expr.isBinaryExpr() && expr.asBinaryExpr().getOperator() == BinaryExpr.Operator.NOT_EQUALS) {
+			count++;
+		}
+
+		for (Expression child : expr.getChildNodesByType(Expression.class)) {
+			count += countNots(child);
+		}
+
+		return count;
+	}
+
 	public static boolean hasSideEffects(Expression expr) {
 		if (expr.isLiteralExpr() || expr.isNameExpr() | expr.isFieldAccessExpr()) {
 			return false;