mschae23 / grind-enchantments

Put enchantments back on a book using a grindstone.
https://legacy.curseforge.com/minecraft/mc-mods/grind-enchantments
GNU Lesser General Public License v3.0
12 stars 13 forks source link

Suggestion: Other ways to see the cost #23

Closed Harvle closed 2 years ago

Harvle commented 2 years ago

If people don't use this mod on the client side (i have a server which uses server-side mods only), currently it's impossible to see the cost. I have thought up a few ways this could be achieved:

Obviously, the current way is the best, so if the player has the mod, it'd use that and ignore these, otherwise there would be a config line to say which one to use, i.e no_mod_cost_display: "chatmessage|guiname|resultlore|actionbar" (can't think of a good key name).

Ideally, this text lines would be configurable as well.

Thanks for taking time to read this! :)

mschae23 commented 2 years ago

Thank you for the suggestions, this is very helpful. There is also another issue that arises if you're not using the mod on the client side. If you don't have enough XP, you can still take the item out of the result slot, but it will be a ghost item that disappears when the inventory is closed.

I'm not sure if there is a way to see if a player has the mod installed (I would have to look into that), but if it isn't, using a config option will still be possible, it would just apply to all players.

Sending a message in chat is possible, but I think the background is darker when you're in a GUI screen, so you could miss the message. Changing the name of the screen is not possible I believe, due to that being done on the client side and taken from a translation file. I think adding the cost as lore is the best option here.

Harvle commented 2 years ago

(Not sure if this is of any use) Carpet detects if the mod exists on the client, however it appears to use some kind of "handshake" that the client mod sends to the server on join, seems out-of-scope for this mod to add something like that.

If you can't find a way to do it, could you do something more simple by just checking the client's version to see if the user logged in with a Fabric client, then just assume they have the mod? Obviously it's not a great solution, but just an idea! I think some anti-cheat mods/plugins use this to detect if a player is using vanilla, fabric, forge, optifine, etc, i'm not sure if this method also supports looking at the individual installed fabric mods though.

I agree item lore would suit the mod best, hopefully you're able to figure something out with it. :)

mschae23 commented 2 years ago

(Not sure if this is of any use) Carpet detects if the mod exists on the client, however it appears to use some kind of "handshake" that the client mod sends to the server on join, seems out-of-scope for this mod to add something like that.

From what I've seen, it should be possible to do this by letting the client send a packet to the server which effectively disables the alternative cost display for that player. This would mean adding a dependency on Fabric API (which is currently not required), but I may try doing this in the future. For now, a simple config option should be fine, but would apply to all players on the server.

could you do something more simple by just checking the client's version to see if the user logged in with a Fabric client, then just assume they have the mod?

I'm not sure if Fabric actually sends the client brand when joining a server.

I agree item lore would suit the mod best, hopefully you're able to figure something out with it. :)

I'll try!

mschae23 commented 2 years ago

So, I actually got this working with item stack lore, and it is also colored green or red depending on whether the player has enough XP (just like the text that is displayed if you have the mod installed).

From my testing, the issue that you can still take the item if you don't have enough XP (which will be a ghost item that disappears when the inventory is synced) didn't seem to appear, but I can't guarantee that.

This behavior is disabled by default, and can be enabled by adding an option in the config for now (which also means that players who have the mod will see the enchantment cost twice, but this shouldn't be a major problem).

Harvle commented 2 years ago

Hi, sorry for the late response, been busy, but well done for figuring this out! That sounds great, thanks for taking the time to implement this, looking forward to using it! :)

Harvle commented 2 years ago

Just started using it, works great. In console there's a "Adding level cost lore" message every time an item has the lore applied, is this intentional (seems like a debug message)?

mschae23 commented 2 years ago

Just started using it, works great. In console there's a "Adding level cost lore" message every time an item has the lore applied, is this intentional (seems like a debug message)?

Yes, it's a debug message, looks like I forgot about it. I'll remove it in the next release.