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

Make the crafting UI look like the vanilla crafting table #599

Merged
merged 5 commits into from Apr 4, 2024

Conversation

viciscat
Copy link
Collaborator

Self explanatory ain't it?

High Res Images

image

image

@AzureAaron AzureAaron added new feature This issue or PR is a new feature reviews needed This PR needs reviews labels Mar 16, 2024
Copy link
Collaborator

@kevinthegreat1 kevinthegreat1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, some naming convensions, and needs testing.

@x-yingcan-x
Copy link

pls remove the quick craft in texture, we all know what it is and this will only make it worse in I18N support (hard to translate)

@viciscat
Copy link
Collaborator Author

viciscat commented Mar 17, 2024

pls remove the quick craft in texture, we all know what it is and this will only make it worse in I18N support (hard to translate)

Not in the texture, however the string is hard coded. Imma change that

@Julienraptor01
Copy link
Contributor

Julienraptor01 commented Mar 17, 2024

pls remove the quick craft in texture, we all know what it is and this will only make it worse in I18N support (hard to translate)

I kinda agree with the fact the quick craft line is kinda bad ?
How does Hypixel communicate that these slots are quick-craft ?
If they don't put any mention we probably should neither

In French it would have to be written as Fabrication Rapide, which obv is way too long
My PoV is that it should stay hard-coded in english at worst and be removed entirely at best

image
image

the game show the things as-is, we should just show the same tooltip that you would when the slot is empty and hovered

the game should also show the tooltip when the + button is hovered

@viciscat
Copy link
Collaborator Author

tooltip is already there cuz it literally just displays the items in that slot. So I'm gonna yeet the string

@Julienraptor01
Copy link
Contributor

Julienraptor01 commented Mar 17, 2024

tooltip is already there cuz it literally just displays the items in that slot. So I'm gonna yeet the string

it displays the glass panes ?
and also display the barrier item ?
would there be a way to show these as empty, but still show the tooltips
and also obv make sure that putting items in those slots behave as vanilla / is impossible (which is probably easier to do)
did you prevent the players from even switching items from the cursor slot to the output slot ?

@viciscat
Copy link
Collaborator Author

viciscat commented Mar 17, 2024

I made barrier not render cuz it looked weird, but glass panes still render. They don't look too out of place

EDIT: did not prevent clicking on output slot aka barrier, and base skyblocker prevents clicking on black stained glass panes

@Julienraptor01
Copy link
Contributor

Julienraptor01 commented Mar 17, 2024

I made barrier not render cuz it looked weird, but glass panes still render. They don't look too out of place

EDIT: did not prevent clicking on output slot aka barrier, and base skyblocker prevents clicking on black stained glass panes

actually preventing to click could go wrong as if you click with the same item, it does things, so not preventing the output one is fine
can you show an example with quickcraft slots with panes ?
does the more button also show the tooltips of its item in game ?

@viciscat
Copy link
Collaborator Author

test it yourself

Here's when ya got no crafts

In spoiler cuz these images real big

image

Simple hover for More Crafts
image

@Julienraptor01
Copy link
Contributor

test it yourself

Here's when ya got no crafts

In spoiler cuz these images real big

image

Simple hover for More Crafts
image

Is the more craft tooltip hardcoded ?
Or do you extract info from the tooltips of the + head item ?

@viciscat
Copy link
Collaborator Author

hard coded, I can try and get the original tooltip

@AzureAaron AzureAaron added changes requested This PR need changes reviews needed This PR needs reviews and removed reviews needed This PR needs reviews changes requested This PR need changes labels Mar 17, 2024
Copy link
Collaborator

@kevinthegreat1 kevinthegreat1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to add config. Also this doesn't work if betterPartyFinder is turned off. See HandledScreenProviderMixin.

@kevinthegreat1 kevinthegreat1 added changes requested This PR need changes and removed reviews needed This PR needs reviews labels Apr 1, 2024
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Apr 3, 2024
@kevinthegreat1 kevinthegreat1 merged commit 97a5b8a into SkyblockerMod:master Apr 4, 2024
1 check passed
@LifeIsAParadox LifeIsAParadox removed the merge me please Pull requests that are ready to merge label Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This issue or PR is a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants