From fc1a8cd9e211e4ef05808e095aba93a1c7aa756d Mon Sep 17 00:00:00 2001 From: Floweynt Date: Thu, 5 Dec 2024 13:40:36 -0600 Subject: [PATCH 1/2] bug: fix package rename Fixed package rename: - Breaking with nested packages - Breaking the tree view - Failing to remap some classes --- .../java/cuchaz/enigma/gui/ClassSelector.java | 28 ++++++--- .../src/main/java/cuchaz/enigma/gui/Gui.java | 60 ++++++++++++------- .../java/cuchaz/enigma/gui/GuiController.java | 51 +++++++++------- 3 files changed, 88 insertions(+), 51 deletions(-) diff --git a/enigma-swing/src/main/java/cuchaz/enigma/gui/ClassSelector.java b/enigma-swing/src/main/java/cuchaz/enigma/gui/ClassSelector.java index 5f53f4d8..adf07331 100644 --- a/enigma-swing/src/main/java/cuchaz/enigma/gui/ClassSelector.java +++ b/enigma-swing/src/main/java/cuchaz/enigma/gui/ClassSelector.java @@ -11,6 +11,8 @@ package cuchaz.enigma.gui; +import cuchaz.enigma.gui.node.ClassSelectorPackageNode; + import java.awt.Component; import java.awt.event.KeyAdapter; import java.awt.event.KeyEvent; @@ -150,14 +152,26 @@ public void editingStopped(ChangeEvent e) { } if (allowEdit && renameSelectionListener != null) { - Object prevData = node.getUserObject(); - Object objectData = node.getUserObject() instanceof ClassEntry ? new ClassEntry(((ClassEntry) prevData).getPackageName() + "/" + data) : data; gui.validateImmediateAction(vc -> { - renameSelectionListener.onSelectionRename(vc, node.getUserObject(), objectData, node); - - if (vc.canProceed()) { - node.setUserObject(objectData); // Make sure that it's modified + String parentName; + + if(node instanceof ClassSelectorPackageNode packageNode) { + String packageName = packageNode.getPackageName(); + int lastSlash = packageName.lastIndexOf("/"); + if(lastSlash != -1) { + parentName = packageName.substring(0, lastSlash); + } else { + parentName = ""; + } + } else if(node instanceof ClassSelectorClassNode classNode) { + parentName = classNode.getClassEntry().getPackageName(); } else { + throw new IllegalStateException("unknown node type " + node.getClass().getSimpleName()); + } + + renameSelectionListener.onSelectionRename(vc, parentName + "/" + data, node); + + if (!vc.canProceed()) { editor.cancelCellEditing(); } }); @@ -305,6 +319,6 @@ public interface ClassSelectionListener { } public interface RenameSelectionListener { - void onSelectionRename(ValidationContext vc, Object prevData, Object data, DefaultMutableTreeNode node); + void onSelectionRename(ValidationContext vc, String targetName, DefaultMutableTreeNode node); } } diff --git a/enigma-swing/src/main/java/cuchaz/enigma/gui/Gui.java b/enigma-swing/src/main/java/cuchaz/enigma/gui/Gui.java index 05146d4a..91eb99d8 100644 --- a/enigma-swing/src/main/java/cuchaz/enigma/gui/Gui.java +++ b/enigma-swing/src/main/java/cuchaz/enigma/gui/Gui.java @@ -16,6 +16,7 @@ import java.awt.Point; import java.awt.event.ActionEvent; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Set; @@ -57,6 +58,8 @@ import cuchaz.enigma.gui.elements.MainWindow; import cuchaz.enigma.gui.elements.MenuBar; import cuchaz.enigma.gui.elements.ValidatableUi; +import cuchaz.enigma.gui.node.ClassSelectorClassNode; +import cuchaz.enigma.gui.node.ClassSelectorPackageNode; import cuchaz.enigma.gui.panels.DeobfPanel; import cuchaz.enigma.gui.panels.EditorPanel; import cuchaz.enigma.gui.panels.IdentifierPanel; @@ -482,32 +485,47 @@ public void redraw() { frame.repaint(); } - public void onRenameFromClassTree(ValidationContext vc, Object prevData, Object data, DefaultMutableTreeNode node) { - if (data instanceof String) { - // package rename - for (int i = 0; i < node.getChildCount(); i++) { - DefaultMutableTreeNode childNode = (DefaultMutableTreeNode) node.getChildAt(i); - ClassEntry prevDataChild = (ClassEntry) childNode.getUserObject(); - ClassEntry dataChild = new ClassEntry(data + "/" + prevDataChild.getSimpleName()); + public void onRenameFromClassTree(ValidationContext vc, String targetName, DefaultMutableTreeNode node) { + List> task = new ArrayList<>(); + onRenameFromClassTree(targetName, node, task); + this.controller.applyChanges(vc, task); + } - onRenameFromClassTree(vc, prevDataChild, dataChild, node); - } + private void onRenameFromClassTree(String targetName, DefaultMutableTreeNode node, List> entryOps) { + if (targetName.startsWith("/")) { + targetName = targetName.substring(1); + } - node.setUserObject(data); - // Ob package will never be modified, just reload deob view - this.deobfPanel.deobfClasses.reload(); - } else if (data instanceof ClassEntry) { - // class rename + if (node instanceof ClassSelectorPackageNode) { + for (int i = 0; i < node.getChildCount(); i++) { + String childName; + DefaultMutableTreeNode child = (DefaultMutableTreeNode) node.getChildAt(i); + + if (child instanceof ClassSelectorPackageNode packageNode) { + String[] parts = packageNode.getPackageName().split("/"); + childName = targetName + "/" + parts[parts.length - 1]; + } else if (child instanceof ClassSelectorClassNode classNode) { + childName = targetName + "/" + classNode.getClassEntry().getSimpleName(); + } else { + throw new IllegalStateException(String.format("unhandled rename object type: '%s'", + child.getClass())); + } - // TODO optimize reverse class lookup, although it looks like it's - // fast enough for now + onRenameFromClassTree(childName, child, entryOps); + } + } else if (node instanceof ClassSelectorClassNode classSelectorClassNode) { EntryRemapper mapper = this.controller.project.getMapper(); - ClassEntry deobf = (ClassEntry) prevData; - ClassEntry obf = mapper.getObfToDeobf().getAllEntries().filter(e -> e instanceof ClassEntry).map(e -> (ClassEntry) e).filter(e -> mapper.deobfuscate(e).equals(deobf)).findAny().orElse(deobf); - - this.controller.applyChange(vc, EntryChange.modify(obf).withDeobfName(((ClassEntry) data).getFullName())); + ClassEntry deobf = classSelectorClassNode.getClassEntry(); + ClassEntry obf = mapper.getObfToDeobf().getAllEntries() + .filter(e -> e instanceof ClassEntry) + .map(e -> (ClassEntry) e) + .filter(e -> mapper.deobfuscate(e).equals(deobf)) + .findAny() + .orElse(deobf); + + entryOps.add(EntryChange.modify(obf).withDeobfName(targetName)); } else { - throw new IllegalStateException(String.format("unhandled rename object data: '%s'", data)); + throw new IllegalStateException(String.format("unhandled rename object type: '%s'", node.getClass())); } } diff --git a/enigma-swing/src/main/java/cuchaz/enigma/gui/GuiController.java b/enigma-swing/src/main/java/cuchaz/enigma/gui/GuiController.java index ad10abfc..6c3112b7 100644 --- a/enigma-swing/src/main/java/cuchaz/enigma/gui/GuiController.java +++ b/enigma-swing/src/main/java/cuchaz/enigma/gui/GuiController.java @@ -510,9 +510,7 @@ public MethodReferenceTreeNode getMethodReferences(MethodEntry entry, boolean re public boolean applyChangeFromServer(EntryChange change) { ValidationContext vc = new ValidationContext(); vc.setActiveElement(PrintValidatable.INSTANCE); - this.applyChange0(vc, change); - gui.showStructure(gui.getActiveEditor()); - + this.applyChanges0(vc, List.of(change)); return vc.canProceed(); } @@ -527,39 +525,46 @@ public void validateChange(ValidationContext vc, EntryChange change) { } public void applyChange(ValidationContext vc, EntryChange change) { - this.applyChange0(vc, change); - gui.showStructure(gui.getActiveEditor()); + applyChanges(vc, List.of(change)); + } + + public void applyChanges(ValidationContext vc, Iterable> changes) { + applyChanges0(vc, changes); if (!vc.canProceed()) { return; } - this.sendPacket(new EntryChangeC2SPacket(change)); + for (EntryChange change : changes) { + this.sendPacket(new EntryChangeC2SPacket(change)); + } } - private void applyChange0(ValidationContext vc, EntryChange change) { - validateChange(vc, change); + private void applyChanges0(ValidationContext vc, Iterable> changes) { + for (EntryChange change : changes) { + validateChange(vc, change); - if (!vc.canProceed()) { - return; - } + if (!vc.canProceed()) { + return; + } - Entry target = change.getTarget(); - EntryMapping prev = this.project.getMapper().getDeobfMapping(target); - EntryMapping mapping = EntryUtil.applyChange(vc, this.project.getMapper(), change); + Entry target = change.getTarget(); + EntryMapping prev = this.project.getMapper().getDeobfMapping(target); + EntryMapping mapping = EntryUtil.applyChange(vc, this.project.getMapper(), change); - boolean renamed = !change.getDeobfName().isUnchanged(); + boolean renamed = !change.getDeobfName().isUnchanged(); - if (renamed && target instanceof ClassEntry && !((ClassEntry) target).isInnerClass()) { - this.gui.moveClassTree(target, prev.targetName() == null, mapping.targetName() == null); - } + if (renamed && target instanceof ClassEntry && !((ClassEntry) target).isInnerClass()) { + this.gui.moveClassTree(target, prev.targetName() == null, mapping.targetName() == null); + } - if (!Objects.equals(prev.targetName(), mapping.targetName())) { - this.chp.invalidateMapped(); - } + if (!Objects.equals(prev.targetName(), mapping.targetName())) { + this.chp.invalidateMapped(); + } - if (!Objects.equals(prev.javadoc(), mapping.javadoc())) { - this.chp.invalidateJavadoc(target.getTopLevelClass()); + if (!Objects.equals(prev.javadoc(), mapping.javadoc())) { + this.chp.invalidateJavadoc(target.getTopLevelClass()); + } } gui.showStructure(gui.getActiveEditor()); From 49fe9685df21dfb9d2666add8053dfd7348d6d91 Mon Sep 17 00:00:00 2001 From: Floweynt Date: Thu, 12 Dec 2024 15:54:19 -0600 Subject: [PATCH 2/2] revert changes to GuiController, apply suggested validation changes --- .../src/main/java/cuchaz/enigma/gui/Gui.java | 9 +++- .../java/cuchaz/enigma/gui/GuiController.java | 51 +++++++++---------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/enigma-swing/src/main/java/cuchaz/enigma/gui/Gui.java b/enigma-swing/src/main/java/cuchaz/enigma/gui/Gui.java index 91eb99d8..db08fbe3 100644 --- a/enigma-swing/src/main/java/cuchaz/enigma/gui/Gui.java +++ b/enigma-swing/src/main/java/cuchaz/enigma/gui/Gui.java @@ -488,7 +488,14 @@ public void redraw() { public void onRenameFromClassTree(ValidationContext vc, String targetName, DefaultMutableTreeNode node) { List> task = new ArrayList<>(); onRenameFromClassTree(targetName, node, task); - this.controller.applyChanges(vc, task); + + task.forEach(c -> this.controller.validateChange(vc, c)); + + if (!vc.canProceed()) { + return; + } + + task.forEach(c -> this.controller.applyChange(vc, c)); } private void onRenameFromClassTree(String targetName, DefaultMutableTreeNode node, List> entryOps) { diff --git a/enigma-swing/src/main/java/cuchaz/enigma/gui/GuiController.java b/enigma-swing/src/main/java/cuchaz/enigma/gui/GuiController.java index 6c3112b7..ad10abfc 100644 --- a/enigma-swing/src/main/java/cuchaz/enigma/gui/GuiController.java +++ b/enigma-swing/src/main/java/cuchaz/enigma/gui/GuiController.java @@ -510,7 +510,9 @@ public MethodReferenceTreeNode getMethodReferences(MethodEntry entry, boolean re public boolean applyChangeFromServer(EntryChange change) { ValidationContext vc = new ValidationContext(); vc.setActiveElement(PrintValidatable.INSTANCE); - this.applyChanges0(vc, List.of(change)); + this.applyChange0(vc, change); + gui.showStructure(gui.getActiveEditor()); + return vc.canProceed(); } @@ -525,46 +527,39 @@ public void validateChange(ValidationContext vc, EntryChange change) { } public void applyChange(ValidationContext vc, EntryChange change) { - applyChanges(vc, List.of(change)); - } - - public void applyChanges(ValidationContext vc, Iterable> changes) { - applyChanges0(vc, changes); + this.applyChange0(vc, change); + gui.showStructure(gui.getActiveEditor()); if (!vc.canProceed()) { return; } - for (EntryChange change : changes) { - this.sendPacket(new EntryChangeC2SPacket(change)); - } + this.sendPacket(new EntryChangeC2SPacket(change)); } - private void applyChanges0(ValidationContext vc, Iterable> changes) { - for (EntryChange change : changes) { - validateChange(vc, change); + private void applyChange0(ValidationContext vc, EntryChange change) { + validateChange(vc, change); - if (!vc.canProceed()) { - return; - } + if (!vc.canProceed()) { + return; + } - Entry target = change.getTarget(); - EntryMapping prev = this.project.getMapper().getDeobfMapping(target); - EntryMapping mapping = EntryUtil.applyChange(vc, this.project.getMapper(), change); + Entry target = change.getTarget(); + EntryMapping prev = this.project.getMapper().getDeobfMapping(target); + EntryMapping mapping = EntryUtil.applyChange(vc, this.project.getMapper(), change); - boolean renamed = !change.getDeobfName().isUnchanged(); + boolean renamed = !change.getDeobfName().isUnchanged(); - if (renamed && target instanceof ClassEntry && !((ClassEntry) target).isInnerClass()) { - this.gui.moveClassTree(target, prev.targetName() == null, mapping.targetName() == null); - } + if (renamed && target instanceof ClassEntry && !((ClassEntry) target).isInnerClass()) { + this.gui.moveClassTree(target, prev.targetName() == null, mapping.targetName() == null); + } - if (!Objects.equals(prev.targetName(), mapping.targetName())) { - this.chp.invalidateMapped(); - } + if (!Objects.equals(prev.targetName(), mapping.targetName())) { + this.chp.invalidateMapped(); + } - if (!Objects.equals(prev.javadoc(), mapping.javadoc())) { - this.chp.invalidateJavadoc(target.getTopLevelClass()); - } + if (!Objects.equals(prev.javadoc(), mapping.javadoc())) { + this.chp.invalidateJavadoc(target.getTopLevelClass()); } gui.showStructure(gui.getActiveEditor());