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

[1.14.x] Rewrite docs for Registries #297

Merged
merged 14 commits into from Jul 19, 2020

Conversation

sciwhiz12
Copy link
Contributor

@sciwhiz12 sciwhiz12 commented May 23, 2020

Rewrote [Concepts > Registries]:

  • Removed references to GameRegistry.register
  • Reordered register methods so DeferredRegister is first and foremost
  • Simplified section on creating registries (if someone will make their own registry, they'll be capable of researching on their own)
  • Changed ObjectHolder rules from block of text to rules list (TIL that final only matters if the class and not the field has @ObjectHolder)

Issue #275 can be closed, and I think the related issue #85 can be closed, since the issue is from long, long ago (2017), and both points are resolved:

  1. GameRegistry.register in preInit is non-existent at this point; and
  2. RegistryBuilder is only used when a modder wants to adds a registry, which is not common I would assume.

docs/concepts/registries.md Outdated Show resolved Hide resolved
docs/concepts/registries.md Outdated Show resolved Hide resolved
docs/concepts/registries.md Outdated Show resolved Hide resolved
docs/concepts/registries.md Outdated Show resolved Hide resolved
docs/concepts/registries.md Show resolved Hide resolved
docs/concepts/registries.md Outdated Show resolved Hide resolved
docs/concepts/registries.md Outdated Show resolved Hide resolved
docs/concepts/registries.md Outdated Show resolved Hide resolved
docs/concepts/registries.md Outdated Show resolved Hide resolved
docs/concepts/registries.md Show resolved Hide resolved
@Cyborgmas
Copy link
Contributor

first review, and it seems that i do not have the hang of it. Got most stuff twice in each... sorry for the hassle

Aeltumn
Aeltumn previously approved these changes Jun 20, 2020
Copy link
Contributor

@Aeltumn Aeltumn left a comment

Choose a reason for hiding this comment

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

Everything looks pretty good, I've only got two small comments. Although I've never used the @ ObjectHolder so I can't give any feedback on that part.

docs/concepts/registries.md Outdated Show resolved Hide resolved
docs/concepts/registries.md Outdated Show resolved Hide resolved
@sciwhiz12 sciwhiz12 changed the title Rewrite docs for Registries [1.14.x] Rewrite docs for Registries Jun 30, 2020
@sciwhiz12
Copy link
Contributor Author

As 1.14.x is moving into out-of-support, I am closing this PR. See the updated PR for 1.15.x.

@sciwhiz12 sciwhiz12 closed this Jun 30, 2020
@sciwhiz12 sciwhiz12 deleted the registries branch June 30, 2020 18:43
@sciwhiz12 sciwhiz12 restored the registries branch July 19, 2020 11:51
@sciwhiz12
Copy link
Contributor Author

As said to me, 1.14.x is the branch to currently target. Since 1.14.x is essentially the same to 1.15.x in regards to registries, the reviews from the other branch will apply to this branch.

@sciwhiz12 sciwhiz12 reopened this Jul 19, 2020
@sciwhiz12
Copy link
Contributor Author

I might have to make a PR once this is merged into 1.15.x, to change the section on DeferredRegister from using the constructor to #create.

docs/concepts/registries.md Outdated Show resolved Hide resolved
@tterrag1098 tterrag1098 added 1.14 For Minecraft 1.14.x Out of Date Information A page contains information which is outdated or incorrect Page Improvement Improves an existing page labels Jul 19, 2020
@tterrag1098 tterrag1098 added the Waiting for Changes PR is waiting on requested changes from author label Jul 19, 2020
@tterrag1098 tterrag1098 merged commit fd1f174 into MinecraftForge:1.14.x Jul 19, 2020
@sciwhiz12 sciwhiz12 deleted the registries branch March 8, 2021 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.14 For Minecraft 1.14.x Out of Date Information A page contains information which is outdated or incorrect Page Improvement Improves an existing page Waiting for Changes PR is waiting on requested changes from author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants