Skip to content

Commit

Permalink
Fix a slew of ContextMenu bugs, improve link buttons, and simplify ev…
Browse files Browse the repository at this point in the history 
…en MORE!
  • Loading branch information
@mrbuilder1961
mrbuilder1961 committed Jan 27, 2025
1 parent 085f8de commit 6dbe76e
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 97 deletions.
111 changes: 39 additions & 72 deletions src/main/java/obro1961/chatpatches/gui/ContextMenu.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.google.gson.JsonParseException;
import com.mojang.authlib.GameProfile;
import net.minecraft.SharedConstants;
import net.minecraft.client.MinecraftClient;
import net.minecraft.client.gui.DrawContext;
import net.minecraft.client.gui.Element;
Expand All @@ -20,6 +19,7 @@
import net.minecraft.text.*;
import net.minecraft.util.Formatting;
import net.minecraft.util.JsonHelper;
import net.minecraft.util.StringHelper;
import net.minecraft.util.math.MathHelper;
import obro1961.chatpatches.ChatPatches;
import obro1961.chatpatches.accessor.ChatHudAccessor;
Expand Down Expand Up @@ -57,7 +57,7 @@ public class ContextMenu {
private static final MinecraftClient mc = MinecraftClient.getInstance();
private static final int buttonPadding = 4;
// text
static final UnaryOperator<Text> UNKNOWN = (id) -> Text.translatable("text.chatpatches.copy.unknownData", id); // todo make sure this looks okay in practice
static final UnaryOperator<Text> UNKNOWN = (id) -> Text.translatable("text.chatpatches.copy.unknownData", id);
static final Text MENU_STRING = Text.translatable("text.chatpatches.copy.copyString");
static final Text RAW_STR = Text.translatable("text.chatpatches.copy.rawString");
static final Text FORMATTED_STR = Text.translatable("text.chatpatches.copy.formattedString");
Expand Down Expand Up @@ -126,7 +126,7 @@ public class ContextMenu {
* also very helpful for closing the menu when it's no
* longer needed.
*/
private boolean noOp = false; public boolean isNoOp() {return noOp;}
private boolean noOp = false;


public ContextMenu(double mX, double mY) {
Expand All @@ -137,28 +137,29 @@ public ContextMenu(double mX, double mY) {

this.selectedVisibles = getFullMessageAt(mX, mY);

// disables the menu if it was passed invalid values or if it's disabled in the config
if(!config.contextMenu || mX < 0 || mY < 0 || selectedVisibles.isEmpty())
noOp = true;

List<ChatHudLine> chatMessages = ((ChatHudAccessor) mc.inGameHud.getChatHud()).chatpatches$getMessages();
String fH = TextUtils.reorder( selectedVisibles.getFirst().content(), false );
String fH = noOp ? "\u0000" : TextUtils.reorder(selectedVisibles.getFirst().content(), false);
// find the first message that starts with the hovered message (aka the hovered message)
this.selectedLine = chatMessages.stream()
.filter(msg ->
Formatting.strip( msg.content().getString() )
!noOp && Formatting.strip(msg.content().getString())
// longer messages sometimes fail because extra spaces appear to be added,
// so it now uses startsWith() bc the first one never has extra spaces.
.startsWith(fH.isEmpty() ? "\n" : fH) // detects messages starting with newlines, along with regular messages
)
.findFirst()
.orElse(NIL_HUD_LINE);
.findFirst().orElse(NIL_HUD_LINE);

if(!noOp && (selectedLine == NIL_HUD_LINE || !chatMessages.contains(selectedLine)))
noOp = true;

Style s = getMsgPart(selectedLine.content(), MSG_SENDER_INDEX).getStyle();
this.messageSender = s.getHoverEvent() != null && s.getHoverEvent().getValue(HoverEvent.Action.SHOW_ENTITY) instanceof HoverEvent.EntityContent ec
? new GameProfile(ec.uuid, ec.name.getString())
? new GameProfile(ec.uuid, Objects.requireNonNullElse(ec.name, UNKNOWN.apply(Text.of( "Sender " + ec.uuid.toString() ))).getString())
: NIL_MSG_DATA.sender();


// disables the menu if it was passed invalid values or if it's disabled in the config
if(!config.contextMenu || mX < 0 || mY < 0 || selectedVisibles.isEmpty() || !chatMessages.contains(selectedLine))
noOp = true;
}

/**
Expand All @@ -171,44 +172,6 @@ public static void updateHooks(Consumer<ClickableWidget> addSelectableChild, Con
ContextMenu.remove = remove;
}

/**
* Creates a new context menu with the specified Minecraft client
* and mouse position. If either mouse coordinate is negative, the
* {@link #noOp} menu is returned instead. Calculates the selected
* message lines from the mouse coordinates, and also returns the
* {@link #noOp} menu if they don't exist.
*
* <p>This method effectively serves as a constructor and
* initializer for the context menu, as it populates the required
* fields. However, this shouldn't be confused with
* {@link #init()}, which creates the widget buttons and
* related data structures.
*
* @apiNote Needed as a separate method from the constructor to
* allow the {@link #noOp} menu to be returned when applicable.
*/
public static ContextMenu of(double mX, double mY) {
List<ChatHudLine.Visible> visibles = getFullMessageAt(mX, mY);
if(!config.contextMenu || visibles.isEmpty() || mX < 0 || mY < 0)
return null;

final List<ChatHudLine> chatMessages = ((ChatHudAccessor) mc.inGameHud.getChatHud()).chatpatches$getMessages();

// longer messages sometimes fail because extra spaces appear to be added,
// so it now uses startsWith() bc the first one never has extra spaces.
String fH = TextUtils.reorder( visibles.getFirst().content(), false );
String firstHovered = fH.isEmpty() ? "\n" : fH; // fixes messages starting with newlines not being detected

// get hovered message index (messages) for all copying data
ChatHudLine selectedLine = chatMessages.stream()
.filter(msg -> Formatting.strip( msg.content().getString() ).startsWith(firstHovered))
.findFirst()
.orElse(NIL_HUD_LINE);

// ensures the selected line is in ChatHud#messages
return chatMessages.contains(selectedLine) ? new ContextMenu(mX, mY) : null;
}


/**
* Registers a button in the {@linkplain #buttonGrid button grid} and
Expand All @@ -232,16 +195,15 @@ private void registerButton(@NotNull Text id, @NotNull Supplier<Text> tooltipCop

PressableWidget button = ButtonWidget.builder(id, b -> {
Text copyText = tooltipCopyTextSupplier.get();
String copyStr = SharedConstants.stripInvalidChars(copyText.getString(), true); // strip section signs//todo fix?
String copyStr = StringHelper.stripTextFormat(copyText.getString());//todo fix?
if(!copyStr.isEmpty()) {
mc.keyboard.setClipboard(copyStr);
mc.getToastManager().add(new SystemToast( SystemToast.Type.PERIODIC_NOTIFICATION, Text.translatable("text.chatpatches.copy.copied"), copyText ));
}

pressAction.onPress(b);

//fixme idk why the menu doesnt close on click... see #mouseClicked
close(); // close the menu after copying (fixme does this work?)
close();
}).dimensions((int)clickPos.x, (int)clickPos.y, w, h).build();

button.setTooltip(Tooltip.of( tooltipCopyTextSupplier.get() )); //Text.of( tooltipCopyTextSupplier.get().getString().replaceAll("§", "&") )//prepub?
Expand Down Expand Up @@ -312,7 +274,6 @@ private void registerCopyOnlyButton(Text id, Text tooltipCopyText, int localRow,
public void init() {
if(noOp)
return;

if(addSelectableChild == null) {
ChatPatches.logReportMsg(new IllegalStateException("ChatScreen hook `addSelectableChild` not initialized"));
return;
Expand All @@ -327,7 +288,7 @@ public void init() {
.resultOrPartial(e -> ChatPatches.logReportMsg(new JsonParseException(e)))
.map(JsonHelper::toSortedString)
.map(Text::of)
.orElse(UNKNOWN.apply(JSON_STR)),
.orElse(UNKNOWN.apply(JSON_STR)),//prepub: can we make this format better? like /data get coded or even just ChatInputSuggestor formatting?
2, 1);

// timestamp buttons - conditional (not on boundary lines)
Expand All @@ -350,22 +311,20 @@ public void init() {
List<String> webLinks = TextUtils.getLinks(selectedLine.content().getString());
List<String> fileLinks = new ArrayList<>();
selectedLine.content().visit((style, str) -> {
if(style.getClickEvent() instanceof ClickEvent ce) {
if(ce.getAction() == ClickEvent.Action.OPEN_FILE)
fileLinks.add(ce.getValue());
else if(ce.getAction() == ClickEvent.Action.OPEN_URL && !webLinks.contains(ce.getValue()))
webLinks.add(ce.getValue());
if(style.getClickEvent() instanceof ClickEvent ce && ce.getValue() instanceof String v && !v.isBlank()) {
if(ce.getAction() == ClickEvent.Action.OPEN_URL && !webLinks.contains(v))
webLinks.add(v);
else if(ce.getAction() == ClickEvent.Action.OPEN_FILE && !fileLinks.contains(v))
fileLinks.add(v);
}
return Optional.empty();
}, Style.EMPTY);
// fixme: image not letting me click (no CE in style) and its not translatable, no idea whats wrong here but it cant work as of now
ChatPatches.LOGGER.warn("[DEBUG] [ContextMenu] webLinks: {}, fileLinks: {} translatable: {}", webLinks, fileLinks, selectedLine.content().getContent() instanceof TranslatableTextContent ttc ?
ttc.getArgs() : "x");

if(!webLinks.isEmpty() || !fileLinks.isEmpty()) {
registerProxyButton(MENU_LINKS, LINK_N.apply(1), 0, 0);

for(int i = 0; i < fileLinks.size(); i++)
registerCopyOnlyButton(LINK_N.apply(i + 1), Text.of("§a§n" + fileLinks.get(i)), i, 1);
registerCopyOnlyButton(LINK_N.apply(i + 1), Text.of("§6§n" + fileLinks.get(i)), i, 1);

for(int i = fileLinks.size(); i < webLinks.size() + fileLinks.size(); i++)
// creates link buttons starting at link 1 up to link n, with ids following the same pattern (LINK_1 - LINK_N)
Expand Down Expand Up @@ -544,19 +503,27 @@ public void mouseMoved(double mX, double mY) {
* screen properly.
*/
public void close() {
noOp = true;
if(remove == null) {
ChatPatches.logReportMsg(new IllegalStateException("ChatScreen hook `remove` not initialized"));
return;
}

buttonGrid.forEachChild(remove::accept);
noOp = true;
//selectedVisibles.clear();
widgets.clear();
}

/**
* @see #noOp
*
* @apiNote Intended for external
* (non-{@link ContextMenu}) use.
*/
public boolean isFunctional() {
return !noOp;
}

public boolean isMouseOver(double mX, double mY) {
return
return !noOp &&
mX >= buttonGrid.getX() && mX <= buttonGrid.getX() + buttonGrid.getWidth()
&& mY >= buttonGrid.getY() && mY <= buttonGrid.getY() + buttonGrid.getHeight();
}
Expand All @@ -568,7 +535,7 @@ public boolean isMouseOver(double mX, double mY) {
* so the last one to render should be the one to return.
*/
private Optional<PressableWidget> getHoveredButton(double mX, double mY) {
return !noOp && isMouseOver(mX, mY)
return isMouseOver(mX, mY)
? new ArrayList<>(widgets)
.reversed() // last button to render is the first to be checked, and for any overlap
.stream()
Expand Down Expand Up @@ -610,7 +577,7 @@ private Optional<PressableWidget> getHoveredButton(double mX, double mY) {
* </ol>
*/
private static @NotNull List<ChatHudLine.Visible> getFullMessageAt(double mX, double mY) {
if(mX < 0 || mY < 0)
if(!config.contextMenu || mX < 0 || mY < 0)
return new ArrayList<>(0);

final ChatHudAccessor chat = (ChatHudAccessor) mc.inGameHud.getChatHud();
Expand Down Expand Up @@ -692,7 +659,7 @@ public GridData(int maxRows, int maxCols) {
this.groups = new ArrayList<>(maxRows);
}

public void add(PressableWidget button, int localRow, int col, Supplier<Text> tooltipCopyTextSupplier, ButtonWidget.PressAction pressAction) { //todo final signature: (int lR, int c, ButtonWidget b)
public void add(PressableWidget button, int localRow, int col, Supplier<Text> tooltipCopyTextSupplier, ButtonWidget.PressAction pressAction) {
boolean newGroupAkaIsMain = button.visible = col == 0; // note: this will only break things if >1 main buttons are grouped together
int groupId = newGroupAkaIsMain ? groupCount++ : groupCount - 1;

Expand Down
46 changes: 25 additions & 21 deletions src/main/java/obro1961/chatpatches/mixin/gui/ChatScreenMixin.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package obro1961.chatpatches.mixin.gui;

import com.llamalad7.mixinextras.injector.v2.WrapWithCondition;
import com.llamalad7.mixinextras.injector.wrapmethod.WrapMethod;
import com.llamalad7.mixinextras.injector.wrapoperation.Operation;
import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation;
import net.fabricmc.api.EnvType;
Expand Down Expand Up @@ -193,8 +194,6 @@ protected void initSearchStuff(CallbackInfo ci) {
*/
@Inject(method = "render", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/gui/screen/Screen;render(Lnet/minecraft/client/gui/DrawContext;IIF)V"))
private void renderSearchAndContextMenuStuff(DrawContext context, int mX, int mY, float delta, CallbackInfo ci) {
client.getProfiler().push("chatpatches"); //prepub keep profiler stuff? see ContextMenu#render for more deets

context.getMatrices().push();
context.getMatrices().translate(0, 0, -1); // easiest fix to render everything effectively under the ChatInputSuggestor (#186)

Expand Down Expand Up @@ -225,7 +224,6 @@ private void renderSearchAndContextMenuStuff(DrawContext context, int mX, int mY
context.getMatrices().pop(); // stop shifting before the context menu renders so the chat field doesn't cut it off

contextMenu.render(context, mX, mY, delta);
client.getProfiler().pop();
}

/**
Expand Down Expand Up @@ -255,7 +253,7 @@ public void onScreenClose(CallbackInfo ci) {
else if(!searchField.getText().isEmpty())
client.inGameHud.getChatHud().reset(); // reset the hud if it had anything in the field (#102)

contextMenu.close(); // not unhooking here, because the screen is totally gone so rendering/usage is impossible
contextMenu.close();
ContextMenu.updateHooks(null, null);
}

Expand Down Expand Up @@ -289,6 +287,25 @@ private Style fixStyleClickthrough(ChatScreen screen, double mX, double mY, Oper
? null
: getTextStyleAt.call(screen, mX, mY);
}

/**
* We always want to close the context menu,
* EXCEPT when we created one (via right-click).
*
* @apiNote This wrapper can be called a LOT, so
* we want to keep its footprint as minimal as
* possible.
*/
@WrapMethod(method = "mouseClicked")
private boolean fixContextMenuNotClosing(double mX, double mY, int button, Operation<Boolean> mouseClicked) {
boolean clicked = mouseClicked.call(mX, mY, button);

if(button != GLFW.GLFW_MOUSE_BUTTON_RIGHT)
contextMenu.close(); // closes the menu if it wasn't just created, we don't care if anything was actually clicked

return clicked;
}

/**
* Returns {@code true} if the mouse clicked on any of the following:
* <ul>
Expand All @@ -315,8 +332,6 @@ public void registerClickEvents(double mX, double mY, int button, CallbackInfoRe
if(cir.getReturnValue())
return;

boolean closeContextMenu = true;

if(searchField.mouseClicked(mX, mY, button))
cir.setReturnValue(true);

Expand All @@ -327,30 +342,19 @@ public void registerClickEvents(double mX, double mY, int button, CallbackInfoRe
cir.setReturnValue(true);
if(regexButton.mouseClicked(mX, mY, button))
cir.setReturnValue(true);
} else if(button == GLFW.GLFW_MOUSE_BUTTON_LEFT || contextMenu.mouseClicked(mX, mY, button)) {
closeContextMenu = false;
contextMenu.close();
} else if(contextMenu.mouseClicked(mX, mY, button)) {
cir.setReturnValue(true);
} else if(button == GLFW.GLFW_MOUSE_BUTTON_RIGHT) {
// fixme: figure out how to close menu if anything other than right-click or menu clicked
// prepub: move this into a static ContextMenu method

ContextMenu mousePosMenu = new ContextMenu(mX, mY);
// if the mouse right-clicked elsewhere and that location can load a context menu, use it
if(contextMenu.clickPos.x != mX || contextMenu.clickPos.y != mY && !mousePosMenu.isNoOp()) {
if(contextMenu.clickPos.x != mX || contextMenu.clickPos.y != mY && mousePosMenu.isFunctional()) {
contextMenu.close(); // unhook the old context menu buttons
contextMenu = mousePosMenu; // keep and use the updated context menu
contextMenu.init(); // initialize the context menu and register the provided buttons
closeContextMenu = false;

contextMenu.init(); // initialize the new menu - old hooks are the same too!
cir.setReturnValue(true);
}
}

// if anything was clicked other than the context menu, and it was open, then close it
if(closeContextMenu && !contextMenu.isNoOp()) {//fixme (mayb not this statement idk) outline still renders after closing menu... but instead of setting = noop, what if
// we just add a noOp field to contextmenu and when we close it, we set it to true, and effectively brick the entire context menu? seems easier and epic
contextMenu.close();
}
}

/**
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/obro1961/chatpatches/util/TextUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import net.minecraft.text.*;
import net.minecraft.util.Formatting;
import net.minecraft.util.dynamic.Codecs;

import java.util.ArrayList;
import java.util.List;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -41,7 +40,7 @@ public static String fillVars(String str, String variable) {
*/
public static List<String> getLinks(String str) {
// slightly modified from https://stackoverflow.com/a/163398 to not include file links
final String urlRegex = "\\bhttps?://[-a-zA-Z0-9+&@#/%?=~_|!:,.;]*[-a-zA-Z0-9+&@#/%=~_|]";
final String urlRegex = "\\b(?:https?://|www)[-a-zA-Z0-9+&@#/%?=~_|!:,.;]*[-a-zA-Z0-9+&@#/%=~_|]";
final Matcher matcher = Pattern.compile(urlRegex).matcher(str);
List<String> urls = new ArrayList<>();

Expand Down Expand Up @@ -118,7 +117,7 @@ public static String reorder(OrderedText renderable, boolean includeStyleData) {
/**
* Takes a {@link Style} and returns a string of {@code &<?>}
* codes based upon the style's formatting data.
*/
*/ //todo: fix known colors returning hex codes, and remove reset codes when no modifier codes were present
@SuppressWarnings("StringBufferReplaceableByString") // StringBuilder is faster and String concatenation looks ugly
public static String getFormattingCodes(Style style) {
StringBuilder codes = new StringBuilder(18);
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/assets/chatpatches/lang/en_us.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@
"text.chatpatches.copy.name": "Username",
"text.chatpatches.copy.uuid": "UUID",
"text.chatpatches.copy.reply": "Reply",
"text.chatpatches.copy.unknownData": "§c§l/§e!§c\\ §r§7Unknown data for '§9%s§7', please report this issue!",
"text.chatpatches.copy.unknownData": "§c/§e!§c\\ §r§7Unknown data for '§6%s§7', please report this issue!",

"text.chatpatches.restored": "Restored message"
}

0 comments on commit 6dbe76e

Please sign in to comment.