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.

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

Add enchants for Enchant Parsing #46

Merged
merged 3 commits into from
Mar 9, 2024
Merged

Conversation

VixidDev
Copy link
Contributor

Repo file for all enchants for Enchant Parsing (will make pr in mod soon). Includes the new ones SBA doesn't have

@jani270
Copy link
Collaborator

jani270 commented Oct 26, 2023

bruh

@VixidDev
Copy link
Contributor Author

bruh

why bruh?

@jani270
Copy link
Collaborator

jani270 commented Oct 26, 2023

Dont see why this is needed, we already have enchant colors in NEU, so good and max level is not needed.

@VixidDev
Copy link
Contributor Author

Dont see why this is needed, we already have enchant colors in NEU, so good and max level is not needed.

It's based on SBA's enchant parsing which colors them based on how good the level of the enchant is, which is different for man y enchants. With NEU you can only color them if they are above, below or equal to a level that matches a certain regex, i.e., 5 isn't max for Power but it is for Charm for example, with NEU you would end up having those be the same color, unless you manually recolor every enchant manually or try to group some with regex rules which is not really viable.

I and many others prefer SBA method of coloring the enchants. If hanni and others really don't want this feature and don't see the need then sure I guess I'll remove it. But I right now I see it as sufficiently different from NEU's one (and better IMO)

@jani270
Copy link
Collaborator

jani270 commented Oct 26, 2023

Or you can just download a preset, which already does that for you. This is just duping a feature and not making it better, but thats does not matter for this pr. I just dislike it really hard. Another thing is that I havent seen one person that prefers SBA enchant colors over neu's

@VixidDev
Copy link
Contributor Author

Why go through the effort of finding a preset that probably has countless rules which someone may or may not have yet made. Then when you want to change a color of 1 or more enchants you have to scroll through to find them then change the color of each individually, and possibly the level of said enchant if Hypixel decide to add a new level to an enchant.

Whereas with this one you can just change the color of the max level enchants, great level enchants, good level enchants, and poor level enchants with relative ease. I wouldn't say its duping a feature, rather a (better) alternative. Also, seriously? You haven't seen one person that prefers SBA's one over NEU? Let me be the first I guess.

@jani270
Copy link
Collaborator

jani270 commented Oct 26, 2023

I think we should wait for hannibals opinion in this, for me its a FAR FAR worse alternative. Instead i would just add a button to neu to maxout enchants.

@hannibal002
Copy link
Owner

While i think it's true that most users only care for hyper maxed/super good /descent/bad enchantments color and ignoring ultimate enchants, i do like the endless possibilities of neu's custom settings. I don't know how useful a preset would be, since it feels a bit static when changes happen.

@Obsidianninja11
Copy link
Contributor

Obsidianninja11 commented Dec 13, 2023

Sba's enchant parsing does more than just change enchant colors
image

enchant parsing off
2023-12-13_00 03 17

on with compress display
2023-12-13_00 03 42

on with normal display
2023-12-13_00 03 57

on with expand display
2023-12-13_00 04 11

@qtlunya
Copy link
Contributor

qtlunya commented Dec 13, 2023

You should add Pesterminator btw, in case this ends up getting merged. I guess maybe good level can be like 3, I saw a lot of people with that. Max is 5.

@VixidDev
Copy link
Contributor Author

You should add Pesterminator btw, in case this ends up getting merged. I guess maybe good level can be like 3, I saw a lot of people with that. Max is 5.

Yeah, there have been a few enchants that have been added to the game since this PR, but I haven't been able to get around to it since I have been unbelievably busy with uni work, if I manage to find time in the next week I'll probably add the missing ones. Also thanks for the suggestion on the good level since I have no idea about any of the new enchants 😅.

Sba's enchant parsing does more than just change enchant colors-

Correct, at the moment my enchant parsing forces compressed at the moment (3 enchants a line), the feature is not as exhaustive as sba's (yet). I've not been able to go over the code for my enchant parsing since I made the pull request, since as mentioned above, I have been extremely busy, and I wanted to re-write some of the code for it since I feel it could be improved a bit. With the exception of waiting until the missing enchants have been added to this PR, it should be (the PR on the main repo) in a state to be merged. I do plan to add the three options of formatting like in sba but don't have the time right now. (Also I think your comment may have been more relevant on the main repo than here)

@hannibal002 hannibal002 merged commit 91106c2 into hannibal002:main Mar 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants