1.21.5 #4692
1.21.5 #4692
Conversation
|
Also need to bump the end of the version range in SETUP.md under "Command Line". |
|
goal beacon renderer has actually been broken since at least 1.20.4 so i dont think ill resolve that in this pr, problem for another day |
| ClickEvent clickEvent = style.getClickEvent(); | ||
| if (clickEvent == null) { | ||
| return; | ||
| } | ||
| if (!(clickEvent instanceof ClickEvent.RunCommand(String command))) return; |
There was a problem hiding this comment.
Cursed, but kinda nice?
Didn't know we can pattern match now. (only knew about o instanceof C o_)
There was a problem hiding this comment.
Quite sure this breaks if a book for example contains a click event running a command (as you are cancelling the original code away)
There was a problem hiding this comment.
You are right, this cancels any commands, even if Baritone doesn't handle them.
Now I do wonder though: Didn't the previous injection do the same thing?
There was a problem hiding this comment.
No because the injection point was different, the new injection point is also wrong I think. The code was previous only called when the command sending failed I think.
There was a problem hiding this comment.
Ok, fell for a little gotcha (tried to use /say hi in a click event) and the command generator I used was broken, but I managed to confirm that Baritone does correctly handle clickable text in 1.19.4 and does not allow running vanilla commands in 1.21.5.
Minecraft (<=1.21.4) used to display an error message when running a chat command without a leading slash, so my guess is that the previous injection point was exactly that error message, meaning it would only run for Baritone commands (or from anything else using anything but "/" as its prefix).
Problem with that behavior is that Minecraft (>=1.21.5) allows commands without prefix in click events now.
There was a problem hiding this comment.
Nah the ordinal was set to 1, meaning only the highlighted one was targetted. They changed this behaviour already in the past (in 1.19 versions) so I could imagine the Mixin silently broke without anyone noticing. (For the exact behaviour changes, see https://github.com/ViaVersion/ViaFabricPlus/blob/main/src/main/java/com/viaversion/viafabricplus/injection/mixin/features/run_command_action/MixinScreen.java#L46)
There was a problem hiding this comment.
Here's what I think should make this work. Not usable as is because it needs an additional import statement.
| if (!(clickEvent instanceof ClickEvent.RunCommand(String command))) return; | |
| if (!(clickEvent instanceof ClickEvent.RunCommand(String command))) return; | |
| if (!command.startsWith(FORCE_COMMAND_PREFIX)) { | |
| return; | |
| } |
There was a problem hiding this comment.
Isn't ordinal zero-based and the two calls in the screenshot the only candidates, so the second one is targeted?
There was a problem hiding this comment.
There is another logger usage above which is not part of the screenshot
There was a problem hiding this comment.
That one (and the fourth one below Nope, only the first one) has the wrong signature.
| @@ -52,23 +79,8 @@ static void glColor(Color color, float alpha) { | |||
| } | |||
|
|
|||
| static BufferBuilder startLines(Color color, float alpha, float lineWidth, boolean ignoreDepth) { | |||
There was a problem hiding this comment.
ignoreDepth here is now completely ignored
| if (settings.renderGoalIgnoreDepth.value) { | ||
| RenderSystem.disableDepthTest(); | ||
|
|
||
| } |
There was a problem hiding this comment.
Leaving this (and the one below) behind as a reminder?
| return nbt.getCompound("Regions") | ||
| .map(CompoundTag::values) | ||
| .map(r -> r.stream() | ||
| .filter(v -> v instanceof CompoundTag) | ||
| .map(CompoundTag.class::cast) | ||
| .toArray(CompoundTag[]::new) | ||
| ).orElse(new CompoundTag[0]); |
There was a problem hiding this comment.
This looks like it also fixes an NPE with malformed schematics. Should this be backported?
| for (String key : properties.keySet()) { | ||
| Property<?> property = block.getStateDefinition().getProperty(key); |
There was a problem hiding this comment.
Object -> String replacement also backportable?
|
There is a bug report for this (see #4697 (comment)), though I'm not yet sure whether that bug is actually introduced here. |
|
The bug also exists on previous versions. |
Goal Beacon Renderer(actually has been broken since at least 1.20.4)