Skip to content

Fix crash when using "Insert Bytes" on the topmost entry #209

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

Merged
merged 2 commits into from
Oct 8, 2021

Conversation

ko1N
Copy link
Contributor

@ko1N ko1N commented Oct 8, 2021

When using the "Insert Bytes" functions after right-clicking on the top-most class ReClass will crash with an ArgumentException due to the starting index being set to -1.

image

This PR will fix the function and insert the Bytes at position 0 (which is kind of the expected behavior).
Alternatively, the "Insert Bytes" menu entries could also be entirely removed when right-clicking on the class.

@KN4CK3R
Copy link
Member

KN4CK3R commented Oct 8, 2021

I think it would be better to deactivate that button because now you can call InsertBytes(null, 4). This will prepend 4 bytes but I don't think that should work.

Something like this:

diff --git a/ReClass.NET/Forms/MainForm.cs b/ReClass.NET/Forms/MainForm.cs
index e9a18b38..010eea25 100644
--- a/ReClass.NET/Forms/MainForm.cs
+++ b/ReClass.NET/Forms/MainForm.cs
@@ -495,7 +495,7 @@ namespace ReClassNET.Forms
 			};
 
 			addBytesToolStripMenuItem.Enabled = parentNode != null || nodeIsClass;
-			insertBytesToolStripMenuItem.Enabled = count == 1 && parentNode != null;
+			insertBytesToolStripMenuItem.Enabled = count == 1 && parentNode != null && !nodeIsClass;
 
 			changeTypeToolStripMenuItem.Enabled = count > 0 && !nodeIsClass;
 
@@ -829,11 +829,12 @@ namespace ReClassNET.Forms
 
 			var node = selectedNodes.FirstOrDefault()?.Node;
 			var parentContainer = node?.GetParentContainer();
+			var nodeIsClass = node is ClassNode;
 
-			addBytesToolStripDropDownButton.Enabled = parentContainer != null || node is ClassNode;
-			insertBytesToolStripDropDownButton.Enabled = selectedNodes.Count == 1 && parentContainer != null;
+			addBytesToolStripDropDownButton.Enabled = parentContainer != null || nodeIsClass;
+			insertBytesToolStripDropDownButton.Enabled = selectedNodes.Count == 1 && parentContainer != null && !nodeIsClass;
 
-			var enabled = selectedNodes.Count > 0 && !(node is ClassNode);
+			var enabled = selectedNodes.Count > 0 && !nodeIsClass;
 			toolStrip.Items.OfType<TypeToolStripButton>().ForEach(b => b.Enabled = enabled);
 		}
 

@ko1N
Copy link
Contributor Author

ko1N commented Oct 8, 2021

I think it would be better to deactivate that button because now you can call InsertBytes(null, 4). This will prepend 4 bytes but I don't think that should work.

Something like this:

diff --git a/ReClass.NET/Forms/MainForm.cs b/ReClass.NET/Forms/MainForm.cs
index e9a18b38..010eea25 100644
--- a/ReClass.NET/Forms/MainForm.cs
+++ b/ReClass.NET/Forms/MainForm.cs
@@ -495,7 +495,7 @@ namespace ReClassNET.Forms
 			};
 
 			addBytesToolStripMenuItem.Enabled = parentNode != null || nodeIsClass;
-			insertBytesToolStripMenuItem.Enabled = count == 1 && parentNode != null;
+			insertBytesToolStripMenuItem.Enabled = count == 1 && parentNode != null && !nodeIsClass;
 
 			changeTypeToolStripMenuItem.Enabled = count > 0 && !nodeIsClass;
 
@@ -829,11 +829,12 @@ namespace ReClassNET.Forms
 
 			var node = selectedNodes.FirstOrDefault()?.Node;
 			var parentContainer = node?.GetParentContainer();
+			var nodeIsClass = node is ClassNode;
 
-			addBytesToolStripDropDownButton.Enabled = parentContainer != null || node is ClassNode;
-			insertBytesToolStripDropDownButton.Enabled = selectedNodes.Count == 1 && parentContainer != null;
+			addBytesToolStripDropDownButton.Enabled = parentContainer != null || nodeIsClass;
+			insertBytesToolStripDropDownButton.Enabled = selectedNodes.Count == 1 && parentContainer != null && !nodeIsClass;
 
-			var enabled = selectedNodes.Count > 0 && !(node is ClassNode);
+			var enabled = selectedNodes.Count > 0 && !nodeIsClass;
 			toolStrip.Items.OfType<TypeToolStripButton>().ForEach(b => b.Enabled = enabled);
 		}
 

Ye that sounds like a reasonable approach. I commited the changes you proposed.

@KN4CK3R
Copy link
Member

KN4CK3R commented Oct 8, 2021

Thank you!

@KN4CK3R KN4CK3R merged commit b407db3 into ReClassNET:master Oct 8, 2021
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

Successfully merging this pull request may close these issues.

2 participants