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.

Sign up for GitHub

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

Fix package rename #549

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix package rename #549

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc1a8cd
bug: fix package rename
Floweynt Dec 5, 2024
49fe968
revert changes to GuiController, apply suggested validation changes
Floweynt Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions enigma-swing/src/main/java/cuchaz/enigma/gui/ClassSelector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
});
Expand Down Expand Up @@ -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);
}
}
60 changes: 39 additions & 21 deletions enigma-swing/src/main/java/cuchaz/enigma/gui/Gui.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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<EntryChange<ClassEntry>> task = new ArrayList<>();
onRenameFromClassTree(targetName, node, task);
this.controller.applyChanges(vc, task);
Copy link
Collaborator

@2xsaiko 2xsaiko Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.controller.applyChanges(vc, task);
task.forEach(c -> this.controller.validateChange(vc, task));
if (!vc.canProceed()) {
return;
}
task.forEach(c -> this.controller.applyChange(vc, task));

This is better because now all the changes will be validated and it won't bail out at the first bad one (the iterator in applyChanges you added doesn't validate the rest after one fails) and also won't partially apply a bad change set. This is how this should be used, validate everything first, then actually apply, and they are deliberately separate steps for batch operations like this.

You don't need to change anything about GuiController.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed.

}

onRenameFromClassTree(vc, prevDataChild, dataChild, node);
}
private void onRenameFromClassTree(String targetName, DefaultMutableTreeNode node, List<EntryChange<ClassEntry>> 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()));
}
}

Expand Down
51 changes: 28 additions & 23 deletions enigma-swing/src/main/java/cuchaz/enigma/gui/GuiController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -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<? extends EntryChange<?>> 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<? extends EntryChange<?>> 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());
Expand Down