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

Explain blocks and blockstates in more depth #21

Merged
merged 31 commits into from
Nov 25, 2023

Conversation

IchHabeHunger54
Copy link
Member

@IchHabeHunger54 IchHabeHunger54 commented Oct 17, 2023

This PR improves and expands on the explanation of blocks and blockstates. More specifically, it:

  • Adds more information on how every block and every blockstates exists in memory once, and why calling new Block() is a bad idea
  • Adds more information on when to use blocks, when to use blockstates, and when to use block entities
  • Adds a brief explanation of blockstate JSONs and block model JSONs, with references to the Resources section
  • Adds information about how most methods expect a BlockState rather than a Block, and why that is the case
  • Adds more information about BlockState's most important usecases, specifically BlockState#getProperty, BlockState#setProperty, Level#getBlockState and Level#setBlock
  • Adds information on the update flags for Level#setBlock
  • Removes the historical section about block metadata, as it is irrelevant to the average modder
  • Removes the disclaimer about optional registration, as that is better suited for the registration section
  • Rewords some sections and fixes some grammar and spelling issues

Preview URL: https://pr-21.neoforged-docs-previews.pages.dev

@neoforged-pages-deployments neoforged-pages-deployments bot deployed to neoforged-docs-previews (Preview) October 17, 2023 17:34 Active
@neoforged-pages-deployments
Copy link

neoforged-pages-deployments bot commented Oct 17, 2023

Deploying with Cloudflare Pages

Name Result
Last commit: a8b4c53e22a511c2a491b4abfbd71d7b63aeb385
Status: ✅ Deploy successful!
Preview URL: https://b248ddeb.neoforged-docs-previews.pages.dev
PR Preview URL: https://pr-21.neoforged-docs-previews.pages.dev

Copy link
Contributor

@ChampionAsh5357 ChampionAsh5357 left a comment

Choose a reason for hiding this comment

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

More explanations and examples are necessary. Also, if you are rewriting the page, use the notes in the new contribution guidelines.

docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/states.md Outdated Show resolved Hide resolved
docs/blocks/states.md Outdated Show resolved Hide resolved
docs/blocks/states.md Show resolved Hide resolved
docs/blocks/states.md Outdated Show resolved Hide resolved
docs/blocks/states.md Outdated Show resolved Hide resolved
@IchHabeHunger54
Copy link
Member Author

I was under the impression that we held to the old project's standard of not including example code in the docs. As I understand it, this has changed, so I will update the PR accordingly.

@ChampionAsh5357
Copy link
Contributor

Yeah, generally prefer pseudocode like java with comments over exact examples unless necessary. The guidelines should show an example

@neoforged-pages-deployments neoforged-pages-deployments bot deployed to neoforged-docs-previews (Preview) October 17, 2023 23:40 Active
@IchHabeHunger54
Copy link
Member Author

IchHabeHunger54 commented Oct 17, 2023

Rewrote major parts of it to consider the feedback, examples have been added. I also added a WIP method reference page that I will finish tomorrow (hopefully). Converting to draft for now.

@IchHabeHunger54 IchHabeHunger54 marked this pull request as draft October 17, 2023 23:42
@IchHabeHunger54
Copy link
Member Author

IchHabeHunger54 commented Oct 17, 2023

I also dismissed most of the feedback since I fixed most of it when rewriting ("fixed" from my point of view). If there are problems with it still, feel free to re-add feedback as needed.

@neoforged-pages-deployments neoforged-pages-deployments bot deployed to neoforged-docs-previews (Preview) October 18, 2023 13:49 Active
Copy link
Contributor

@ChampionAsh5357 ChampionAsh5357 left a comment

Choose a reason for hiding this comment

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

Wow, this has improved so much. In general, I think the detail is great, but there are instances where you are straight up defining the method. That's fine when comparing between vanilla and NeoForge, but I believe most of those descriptions should go into Parchment or NeoForge itself. The interaction workflow meanwhile, can probably stick around.

Anything else I still need to think on what makes sense in the docs. I'm now marking this as the issue to resolve the block portion of #20

docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/states.md Outdated Show resolved Hide resolved
docs/blocks/states.md Outdated Show resolved Hide resolved
docs/blocks/states.md Outdated Show resolved Hide resolved
docs/blocks/states.md Outdated Show resolved Hide resolved
docs/items/interactionpipeline.md Show resolved Hide resolved
IchHabeHunger54 and others added 2 commits October 22, 2023 12:35
Co-authored-by: ChampionAsh5357 <championash5357@gmail.com>
Co-authored-by: ChampionAsh5357 <championash5357@gmail.com>
@neoforged-pages-deployments neoforged-pages-deployments bot deployed to neoforged-docs-previews (Preview) October 22, 2023 10:37 Active
@neoforged-pages-deployments neoforged-pages-deployments bot deployed to neoforged-docs-previews (Preview) October 22, 2023 10:38 Active
Co-authored-by: ChampionAsh5357 <championash5357@gmail.com>
@neoforged-pages-deployments neoforged-pages-deployments bot deployed to neoforged-docs-previews (Preview) October 22, 2023 10:49 Active
@neoforged-pages-deployments neoforged-pages-deployments bot deployed to neoforged-docs-previews (Preview) October 22, 2023 10:56 Active
docs/blocks/states.md Outdated Show resolved Hide resolved
Co-authored-by: Dennis C <xfacthd@gmx.de>
@neoforged-pages-deployments neoforged-pages-deployments bot deployed to neoforged-docs-previews (Preview) November 22, 2023 14:39 Active
XFactHD
XFactHD previously approved these changes Nov 22, 2023
docs/blocks/index.md Outdated Show resolved Hide resolved
docs/blocks/index.md Outdated Show resolved Hide resolved
Co-authored-by: Dennis C <xfacthd@gmx.de>
@neoforged-pages-deployments neoforged-pages-deployments bot deployed to neoforged-docs-previews (Preview) November 22, 2023 20:43 Active
@Matyrobbrt Matyrobbrt changed the title explain blocks and blockstates in more depth Explain blocks and blockstates in more depth Nov 23, 2023
Matyrobbrt
Matyrobbrt previously approved these changes Nov 23, 2023
Copy link
Member

@XFactHD XFactHD left a comment

Choose a reason for hiding this comment

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

Final thing I missed yesterday, finally good to go from me when that's fixed

docs/blocks/index.md Outdated Show resolved Hide resolved
Co-authored-by: Dennis C <xfacthd@gmx.de>
Copy link
Contributor

@ChampionAsh5357 ChampionAsh5357 left a comment

Choose a reason for hiding this comment

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

Looks good. Will try to merge on phone.

@ChampionAsh5357 ChampionAsh5357 merged commit 346c3a6 into neoforged:main Nov 25, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants