KosmoMoustache / FixBookGUI

Fix MC-61489
GNU Lesser General Public License v3.0
3 stars 1 forks source link

Refactoring & conflict fixes #21

Closed diskree closed 10 months ago

diskree commented 10 months ago

Huh?

Hello! I'm making a mod that also makes changes to the GUI of the book, and I had a mixin conflict. The problem is that your code is currently doing more than it should. Instead of changing the code associated with the Y coordinate, your mod overwrites some of the vanilla code in different places. This is what bothered me, so I decided to open this pull request.

Changes

KosmoMoustache commented 10 months ago

Thank you for your contribution, unfortunately, the hoverEvent show_entity don't work

Here the command to give you the book I used: /give @p minecraft:written_book{generation: 3, pages: ['{"extra":[{"underlined":true,"clickEvent":{"action":"open_url","value":"https://google.com"},"text":"open url"},{"underlined":true,"text":"\\n\\n"},{"underlined":true,"clickEvent":{"action":"copy_to_clipboard","value":"test copied to clipboard from minecraft"},"text":"Copy to clipboard"},{"text":"\\n\\n"},{"color":"green","clickEvent":{"action":"change_page","value":"2"},"text":"goto Page 2"},{"text":"\\n\\n"},{"color":"green","clickEvent":{"action":"run_command","value":"/execute as @a run tellraw @a {\\"text\\":\\"hi\\"}"},"text":"Say hi"},{"text":"\\n\\n"},{"obfuscated":true,"hoverEvent":{"action":"show_item","contents":{"id":"minecraft:stone"}},"text":"Block"},{"text":"\\n\\n"},{"bold":true,"text":"theEnd2"},{"text":"\\n\\n"},{"hoverEvent":{"action":"show_text","contents":{"text":"tooltip"}},"text":"tooltip"},{"text":"\\n\\n\\n\\n\\n "}],"text":""}', '{"extra":[{"text":"Page 2\\n\\nstats\\n\\n"},{"keybind":"key.forward"},{"text":"\\n"},{"keybind":"key.use"},{"text":"\\n"},{"insertion":"Player236","clickEvent":{"action":"suggest_command","value":"/tell Player236 "},"hoverEvent":{"action":"show_entity","contents":{"type":"minecraft:player","id":"4e460400-9f4b-3a1c-89fb-10907a37bce7","name":{"text":"Player236"}}},"text":"Player236"},{"text":"\\n\\n"},{"text":""}],"text":""}'], title: "Mon livre", author: "http://minecraft.tools/"}

diskree commented 10 months ago

I tested this command on the stable 1.20.4 branch and also in vanilla. show_entity from this command does not work

diskree commented 10 months ago

I don’t understand what this is, since I’m not very good in commands. But the overlay display from hoverEvent should be displayed correctly like this (vanilla render is not changed, only shifted to the center of the screen) image

KosmoMoustache commented 10 months ago

Vanilla, latest snapshot image

With the mod in 1.20.4 image

diskree commented 10 months ago

I checked everything again and this particular tooltip is not displayed anywhere...

diskree commented 10 months ago

I'm not sure, but maybe it's because Minecraft doesn't display tooltip about unknown players for current server? That's why I don't see it for Player236 and you don't see because you switched between players during testing. 🤔 I really don’t understand how this can be related to the mod, it looks very much like command issue, because for the code it doesn’t matter which tooltip to display. Everything related to rendering these tooltips is here:

https://github.com/diskree/FixBookGUI/blob/e9e59ac26df3d172efb1fc2e37ea4e5e34587947/src/main/java/net/kosmo/fixbookgui/mixins/MixinBookScreen.java#L114

KosmoMoustache commented 10 months ago

I don't know its weird. Can you add yourself to the contributors inside fabric.mod.json before I merge

diskree commented 10 months ago

Ok, but if the problem is still confirmed and will be reproduces, I will try to find a solution

diskree commented 10 months ago

Can you add yourself to the contributors inside fabric.mod.json

Yes, 1 min

KosmoMoustache commented 10 months ago

I will push a new update tomorrow

diskree commented 10 months ago

Nice!