Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GBA Cheats: Nested/sequential multi-line Action Replay v3 conditionals not handled correctly #889

Closed
Prof9 opened this issue Sep 28, 2017 · 18 comments

Comments

@Prof9
Copy link
Contributor

Prof9 commented Sep 28, 2017

I'm getting some strange behavior using multi-line conditionals with (Pro) Action Replay v3 codes in mGBA, using 0.7-4855-3914da27. I have the following code using nested multi-line conditionals, which is being added as a single code set.

9020A7E0 00000000	//	if 8-bit [0200A7E0] != 00, then until end:
0A202136 0000252A	//	|   if 16-bit [02002136] == 252A, then 1 line:
00202137 00000022	//	|   \   [02002137] <- 22
8820A7E0 00000004	//	|   if 8-bit [0200A7E0] == 04, then until end:
48202134 00000082	//	|   |   if 8-bit [02002134] == 82, then 2 lines:
0A202136 0000222A	//	|   |   |   if 16-bit [02002136] == 222A, then 1 line:
00202137 00000025	//	\   \   \   \   [02002137] <- 25
00000000 40000000	//	end

Or, as C:

9020A7E0 00000000	//	if (*((u8*)0x200A7E0) != 0x00) {
0A202136 0000252A	//	    if (*((u16*)0x2002136) == 0x252A) {
00202137 00000022	//	        *((u8*)0x2002137 = 0x22;
			//	    }
8820A7E0 00000004	//	    if (*((u8*)0x200A7E0) == 0x04) {
48202134 00000082	//	        if (*((u8*)0x2002134) == 0x82) {
0A202136 0000222A	//	            if (*((u16*)0x2002136) == 0x252A) {
00202137 00000025	//	                *((u8*)0x2002137 = 0x25;
			//	            }
			//	        }
00000000 40000000	//	    }
			//	}

Clearly this code should not do anything if *((u8*)0x200A7E0) == 0x00. However, mGBA is writing 0x25 to 0x2002137 even when this is the case. Furthermore, the value at 0x2002134 doesn't even seem to affect this.

Unfortunately I don't have an actual ARv3, so I can't verify on real hardware. And No$gba's ARv3 implementation happens to be broken, so I can't compare with that. According to GBATEK, multi-line conditional codes, if true, execute all following codes until ELSE or ENDIF, so if the conditional is false, then it should skip everything until ELSE or ENDIF.

I tried getting rid of the multi-line nesting by adding an extra ENDIF, as shown below. This should not be necessary since 8-bit [0200A7E0] == 04 is stronger than 8-bit [0200A7E0] != 00. Still entering this as a single code set.

9020A7E0 00000000	//	if 8-bit [0200A7E0] != 00, then until end:
0A202136 0000252A	//	|   if 16-bit [02002136] == 252A, then 1 line:
00202137 00000022	//	\   \   [02002137] <- 22
00000000 40000000	//	end
8820A7E0 00000004	//	if 8-bit [0200A7E0] == 04, then until end:
48202134 00000082	//	|   if 8-bit [02002134] == 82, then 2 lines:
0A202136 0000222A	//	|   |   if 16-bit [02002136] == 222A, then 1 line:
00202137 00000025	//	\   \   \   [02002137] <- 25
00000000 40000000	//	end

Unfortunately this still doesn't work. I've tried adding ELSE lines (00000000 60000000) before every END as well.

However, I've found that separating the code into two sets DOES produce the correct behavior.

9020A7E0 00000000	//	if 8-bit [0200A7E0] != 00, then until end:
0A202136 0000252A	//	|   if 16-bit [02002136] == 252A, then 1 line:
00202137 00000022	//	\   \   [02002137] <- 22
00000000 40000000	//	end
8820A7E0 00000004	//	if 8-bit [0200A7E0] == 04, then until end:
48202134 00000082	//	|   if 8-bit [02002134] == 82, then 2 lines:
0A202136 0000222A	//	|   |   if 16-bit [02002136] == 222A, then 1 line:
00202137 00000025	//	\   \   \   [02002137] <- 25
00000000 40000000	//	end

Assuming mGBA considers the end of a code set as the end of the code list, this seems to indicate something is going wrong with parsing the END line?

@endrift
Copy link
Member

endrift commented Sep 28, 2017

https://gamehacking.org/faqs/hackv500c.html#AR_V3_Code_Types implies that you cannot nest if statements.

@endrift
Copy link
Member

endrift commented Sep 28, 2017

Looks like my raw code detection was quite broken. I've pushed some fixes, though nested ifs will still be somewhat broken.

@Prof9
Copy link
Contributor Author

Prof9 commented Sep 28, 2017

https://gamehacking.org/faqs/hackv500c.html#AR_V3_Code_Types implies that you cannot nest if statements.

What implies that you cannot nest if statements? Every if statement that is executed should just update the execution status, until either 1/2 lines have been skipped or the END code is encountered.

True, you can't have something like:

IF multiline 1
IF multiline 2
... do stuff ...
ENDIF 2
... do more stuff ...
ENDIF 1

But if the terminators of every THEN block are grouped together, I don't see why it wouldn't work. And the 1/2-line IF codes should be separate from that. Action Replay codes don't really have IF statements in the traditional sense.

(For what it's worth, Action Replay DS codes do let you nest IFs as long as they all terminate at the same time.)

Looks like my raw code detection was quite broken. I've pushed some fixes, though nested ifs will still be somewhat broken.

I guess I should have mentioned, I was actually entering them as encrypted codes. I used arcrypt to encrypt them.

@endrift
Copy link
Member

endrift commented Sep 28, 2017

Yeah, I can believe that the 1/2 line ones would work on hardware but not in mGBA, due to how conditionals are implemented. Hm.

@endrift
Copy link
Member

endrift commented Sep 28, 2017

I don't know if you have a way to compile mGBA, but I don't have a great way to test this; can you confirm if this patch works?

diff --git a/src/core/cheats.c b/src/core/cheats.c
index 5085a3d67..8f4fec31f 100644
--- a/src/core/cheats.c
+++ b/src/core/cheats.c
@@ -256,35 +256,23 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 	if (!cheats->enabled) {
 		return;
 	}
-	bool condition = true;
-	int conditionRemaining = 0;
-	int negativeConditionRemaining = 0;
 	cheats->refresh(cheats, device);
 
+	int untilElse = 0;
+	int elseBlock = 0;
 	size_t nCodes = mCheatListSize(&cheats->list);
 	size_t i;
 	for (i = 0; i < nCodes; ++i) {
-		if (conditionRemaining > 0) {
-			--conditionRemaining;
-			if (!condition) {
-				continue;
-			}
-		} else if (negativeConditionRemaining > 0) {
-			conditionRemaining = negativeConditionRemaining - 1;
-			negativeConditionRemaining = 0;
-			condition = !condition;
-			if (!condition) {
-				continue;
-			}
-		} else {
-			condition = true;
-		}
 		struct mCheat* cheat = mCheatListGetPointer(&cheats->list, i);
 		int32_t value = 0;
 		int32_t operand = cheat->operand;
 		uint32_t operationsRemaining = cheat->repeat;
 		uint32_t address = cheat->address;
 		bool performAssignment = false;
+		bool condition = true;
+		int conditionRemaining = 0;
+		int negativeConditionRemaining = 0;
+
 		for (; operationsRemaining; --operationsRemaining) {
 			switch (cheat->type) {
 			case CHEAT_ASSIGN:
@@ -312,46 +300,55 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 				condition = _readMem(device->p, address, cheat->width) == operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_NE:
 				condition = _readMem(device->p, address, cheat->width) != operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_LT:
 				condition = _readMem(device->p, address, cheat->width) < operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_GT:
 				condition = _readMem(device->p, address, cheat->width) > operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_ULT:
 				condition = (uint32_t) _readMem(device->p, address, cheat->width) < (uint32_t) operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_UGT:
 				condition = (uint32_t) _readMem(device->p, address, cheat->width) > (uint32_t) operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_AND:
 				condition = _readMem(device->p, address, cheat->width) & operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_LAND:
 				condition = _readMem(device->p, address, cheat->width) && operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_NAND:
 				condition = !(_readMem(device->p, address, cheat->width) & operand);
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			}
 
@@ -362,6 +359,18 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 			address += cheat->addressOffset;
 			operand += cheat->operandOffset;
 		}
+		if (untilElse) {
+			--untilElse;
+			if (untilElse == 0) {
+				i += elseBlock - 1;
+			}
+		}
+		if (conditionRemaining > 0 && !condition) {
+			i += conditionRemaining - 1;
+		} else if (negativeConditionRemaining > 0) {
+			untilElse = conditionRemaining;
+			elseBlock = negativeConditionRemaining;
+		}
 	}
 }
 

@Prof9
Copy link
Contributor Author

Prof9 commented Sep 28, 2017

I've managed to apply the patch and build it on Windows, here are my findings. All codes inserted as encrypted and as a single code set.

Unfortunately this patch just seems to break a bunch of cases.

Simple 1-line:

08200000 00000001	//	if 8-bit [02000000] == 01, then 1 line:
00200008 00000088	//	\    [02000008] <- 88

Desired behavior:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88.

Behavior after patch:

  • If [02000000] == 00, writes 88.
  • If [02000000] == 01, writes 88.

Simple 2-line:

48200000 00000001	//	if 8-bit [02000000] == 01, then 2 lines:
00200008 00000088	//	|    [02000008] <- 88
00200009 00000099	//	\    [02000009] <- 99

Desired behavior:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

Behavior after patch:

  • If [02000000] == 00, writes 99.
  • If [02000000] == 01, writes 88 and 99.

Simple multi-line:

88200000 00000001	//	if 8-bit [02000000] == 01, then until end:
00200008 00000088	//	|   [02000008] <- 88
00200009 00000099	//	\   [02000009] <- 99
00000000 40000000	//	end

Desired behavior:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

Behavior after patch:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

1-line inside multi-line:

88200000 00000001	//	if 8-bit [02000000] == 01, then until end:
08200000 00000001	//	|   if 8-bit [02000000] == 01, then 1 line:
00200008 00000088	//	\   \   [02000008] <- 88
00000000 40000000	//	end

Desired behavior:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88.

Behavior after patch:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88.

2-line inside multi-line:

88200000 00000001	//	if 8-bit [02000000] == 01, then until end:
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
00200008 00000088	//	|   |   [02000008] <- 88
00200009 00000099	//	\   \   [02000009] <- 99
00000000 40000000	//	end

Desired behavior:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

Behavior after patch:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

Multi-line inside multi-line:

88200000 00000001	//	if 8-bit [02000000] == 01, then until end:
88200000 00000001	//	|   if 8-bit [02000000] == 01, then until end:
00200008 00000088	//	|   |   [02000008] <- 88
00200009 00000099	//	\   \   [02000009] <- 99
00000000 40000000	//	end

Desired behavior:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes 99.
  • If [02000000] == 01, writes 88 and 99.

Behavior after patch:

  • If [02000000] == 00, writes 88 and 99.
  • If [02000000] == 01, writes 88 and 99.

Complex nesting (note the 99 is not in the same block as the 88):

88200000 00000001	//	if 8-bit [02000000] == 1, then until end:
88200000 00000001	//	|   if 8-bit [02000000] == 1, then until end:
08200000 00000001	//	|   |   if 8-bit [02000000] == 1, then 1 line:
00200008 00000088	//	|   |   \   [02000008] <- 88
00200009 00000099	//	\   \   [02000009] <- 99
00000000 40000000	//	end

Desired behavior:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes 88 and 99.
  • If [02000000] == 01, writes nothing.

Behavior after patch:

  • If [02000000] == 00, writes 88 and 99.
  • If [02000000] == 01, writes 88 and 99.

More complex nesting

88200000 00000001	//	if 8-bit [02000000] == 01, then until end:
88200000 00000001	//	|   if 8-bit [02000000] == 01, then until end:
48200000 00000001	//	|   |   if 8-bit [02000000] == 01, then 2 lines:
08200000 00000001	//	|   |   |   if 8-bit [02000000] == 01, then 1 line:
00200008 00000088	//	|   |   \   \   [02000008] <- 88
00200009 00000099	//	\   \   [02000009] <- 99
00000000 40000000	//	end

Desired behavior:

  • If [02000000] == 00, writes nothing.
  • If [02000000] == 01, writes 88 and 99.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes 99.
  • If [02000000] == 01, writes 88 and 99.

Behavior after patch:

  • If [02000000] == 00, writes 88 and 99.
  • If [02000000] == 01, writes 88 and 99.

@endrift
Copy link
Member

endrift commented Sep 28, 2017

Multi-line inside of multi-line is not and will not be supported without major changes. Nesting 1/2 lines inside of each and with a multi-line on the top should work but clearly don't at the moment. I'll look into it when I'm done with work.

@endrift
Copy link
Member

endrift commented Sep 29, 2017

Here's a revised version that should fix the ones that the previous one broke, and hopefully fix nesting 1/2 lines inside of multilines, though not multiline inside anything else (those still have to be on the outside):

diff --git a/src/core/cheats.c b/src/core/cheats.c
index 5085a3d67..1d65e4411 100644
--- a/src/core/cheats.c
+++ b/src/core/cheats.c
@@ -256,35 +256,23 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 	if (!cheats->enabled) {
 		return;
 	}
-	bool condition = true;
-	int conditionRemaining = 0;
-	int negativeConditionRemaining = 0;
 	cheats->refresh(cheats, device);
 
+	int untilElse = 0;
+	int elseBlock = 0;
 	size_t nCodes = mCheatListSize(&cheats->list);
 	size_t i;
 	for (i = 0; i < nCodes; ++i) {
-		if (conditionRemaining > 0) {
-			--conditionRemaining;
-			if (!condition) {
-				continue;
-			}
-		} else if (negativeConditionRemaining > 0) {
-			conditionRemaining = negativeConditionRemaining - 1;
-			negativeConditionRemaining = 0;
-			condition = !condition;
-			if (!condition) {
-				continue;
-			}
-		} else {
-			condition = true;
-		}
 		struct mCheat* cheat = mCheatListGetPointer(&cheats->list, i);
 		int32_t value = 0;
 		int32_t operand = cheat->operand;
 		uint32_t operationsRemaining = cheat->repeat;
 		uint32_t address = cheat->address;
 		bool performAssignment = false;
+		bool condition = true;
+		int conditionRemaining = 0;
+		int negativeConditionRemaining = 0;
+
 		for (; operationsRemaining; --operationsRemaining) {
 			switch (cheat->type) {
 			case CHEAT_ASSIGN:
@@ -312,46 +300,55 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 				condition = _readMem(device->p, address, cheat->width) == operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_NE:
 				condition = _readMem(device->p, address, cheat->width) != operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_LT:
 				condition = _readMem(device->p, address, cheat->width) < operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_GT:
 				condition = _readMem(device->p, address, cheat->width) > operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_ULT:
 				condition = (uint32_t) _readMem(device->p, address, cheat->width) < (uint32_t) operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_UGT:
 				condition = (uint32_t) _readMem(device->p, address, cheat->width) > (uint32_t) operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_AND:
 				condition = _readMem(device->p, address, cheat->width) & operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_LAND:
 				condition = _readMem(device->p, address, cheat->width) && operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_NAND:
 				condition = !(_readMem(device->p, address, cheat->width) & operand);
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			}
 
@@ -362,6 +359,18 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 			address += cheat->addressOffset;
 			operand += cheat->operandOffset;
 		}
+		if (untilElse) {
+			--untilElse;
+			if (untilElse == 0) {
+				i += elseBlock;
+			}
+		}
+		if (conditionRemaining > 0 && !condition) {
+			i += conditionRemaining;
+		} else if (negativeConditionRemaining > 0) {
+			untilElse = conditionRemaining;
+			elseBlock = negativeConditionRemaining;
+		}
 	}
 }
 

Let me know. I'm not in a great position to be testing it right now, so ELSE/END may still be broken for all I know.

@Prof9
Copy link
Contributor Author

Prof9 commented Sep 29, 2017

I've applied the second patch. This fixes the single 1-line and single 2-line test cases in my previous post. The other test cases yield the same result.

I've done some further testing with nesting 1/2-line conditionals inside a multi-line conditional, as well as sequential 1/2/multi-line conditionals. Anything involving multi-line conditionals still seems to be quite broken, even when you're not nesting them.

Sequential 1-line:

00200004 00000044	//	[02000004] <- 44
08200000 00000001	//	if 8-bit [02000000] == 01, then 1 line:
00200008 00000088	//	\   [02000008] <- 88
00200005 00000055	//	[02000005] <- 55
08200000 00000001	//	if 8-bit [02000000] == 01, then 1 line:
00200009 00000099	//	\   [02000009] <- 99
00200006 00000066	//	[02000006] <- 66
08200000 00000001	//	if 8-bit [02000000] == 01, then 1 line:
0020000A 000000AA	//	\   [0200000A] <- AA
08200000 00000001	//	if 8-bit [02000000] == 01, then 1 line:
0020000B 000000BB	//	\   [0200000B] <- BB
00200007 00000077	//	[02000007] <- 77

Desired behavior:

  • If [02000000] == 00, writes 44, 55, 66, 77.
  • If [02000000] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes 44, 55, 66, 77.
  • If [02000000] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB

Behavior after patch v2:

  • If [02000000] == 00, writes 44, 55, 66, 77.
  • If [02000000] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB

Sequential 2-line:

00200004 00000044	//	[02000004] <- 44
48200000 00000001	//	if 8-bit [02000000] == 01, then 2 lines:
00200008 00000088	//	|   [02000008] <- 88
00200009 00000099	//	\   [02000009] <- 99
00200005 00000055	//	[02000005] <- 55
48200000 00000001	//	if 8-bit [02000000] == 01, then 2 lines:
0020000A 000000AA	//	|   [0200000A] <- AA
0020000B 000000BB	//	\   [0200000B] <- BB
00200006 00000066	//	[02000006] <- 66
48200000 00000001	//	if 8-bit [02000000] == 01, then 2 lines:
0020000C 000000CC	//	|   [0200000C] <- CC
0020000D 000000DD	//	\   [0200000D] <- DD
48200000 00000001	//	if 8-bit [02000000] == 01, then 2 lines:
0020000E 000000EE	//	|   [0200000E] <- EE
0020000F 000000FF	//	\   [0200000F] <- FF
00200007 00000077	//	[02000007] <- 77

Desired behavior:

  • If [02000000] == 00, writes 44, 55, 66, 77.
  • If [02000000] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB, CC, DD, EE, FF.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes 44, 55, 66, 77.
  • If [02000000] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB, CC, DD, EE, FF.

Behavior after patch v2:

  • If [02000000] == 00, writes 44, 55, 66, 77.
  • If [02000000] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB, CC, DD, EE, FF.

Sequential multi-line:

00200004 00000044	//	[02000004] <- 44
88200000 00000001	//	if 8-bit [02000000] == 01, then until end:
00200008 00000088	//	|   [02000008] <- 88
00200009 00000099	//	\   [02000009] <- 99
00000000 40000000	//	end
00200005 00000055	//	[02000005] <- 55
88200000 00000001	//	if 8-bit [02000000] == 01, then until end:
0020000A 000000AA	//	|   [0200000A] <- AA
0020000B 000000BB	//	\   [0200000B] <- BB
00000000 40000000	//	end
00200006 00000066	//	[02000006] <- 66
88200000 00000001	//	if 8-bit [02000000] == 01, then until end:
0020000C 000000CC	//	|   [0200000C] <- CC
0020000D 000000DD	//	\   [0200000D] <- DD
00000000 40000000	//	end
88200000 00000001	//	if 8-bit [02000000] == 01, then until end:
0020000E 000000EE	//	|   [0200000E] <- EE
0020000F 000000FF	//	\   [0200000F] <- FF
00000000 40000000	//	end
00200007 00000077	//	[02000007] <- 77

Desired behavior:

  • If [02000000] == 00, writes 44, 55, 66, 77.
  • If [02000000] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB, CC, DD, EE, FF.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00, writes 44, EE and FF.
  • If [02000000] == 01, writes 44, 55, 66, 88, 99, AA, BB, CC, DD, EE, FF.

Behavior after patch v2:

  • If [02000000] == 00, writes 44, EE and FF.
  • If [02000000] == 01, writes 44, 55, 66, 88, 99, AA, BB, CC, DD, EE, FF.

Sequential 1-line inside a multi-line:

00200010 000000F0	//	[02000010] <- F0
00200011 000000F1	//	[02000011] <- F1
88200001 00000001	//	if 8-bit [02000001] == 01, then until end:
00200004 00000044	//	|   [02000004] <- 44
08200000 00000001	//	|   if 8-bit [02000000] == 01, then 1 line:
00200008 00000088	//	|   \   [02000008] <- 88
00200005 00000055	//	|   [02000005] <- 55
08200000 00000001	//	|   if 8-bit [02000000] == 01, then 1 line:
00200009 00000099	//	|   \   [02000009] <- 99
00200006 00000066	//	|   [02000006] <- 66
08200000 00000001	//	|   if 8-bit [02000000] == 01, then 1 line:
0020000A 000000AA	//	|   \   [0200000A] <- AA
08200000 00000001	//	|   if 8-bit [02000000] == 01, then 1 line:
0020000B 000000BB	//	|   \   [0200000B] <- BB
00200007 00000077	//	\   [02000007] <- 77
00000000 40000000	//	end
00200012 000000F2	//	[02000012] <- F2
00200013 000000F3	//	[02000013] <- F3

Desired behavior:

  • If [02000000] == 00 and [02000001] == 00, writes F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 00, writes F0, F1, F2, F3.
  • If [02000000] == 00 and [02000001] == 01, writes 44, 55, 66, 77, F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB, F0, F1, F2, F3.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00 and [02000001] == 00, writes F0, F1, F3.
  • If [02000000] == 01 and [02000001] == 00, writes F0, F1, F3.
  • If [02000000] == 00 and [02000001] == 01, writes 44, 55, 66, 77, F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB, F0, F1, F2, F3.

Behavior after patch v2:

  • If [02000000] == 00 and [02000001] == 00, writes F0, F1, F3.
  • If [02000000] == 01 and [02000001] == 00, writes F0, F1, F3.
  • If [02000000] == 00 and [02000001] == 01, writes 44, 55, 66, 77, F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB, F0, F1, F2, F3.

Sequential 2-line inside a multi-line:

00200010 000000F0	//	[02000010] <- F0
00200011 000000F1	//	[02000011] <- F1
88200001 00000001	//	if 8-bit [02000001] == 01, then until end:
00200004 00000044	//	|   [02000004] <- 44
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
00200008 00000088	//	|   |   [02000008] <- 88
00200009 00000099	//	|   \   [02000009] <- 99
00200005 00000055	//	|   [02000005] <- 55
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
0020000A 000000AA	//	|   |   [0200000A] <- AA
0020000B 000000BB	//	|   \   [0200000B] <- BB
00200006 00000066	//	|   [02000006] <- 66
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
0020000C 000000CC	//	|   |   [0200000C] <- CC
0020000D 000000DD	//	|   \   [0200000D] <- DD
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
0020000E 000000EE	//	|   |   [0200000E] <- EE
0020000F 000000FF	//	|   \   [0200000F] <- FF
00200007 00000077	//	\   [02000007] <- 77
00000000 40000000	//	end
00200012 000000F2	//	[02000012] <- F2
00200013 000000F3	//	[02000013] <- F3

Desired behavior:

  • If [02000000] == 00 and [02000001] == 00, writes F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 00, writes F0, F1, F2, F3.
  • If [02000000] == 00 and [02000001] == 01, writes 44, 55, 66, 77, F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB, CC, DD, EE, FF, F0, F1, F2, F3.

Behavior on 0.7-4855-3914da27:

  • If [02000000] == 00 and [02000001] == 00, writes F0, F1.
  • If [02000000] == 01 and [02000001] == 00, writes F0, F1.
  • If [02000000] == 00 and [02000001] == 01, writes 44, 55, 66, 77, F0, F1, F2.
  • If [02000000] == 01 and [02000001] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB, CC, DD, EE, FF, F0, F1, F2.

Behavior after patch v2:

  • If [02000000] == 00 and [02000001] == 00, writes F0, F1.
  • If [02000000] == 01 and [02000001] == 00, writes F0, F1.
  • If [02000000] == 00 and [02000001] == 01, writes 44, 55, 66, 77, F0, F1, F2.
  • If [02000000] == 01 and [02000001] == 01, writes 44, 55, 66, 77, 88, 99, AA, BB, CC, DD, EE, FF, F0, F1, F2.

@endrift
Copy link
Member

endrift commented Sep 29, 2017

Looks like I somehow missed two changes in that patch, here now:

diff --git a/src/core/cheats.c b/src/core/cheats.c
index 5085a3d67..1d65e4411 100644
--- a/src/core/cheats.c
+++ b/src/core/cheats.c
@@ -256,35 +256,23 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 	if (!cheats->enabled) {
 		return;
 	}
-	bool condition = true;
-	int conditionRemaining = 0;
-	int negativeConditionRemaining = 0;
 	cheats->refresh(cheats, device);
 
+	int untilElse = 0;
+	int elseBlock = 0;
 	size_t nCodes = mCheatListSize(&cheats->list);
 	size_t i;
 	for (i = 0; i < nCodes; ++i) {
-		if (conditionRemaining > 0) {
-			--conditionRemaining;
-			if (!condition) {
-				continue;
-			}
-		} else if (negativeConditionRemaining > 0) {
-			conditionRemaining = negativeConditionRemaining - 1;
-			negativeConditionRemaining = 0;
-			condition = !condition;
-			if (!condition) {
-				continue;
-			}
-		} else {
-			condition = true;
-		}
 		struct mCheat* cheat = mCheatListGetPointer(&cheats->list, i);
 		int32_t value = 0;
 		int32_t operand = cheat->operand;
 		uint32_t operationsRemaining = cheat->repeat;
 		uint32_t address = cheat->address;
 		bool performAssignment = false;
+		bool condition = true;
+		int conditionRemaining = 0;
+		int negativeConditionRemaining = 0;
+
 		for (; operationsRemaining; --operationsRemaining) {
 			switch (cheat->type) {
 			case CHEAT_ASSIGN:
@@ -312,46 +300,55 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 				condition = _readMem(device->p, address, cheat->width) == operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_NE:
 				condition = _readMem(device->p, address, cheat->width) != operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_LT:
 				condition = _readMem(device->p, address, cheat->width) < operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_GT:
 				condition = _readMem(device->p, address, cheat->width) > operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_ULT:
 				condition = (uint32_t) _readMem(device->p, address, cheat->width) < (uint32_t) operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_UGT:
 				condition = (uint32_t) _readMem(device->p, address, cheat->width) > (uint32_t) operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_AND:
 				condition = _readMem(device->p, address, cheat->width) & operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_LAND:
 				condition = _readMem(device->p, address, cheat->width) && operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_NAND:
 				condition = !(_readMem(device->p, address, cheat->width) & operand);
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			}
 
@@ -362,6 +359,18 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 			address += cheat->addressOffset;
 			operand += cheat->operandOffset;
 		}
+		if (untilElse) {
+			--untilElse;
+			if (untilElse == 0) {
+				i += elseBlock;
+			}
+		}
+		if (conditionRemaining > 0 && !condition) {
+			i += conditionRemaining;
+		} else if (negativeConditionRemaining > 0) {
+			untilElse = conditionRemaining;
+			elseBlock = negativeConditionRemaining;
+		}
 	}
 }
 
diff --git a/src/gba/cheats/parv3.c b/src/gba/cheats/parv3.c
index 63d2d2685..3f8563978 100644
--- a/src/gba/cheats/parv3.c
+++ b/src/gba/cheats/parv3.c
@@ -53,7 +53,7 @@ static uint32_t _parAddr(uint32_t x) {
 }
 
 static void _parEndBlock(struct GBACheatSet* cheats) {
-	size_t size = mCheatListSize(&cheats->d.list) - cheats->currentBlock;
+	size_t size = mCheatListSize(&cheats->d.list) - cheats->currentBlock - 1;
 	struct mCheat* currentBlock = mCheatListGetPointer(&cheats->d.list, cheats->currentBlock);
 	if (currentBlock->repeat) {
 		currentBlock->negativeRepeat = size - currentBlock->repeat;
@@ -64,7 +64,7 @@ static void _parEndBlock(struct GBACheatSet* cheats) {
 }
 
 static void _parElseBlock(struct GBACheatSet* cheats) {
-	size_t size = mCheatListSize(&cheats->d.list) - cheats->currentBlock;
+	size_t size = mCheatListSize(&cheats->d.list) - cheats->currentBlock - 1;
 	struct mCheat* currentBlock = mCheatListGetPointer(&cheats->d.list, cheats->currentBlock);
 	currentBlock->repeat = size;
 }

@Prof9
Copy link
Contributor Author

Prof9 commented Sep 29, 2017

All the new tests are now behaving correctly, looks good! I also noticed that my copy of ARCrypt was dropping the last line of code for some of the tests, which caused some of the test results above to be wrong. I double checked it for this round of tests, though.

I re-checked all of the test codes again on this new patch. The following tests still fail, but they all nest multi-line inside multi-line, so that's not surprising.

Multi-line inside multi-line:

  • If [02000000] == 00, writes 88 and 99.
  • If [02000000] == 01, writes 88 and 99.

Complex nesting:

  • If [02000000] == 00, writes 99.
  • If [02000000] == 01, writes 88 and 99.

More complex nesting:

  • If [02000000] == 00, writes 99.
  • If [02000000] == 01, writes 88 and 99.

By the way, the RAW code detection still seems to be quite broken. None of the test codes do anything if I paste them in without encrypting them first, a bunch of lines get dropped as well. I'm applying the patch to b501ca5, so your recent changes to RAW code detection should be included. Do I need to do something special when adding RAW codes?

PARv3 codes aren't meant to be used unencrypted anyway, though, so that's not really a huge deal.

I'll do some more testing with else blocks and multi->2->1 line nesting, I'll let you know if that yields any interesting results.

@endrift
Copy link
Member

endrift commented Sep 29, 2017

The heuristics are probably detecting the wrong code type over the right one. They’re pretty primitive heuristics. You can force it by loading the cheats from file but there’s no way to do it in the UI yet.

I’m probably gonna merge these fixes tonight and back port them to 0.6.1, closing this bug. After that I’ll add a BRANCH op to make ELSE work while nested. BRANCH won’t get merged back into 0.6.x though.

@Prof9
Copy link
Contributor Author

Prof9 commented Sep 29, 2017

Yeah, else blocks still seem to be wonky. The second test code below doesn't work right with this new patch.

Multi->2->1 nesting:

00200010 000000F0	//	[02000010] <- F0
00200011 000000F1	//	[02000011] <- F1
88200001 00000001	//	if 8-bit [02000001] == 01, then until end:
00200004 00000044	//	|   [02000004] <- 44
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
08200000 00000001	//	|   |   if 8-bit [02000000] == 01, then 1 line:
0020000A 000000AA	//	|   \   \   [0200000A] <- AA
00200005 00000055	//	|   [02000005] <- 55
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
08200000 00000001	//	|   |   if 8-bit [02000000] == 01, then 1 line:
0020000B 000000BB	//	|   \   \   [0200000B] <- BB
00200006 00000066	//	\   [02000006] <- 66
00000000 40000000	//	end
00200012 000000F2	//	[02000012] <- F2
00200013 000000F3	//	[02000013] <- F3

Desired behavior:

  • If [02000000] == 00 and [02000001] == 00, writes F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 00, writes F0, F1, F2, F3.
  • If [02000000] == 00 and [02000001] == 01, writes 44, 55, 66, F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 01, writes 44, 55, 66, AA, BB, F0, F1, F2, F3.

Behavior after patch v3:

  • If [02000000] == 00 and [02000001] == 00, writes F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 00, writes F0, F1, F2, F3.
  • If [02000000] == 00 and [02000001] == 01, writes 44, 55, 66, F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 01, writes 44, 55, 66, AA, BB, F0, F1, F2, F3.

Multi->2->1 nesting with else block:

00200010 000000F0	//	[02000010] <- F0
00200011 000000F1	//	[02000011] <- F1
88200001 00000001	//	if 8-bit [02000001] == 01, then until end:
00200004 00000044	//	|   [02000004] <- 44
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
08200000 00000001	//	|   |   if 8-bit [02000000] == 01, then 1 line:
0020000A 000000AA	//	|   \   \   [0200000A] <- AA
00200005 00000055	//	|   [02000005] <- 55
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
08200000 00000001	//	|   |   if 8-bit [02000000] == 01, then 1 line:
0020000B 000000BB	//	|   \   \   [0200000B] <- BB
00200006 00000066	//	\   [02000006] <- 66
00000000 60000000	//	else
00200007 00000077	//	|   [02000007] <- 77
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
08200000 00000001	//	|   |   if 8-bit [02000000] == 01, then 1 line:
0020000C 000000CC	//	|   \   \   [0200000C] <- CC
00200008 00000088	//	|   [02000008] <- 88
48200000 00000001	//	|   if 8-bit [02000000] == 01, then 2 lines:
08200000 00000001	//	|   |   if 8-bit [02000000] == 01, then 1 line:
0020000D 000000DD	//	|   \   \   [0200000D] <- DD
00200009 00000099	//	\   [02000009] <- 99
00000000 40000000	//	end
00200012 000000F2	//	[02000012] <- F2
00200013 000000F3	//	[02000013] <- F3

Desired behavior:

  • If [02000000] == 00 and [02000001] == 00, writes 77, 88, 99, F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 00, writes 77, 88, 99, CC, DD, F0, F1, F2, F3.
  • If [02000000] == 00 and [02000001] == 01, writes 44, 55, 66, F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 01, writes 44, 55, 66, AA, BB, F0, F1, F2, F3.

Behavior after patch v3:

  • If [02000000] == 00 and [02000001] == 00, writes 77, 88, 99, F0, F1, F2, F3.
  • If [02000000] == 01 and [02000001] == 00, writes 77, 88, 99, CC, DD, F0, F1, F2, F3.
  • If [02000000] == 00 and [02000001] == 01, writes 44, 55, 66, 77, 88, F0, F1.
  • If [02000000] == 01 and [02000001] == 01, writes 44, 55, 66, AA, BB, F0, F1, F2, F3.

@endrift
Copy link
Member

endrift commented Sep 29, 2017

Ok, I know what went wrong there but I don’t have my computer on me so I can’t look at it at all right now, and it’s too complicated to type up on my phone, but basically the problem is that untilElse doesn’t get decremented properly when an if block gets skipped. I should have time this weekend and I’ll write a proper test suite then.

@endrift
Copy link
Member

endrift commented Sep 30, 2017

Still in the middle of writing that test suite but I THINK this version works (I did test that specific last case):

Test suite says this one is good. Try it and make sure I'm not wrong before I merge it. Thanks.

diff --git a/src/core/cheats.c b/src/core/cheats.c
index 5085a3d67..21a7b8a7b 100644
--- a/src/core/cheats.c
+++ b/src/core/cheats.c
@@ -256,35 +256,23 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 	if (!cheats->enabled) {
 		return;
 	}
-	bool condition = true;
-	int conditionRemaining = 0;
-	int negativeConditionRemaining = 0;
 	cheats->refresh(cheats, device);
 
+	size_t elseLoc = 0;
+	size_t endLoc = 0;
 	size_t nCodes = mCheatListSize(&cheats->list);
 	size_t i;
 	for (i = 0; i < nCodes; ++i) {
-		if (conditionRemaining > 0) {
-			--conditionRemaining;
-			if (!condition) {
-				continue;
-			}
-		} else if (negativeConditionRemaining > 0) {
-			conditionRemaining = negativeConditionRemaining - 1;
-			negativeConditionRemaining = 0;
-			condition = !condition;
-			if (!condition) {
-				continue;
-			}
-		} else {
-			condition = true;
-		}
 		struct mCheat* cheat = mCheatListGetPointer(&cheats->list, i);
 		int32_t value = 0;
 		int32_t operand = cheat->operand;
 		uint32_t operationsRemaining = cheat->repeat;
 		uint32_t address = cheat->address;
 		bool performAssignment = false;
+		bool condition = true;
+		int conditionRemaining = 0;
+		int negativeConditionRemaining = 0;
+
 		for (; operationsRemaining; --operationsRemaining) {
 			switch (cheat->type) {
 			case CHEAT_ASSIGN:
@@ -312,46 +300,55 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 				condition = _readMem(device->p, address, cheat->width) == operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_NE:
 				condition = _readMem(device->p, address, cheat->width) != operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_LT:
 				condition = _readMem(device->p, address, cheat->width) < operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_GT:
 				condition = _readMem(device->p, address, cheat->width) > operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_ULT:
 				condition = (uint32_t) _readMem(device->p, address, cheat->width) < (uint32_t) operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_UGT:
 				condition = (uint32_t) _readMem(device->p, address, cheat->width) > (uint32_t) operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_AND:
 				condition = _readMem(device->p, address, cheat->width) & operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_LAND:
 				condition = _readMem(device->p, address, cheat->width) && operand;
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			case CHEAT_IF_NAND:
 				condition = !(_readMem(device->p, address, cheat->width) & operand);
 				conditionRemaining = cheat->repeat;
 				negativeConditionRemaining = cheat->negativeRepeat;
+				operationsRemaining = 1;
 				break;
 			}
 
@@ -362,6 +359,18 @@ void mCheatRefresh(struct mCheatDevice* device, struct mCheatSet* cheats) {
 			address += cheat->addressOffset;
 			operand += cheat->operandOffset;
 		}
+
+
+		if (elseLoc && i == elseLoc) {
+			i = endLoc;
+			endLoc = 0;
+		}
+		if (conditionRemaining > 0 && !condition) {
+			i += conditionRemaining;
+		} else if (negativeConditionRemaining > 0) {
+			elseLoc = i + conditionRemaining;
+			endLoc = elseLoc + negativeConditionRemaining;
+		}
 	}
 }
 
diff --git a/src/gba/cheats/parv3.c b/src/gba/cheats/parv3.c
index d2c9df589..087294d0f 100644
--- a/src/gba/cheats/parv3.c
+++ b/src/gba/cheats/parv3.c
@@ -53,7 +53,7 @@ static uint32_t _parAddr(uint32_t x) {
 }
 
 static void _parEndBlock(struct GBACheatSet* cheats) {
-	size_t size = mCheatListSize(&cheats->d.list) - cheats->currentBlock;
+	size_t size = mCheatListSize(&cheats->d.list) - cheats->currentBlock - 1;
 	struct mCheat* currentBlock = mCheatListGetPointer(&cheats->d.list, cheats->currentBlock);
 	if (currentBlock->repeat) {
 		currentBlock->negativeRepeat = size - currentBlock->repeat;
@@ -64,7 +64,7 @@ static void _parEndBlock(struct GBACheatSet* cheats) {
 }
 
 static void _parElseBlock(struct GBACheatSet* cheats) {
-	size_t size = mCheatListSize(&cheats->d.list) - cheats->currentBlock;
+	size_t size = mCheatListSize(&cheats->d.list) - cheats->currentBlock - 1;
 	struct mCheat* currentBlock = mCheatListGetPointer(&cheats->d.list, cheats->currentBlock);
 	currentBlock->repeat = size;
 }

@Prof9
Copy link
Contributor Author

Prof9 commented Sep 30, 2017

All my test codes are now behaving correctly, except for the ones I outlined in #889 (comment) (the behavior of these is unchanged). Looks good!

I also tested the original code I was trying to get to work, specifically the one with multi-line nesting removed:

9020A7E0 00000000	//	if 8-bit [0200A7E0] != 00, then until end:
0A202136 0000252A	//	|   if 16-bit [02002136] == 252A, then 1 line:
00202137 00000022	//	\   \   [02002137] <- 22
00000000 40000000	//	end
8820A7E0 00000004	//	if 8-bit [0200A7E0] == 04, then until end:
48202134 00000082	//	|   if 8-bit [02002134] == 82, then 2 lines:
0A202136 0000222A	//	|   |   if 16-bit [02002136] == 222A, then 1 line:
00202137 00000025	//	\   \   \   [02002137] <- 25
00000000 40000000	//	end

With the new patch, it now functions correctly when entered as a single code set.

Thanks for the quick fixes!

@Prof9
Copy link
Contributor Author

Prof9 commented Sep 30, 2017

I'm sorry endrift, I made even more weird tests. I decided to try some more exotic constructs that can't really be translated to if-then-else on a line-by-line basis. mGBA actually holds up really well here, though some cases break in interesting ways. It might be worth checking to see if this is really how mGBA should be parsing them. Anyway, I don't really expect you to support these, lol.

Skipping over ifs:

00200004 00000044	//	write 44
08200000 00000001	//	1-line if [02000000] == 01
48200001 00000001	//	2-line if [02000001] == 01
00200008 00000088	//	write 88
08200002 00000001	//	1-line if [02000002] == 01
00200009 00000099	//	write 99
00200005 00000055	//	write 55

Expected behavior:

  • If 00, 00, 00, writes 44, 55, 88
  • If 01, 00, 00, writes 44, 55, 99
  • If 00, 01, 00, writes 44, 55, 88
  • If 01, 01, 00, writes 44, 55, 88
  • If 00, 00, 01, writes 44, 55, 88, 99
  • If 01, 00, 01, writes 44, 55, 99
  • If 00, 01, 01, writes 44, 55, 88, 99
  • If 01, 01, 01, writes 44, 55, 88, 99

Actual behavior:

  • If 00, 00, 00, writes 44, 55, 88
  • If 01, 00, 00, writes 44, 55, 99
  • If 00, 01, 00, writes 44, 55, 88
  • If 01, 01, 00, writes 44, 55, 88
  • If 00, 00, 01, writes 44, 55, 88, 99
  • If 01, 00, 01, writes 44, 55, 99
  • If 00, 01, 01, writes 44, 55, 88, 99
  • If 01, 01, 01, writes 44, 55, 88, 99

Skipping over multi-line if:

00200004 00000044	//	write 44
48200000 00000001	//	2-line if [02000000] == 01
88200001 00000001	//	n-line if [02000001] == 01
00200008 00000088	//	write 88
00200009 00000099	//	write 99
00000000 40000000	//	end
00200005 00000055	//	write 55

Expected behavior:

  • If 00, 00, writes 44, 55, 99
  • If 01, 00, writes 44, 55
  • If 00, 01, writes 44, 55, 99
  • If 01, 01, writes 44, 55, 88, 99

Actual behavior:

  • If 00, 00, writes 44, 55, 99
  • If 01, 00, writes 44, 55
  • If 00, 01, writes 44, 55, 99
  • If 01, 01, writes 44, 55, 88, 99

Skipping over end:

00200004 00000044	//	write 44
48200000 00000001	//	2-line if [02000000] == 01
88200001 00000001	//	n-line if [02000001] == 01
00200008 00000088	//	write 88
00200009 00000099	//	write 99
08200002 00000001	//	1-line if [02000002] == 01
00000000 40000000	//	end
0020000A 000000AA	//	write AA
0020000B 000000BB	//	write BB
00000000 40000000	//	end
00200005 00000055	//	write 55

Expected behavior:

  • If 00, 00, 00, writes 44, 55, 99, AA, BB
  • If 01, 00, 00, writes 44, 55, AA, BB
  • If 00, 01, 00, writes 44, 55, 99, AA, BB
  • If 01, 01, 00, writes 44, 55, 88, 99, AA, BB
  • If 00, 00, 01, writes 44, 55, 99, AA, BB
  • If 01, 00, 01, writes 44, 55, AA, BB
  • If 00, 01, 01, writes 44, 55, 99, AA, BB
  • If 01, 01, 01, writes 44, 55, 88, 99, AA, BB

Actual behavior:

  • If 00, 00, 00, writes 44, 55, 99, BB
  • If 01, 00, 00, writes 44, 55, AA, BB
  • If 00, 01, 00, writes 44, 55, 99, BB
  • If 01, 01, 00, writes 44, 55, 88, 99, BB
  • If 00, 00, 01, writes 44, 55, 99, AA, BB
  • If 01, 00, 01, writes 44, 55, AA, BB
  • If 00, 01, 01, writes 44, 55, 99, AA, BB
  • If 01, 01, 01, writes 44, 55, 88, 99, AA, BB

Skipping over else:

I'm not sure about the expected behavior on this one; might need to check on a real PARv3. Depends on what ELSE will do in the regular execution state.

00200004 00000044	//	write 44
48200000 00000001	//	2-line if [02000000] == 01
88200001 00000001	//	n-line if [02000001] == 01
00200008 00000088	//	write 88
00200009 00000099	//	write 99
08200002 00000001	//	1-line if [02000002] == 01
00000000 60000000	//	else
0020000A 000000AA	//	write AA
0020000B 000000BB	//	write BB
00000000 40000000	//	end
00200005 00000055	//	write 55

Expected behavior:

  • If 00, 00, 00, writes 44, 55, 99, AA, BB
  • If 01, 00, 00, writes 44, 55, AA, BB
  • If 00, 01, 00, writes 44, 55, 99, AA, BB
  • If 01, 01, 00, writes 44, 55, 88, 99, AA, BB
  • If 00, 00, 01, writes 44, 55, 99, AA, BB
  • If 01, 00, 01, writes 44, 55, AA, BB
  • If 00, 01, 01, writes 44, 55, 99, AA, BB
  • If 01, 01, 01, writes 44, 55, 88, 99

Actual behavior:

  • If 00, 00, 00, writes 44, 55, 99, BB <- This one is definitely wrong since AA and BB should be grouped together.
  • If 01, 00, 00, writes 44, 55, AA, BB
  • If 00, 01, 00, writes 44, 55, 99, BB
  • If 01, 01, 00, writes 44, 88, 99 <- 55 should never be skipped.
  • If 00, 00, 01, writes 44, 55, 99, AA, BB
  • If 01, 00, 01, writes 44, 55, AA, BB
  • If 00, 01, 01, writes 44, 55, 99, AA, BB
  • If 01, 01, 01, writes 44, 55, 88, 99

@endrift
Copy link
Member

endrift commented Sep 30, 2017

I knew skipping over ELSE and END would be broken due to how conditionals are implemented in mGBA but there’s no real reason to support those heh. Anyway when I implement BRANCH it should fix the ELSE case, but that’s not why I’d do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants