Skip to content

Fix wood recipe - #523

Closed
Bawnorton wants to merge 97 commits into
FallingColors:mainfrom
Bawnorton:patch-wood-recipe
Closed

Fix wood recipe#523
Bawnorton wants to merge 97 commits into
FallingColors:mainfrom
Bawnorton:patch-wood-recipe

Conversation

@Bawnorton

Copy link
Copy Markdown
Contributor
Screenshot 2023-08-31 at 12 17 58

I don't believe this recipe is intentional

Talia-12 and others added 30 commits June 15, 2023 15:34
…ls are gone, so block properties seem to be mostly made by copying from other blocks and changing relevant things. Data generation seems to have changed, and recipes need a recipe category.
…hich blocks can be removed by OpDestroyFluid.
…rge, committing this so I don't lose it in case I want to come back to it
@object-Object

Copy link
Copy Markdown
Member

hey, thanks for the PR! unfortunately this file is in .../generated/resources, which means it's generated automatically, so we need to update the code that generated it instead.

@Bawnorton

Copy link
Copy Markdown
Contributor Author

I totally forgot about this pr and yeah I see I missed that, I can amend the pr to change the generator instead.

@Bawnorton

Copy link
Copy Markdown
Contributor Author

I can't load this project locally because of various gradle issues so I didn't have IntelliSense and couldn't run the data-gen to check but it should be correct 😅

@object-Object object-Object added the needs reviewer We need to assign someone to review this PR label Nov 16, 2023
@SamsTheNerd

Copy link
Copy Markdown
Member

looks good to me too, although we should probably run it just to make sure it works first.

I think the edified wood variants should exist ? but don't currently. It looks like we don't have models for them either. Should be easy enough to add them in though, don't know if we want to do that as a part of this PR or separate, @object-Object ?

@object-Object object-Object removed the needs reviewer We need to assign someone to review this PR label Nov 16, 2023
@object-Object

Copy link
Copy Markdown
Member

@SamsTheNerd depends how much effort it takes / how big of a change it would be - I see EDIFIED_WOOD in a lot of places but i'm not sure if you'd need all those for the variants?
@Bawnorton are you ok with doing that? otherwise we can just merge it and PR the variants separately.

@Bawnorton

Copy link
Copy Markdown
Contributor Author

I can't get this repo setup locally, so it would be best if someone else did that if it were done at all.

@object-Object

object-Object commented Nov 16, 2023

Copy link
Copy Markdown
Member

(side note: i feel like the ideal solution would just be to do all of the edified wood variant stuff in one loop and/or based on one data source somewhere (an enum?), so we don't keep forgetting to add blocks for the variants, but idk how doable that is.)

@object-Object

Copy link
Copy Markdown
Member

ok - I'll run it locally, then assuming it works we can merge this and figure out the variants later.

@object-Object

object-Object commented Nov 17, 2023

Copy link
Copy Markdown
Member

Right... the reason you can't get it working locally is probably because the build is still broken on main. I'd also rather not merge into main right now since it'll probably make resolving the 1.20 merge conflicts even more of a hassle. @Bawnorton would you be able to rebase your PR onto the 1.20 branch? (or I can do it if you want)

@Bawnorton

Copy link
Copy Markdown
Contributor Author

rebased... probably better to just close this and start a new pr onto the 1.20 branch tbh lol

@object-Object

object-Object commented Nov 17, 2023

Copy link
Copy Markdown
Member

uh, yeah probably... that was not exactly the type of rebase i was talking about hah
up to you if you want to fix it or just throw it out and open a new one

(for posterity, the original rebase command should have been this: git rebase --onto 1.20 63014695)

@Bawnorton Bawnorton closed this Nov 17, 2023
@Bawnorton
Bawnorton deleted the patch-wood-recipe branch November 17, 2023 02:08
@Bawnorton

Copy link
Copy Markdown
Contributor Author

Yeah, my bad... I just realized I rebased the wrong way... git is hard sometimes 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants