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.

Sign up for GitHub

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

Jump to bottom

Item list things and forge!!!!!! #948

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Item list things and forge!!!!!! #948

wants to merge 10 commits into from

Conversation

viciscat
Copy link
Collaborator

@viciscat viciscat commented Aug 20, 2024 •
edited
Loading

  • Make recipes mod independent
  • Forge yipee
  • Did not do JEI cuz what in the hell is hapenning in there

There's probably some cleaning to do but i'm so done with this gaaahhh

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Aug 20, 2024
@AzureAaron AzureAaron added the new feature This issue or PR is a new feature label Aug 22, 2024
@AzureAaron AzureAaron added this to the 1.23.0 milestone Aug 22, 2024
kevinthegreat1
kevinthegreat1 requested changes Sep 8, 2024
View reviewed changes
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.

I added forge duration with 203d4a2. You might want to look at that. Also what's going on with CreeperEntityAccessor? Other than that looks good.

build.gradle Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/mixins/CreeperEntityAccessor.java Outdated Show resolved Hide resolved
src/main/resources/skyblocker.mixins.json Outdated Show resolved Hide resolved
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed reviews needed This PR needs reviews labels Sep 8, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels Sep 17, 2024
kevinthegreat1
kevinthegreat1 previously approved these changes Sep 17, 2024
View reviewed changes
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.

Not tested.

@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Sep 17, 2024
@AzureAaron AzureAaron removed the merge me please Pull requests that are ready to merge label Oct 12, 2024
@AzureAaron
Copy link
Collaborator

Removed merge me label because half of this just won't work with 1.21.2.

@kevinthegreat1 kevinthegreat1 added the merge conflicts This PR has merge conflicts that need solving. label Oct 30, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge conflicts This PR has merge conflicts that need solving. labels Nov 30, 2024
@LifeIsAParadox LifeIsAParadox added merge conflicts This PR has merge conflicts that need solving. and removed reviews needed This PR needs reviews labels Dec 22, 2024
@Emirlol
Copy link
Collaborator

Emirlol commented Dec 23, 2024

Why do REI and recipe book use furnace to represent forge category while EMI uses a lava bucket?

Recipe book:

https://github.com/viciscat/Skyblocker_vic/blob/2fe34c196827827e82bff1eefab65114b6e7489d/src/main/java/de/hysky/skyblocker/skyblock/itemlist/recipebook/SkyblockRecipeResults.java#L250

REI:

https://github.com/viciscat/Skyblocker_vic/blob/2fe34c196827827e82bff1eefab65114b6e7489d/src/main/java/de/hysky/skyblocker/compatibility/rei/SkyblockerREIClientPlugin.java#L29

EMI:

https://github.com/viciscat/Skyblocker_vic/blob/2fe34c196827827e82bff1eefab65114b6e7489d/src/main/java/de/hysky/skyblocker/compatibility/emi/SkyblockerEMIPlugin.java#L26
https://github.com/viciscat/Skyblocker_vic/blob/2fe34c196827827e82bff1eefab65114b6e7489d/src/main/java/de/hysky/skyblocker/compatibility/emi/SkyblockerEMIPlugin.java#L43

Emirlol
Emirlol reviewed Dec 23, 2024
View reviewed changes
src/main/java/de/hysky/skyblocker/skyblock/itemlist/recipes/SkyblockForgeRecipe.java

@Override
public Text getExtraText() {
return Text.empty();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Forge items can have requirements too, this could return the craft text like SkyblockCraftingRecipe does.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not in neu repo parser, opened a pr

@viciscat
Merge branch 'master' into item-list-things
8634b40 
# Conflicts:
#	src/main/java/de/hysky/skyblocker/compatibility/emi/SkyblockerEMIPlugin.java
#	src/main/java/de/hysky/skyblocker/compatibility/jei/SkyblockerJEIPlugin.java
#	src/main/java/de/hysky/skyblocker/compatibility/rei/SkyblockCraftingDisplayGenerator.java
#	src/main/java/de/hysky/skyblocker/compatibility/rei/SkyblockerREIClientPlugin.java
#	src/main/resources/assets/skyblocker/lang/en_us.json
@viciscat
requested changes and uh stuff
e87e4b3 
ye
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews merge conflicts This PR has merge conflicts that need solving. and removed merge conflicts This PR has merge conflicts that need solving. reviews needed This PR needs reviews labels Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Emirlol Emirlol Emirlol left review comments

@kevinthegreat1 kevinthegreat1 kevinthegreat1 left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
merge conflicts This PR has merge conflicts that need solving. new feature This issue or PR is a new feature
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.
5 participants
@viciscat @AzureAaron @Emirlol @kevinthegreat1 @LifeIsAParadox