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

Adding Trees, Treeblocks, and Signs #596

Merged
merged 53 commits into from
Apr 13, 2024

Conversation

glubtier
Copy link
Member

@glubtier glubtier commented Apr 7, 2024

Signs: Probably the biggest addition here. Required the creation of several classes. Of those, I was unsure if I put some in the right place-
entity.MSBlockEntities - Actually not sure if there already was a class that serves the same purpose? But I couldn't find it.
util.MSWoodTypes - Necessary for signs but not sure if it's better put in util or data or somewhere else.

MSWoodTypes: These are not threadsafe and need to go in enqueueWork, so I added them to mainThreadSetup(). It looks kind of clunky but this is fairly standard implementation, based on other mod repos I've poked around. I think there are better ways, but that's the kind of work I'd like to save for another post-4/13 PR, unless someone is chomping at the bit to take it on now.

End log: I'm not entirely sure what the design intent with the "double log" block was, but in the interest of not reinventing the wheel, I matched the stripped variant to this, and added a third texture for both log and stripped log blocks. (Lovingly named end_log_triple :) For now.) After 4/13, I plan on giving End lands a facelift like we're doing with some of the other lands.

Also as such, the model, blockstate, and item files for the stripped_end_log are hard-coded, like the end_log already is.

MSCreativeTabs: I tried to put all the related tree blocks together, in a similar order to the way vanilla tree blocks are ordered.

MSBlockStateProvider: small change to how the render type is applied to doors.

MSBlockSetType: I added all the woods, as well as all the stones that I could sus out, but I may have missed some. I did not update the BlockSetTypes for anything but the trees/woods I was working on.

Texture Credits:

  • Space aspect carved planks texture by Riotmode and edited by glubtier
  • Void aspect carved planks texture by Riotmode and edited by glubtier
  • Other new textures by glubtier
  • Texture edits by glubtier

(see also: https://docs.google.com/document/d/1MON4wpOaZjSfU_PFoJ_unIyOCggCKknxfZ96HZTj4x0/edit?usp=sharing)

# Conflicts:
#	src/main/generated/resources/assets/minestuck/lang/en_us.json
# Conflicts:
#	src/main/generated/resources/data/minecraft/tags/blocks/wooden_fences.json
#	src/main/java/com/mraof/minestuck/block/MSBlocks.java
#	src/main/java/com/mraof/minestuck/data/MSBlockStateProvider.java
#	src/main/java/com/mraof/minestuck/data/MinestuckEnUsLanguageProvider.java
#	src/main/java/com/mraof/minestuck/data/loot_table/MSBlockLootTables.java
#	src/main/java/com/mraof/minestuck/data/tag/MinestuckBlockTagsProvider.java
@kirderf1 kirderf1 added the enhancement New feature or request label Apr 7, 2024
@kirderf1
Copy link
Member

kirderf1 commented Apr 7, 2024

Changelog is also a thing. I assume that additions won't be a problem, but we should take extra care with changes (textures, models and the like) so that we catch changes to preexisting blocks (blocks that have already been released, like frost leaves and shadewood).

@kirderf1 kirderf1 added the missing credit The author of some asset need to be made clear label Apr 7, 2024
Copy link
Member

@Dweblenod Dweblenod left a comment

Choose a reason for hiding this comment

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

Looks like some useful changes and additions! Its understandable since you havent had as much experience with our codebase before but there is some classes you added which are derivative of existing code. I pointed out such locations I identified in their own comments.

It also looks like you merged content from the 1.20.1 branch, trying to push those changes to upcoming-content is not the best thing to do here. We can discuss how to remedy that on discord!

@kirderf1
Copy link
Member

kirderf1 commented Apr 7, 2024

It also looks like you merged content from the 1.20.1 branch, trying to push those changes to upcoming-content is not the best thing to do here. We can discuss how to remedy that on discord!

It doesn't matter much in this occurrence. Everything in 1.20.1 already exists in upcoming-content. I imagine that the changes from 1.20.1 will disappear from the diff once the merge conflicts with upcoming-content has been resolved.

@glubtier
Copy link
Member Author

glubtier commented Apr 7, 2024

Changelog is also a thing. I assume that additions won't be a problem, but we should take extra care with changes (textures, models and the like) so that we catch changes to preexisting blocks (blocks that have already been released, like frost leaves and shadewood).

Yep! Once I'm actually done with textures and know what is and isn't going in, then I have to go through them all. I'll update the changelog and credits after that.

@glubtier
Copy link
Member Author

glubtier commented Apr 7, 2024

It also looks like you merged content from the 1.20.1 branch, trying to push those changes to upcoming-content is not the best thing to do here. We can discuss how to remedy that on discord!

It doesn't matter much in this occurrence. Everything in 1.20.1 already exists in upcoming-content. I imagine that the changes from 1.20.1 will disappear from the diff once the merge conflicts with upcoming-content has been resolved.

This might also be because I had some file issues on my PC and then some merging shenanigans in my own repo. Future PRs shouldn't have any issue but if you have any tips, I'm happy to hear them. GitHub (the site itself) doesn't always make sense to me.

# Conflicts:
#	src/main/generated/resources/assets/minestuck/lang/en_us.json
#	src/main/java/com/mraof/minestuck/data/MinestuckEnUsLanguageProvider.java
#	src/main/java/com/mraof/minestuck/data/loot_table/MSBlockLootTables.java
#	src/main/java/com/mraof/minestuck/data/recipe/MinestuckRecipeProvider.java
#	src/main/java/com/mraof/minestuck/item/MSItems.java
@glubtier
Copy link
Member Author

@kirderf1 Credits! https://docs.google.com/document/d/1MON4wpOaZjSfU_PFoJ_unIyOCggCKknxfZ96HZTj4x0/edit?usp=sharing

I tried to note down which textures are new, which ones were modified, which ones were repurposed, etc. I don't care about credits for anything I color-corrected or did some light reshading/retouching, I just included them to be thorough. If I missed something or something is unclear, please DM me!

@Dweblenod Dweblenod marked this pull request as ready for review April 13, 2024 02:45
@Dweblenod Dweblenod removed the missing credit The author of some asset need to be made clear label Apr 13, 2024
@glubtier
Copy link
Member Author

As of now, everything I intended to do is done, short of any fixes or merge conflicts.

# Conflicts:
#	CHANGELOG.md
#	src/main/java/com/mraof/minestuck/world/lands/terrain/RainLandType.java
@kirderf1 kirderf1 merged commit 745e36e into lunar-sway:upcoming-content Apr 13, 2024
2 checks passed
@glubtier glubtier deleted the Signs branch September 30, 2024 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants