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

Map the RedstoneConstants class #2961

Open
wants to merge 2 commits into
base: 1.19.3
Choose a base branch
from

Conversation

haykam821
Copy link
Contributor

This is likely to be a class holding redstone constants for the following reasons:

  • Redstone is one of the two major mechanics that could be considered large enough to have its own constants class, the other being light
  • The LightingProvider#field_31713 field already contains a max light constant
  • Light does not have a prevalent concept of a default level, unlike block state properties
  • The package is ordered between portals and state

@haykam821 haykam821 requested a review from a team January 17, 2022 02:41
@haykam821 haykam821 added javadoc A PR that adds or refactors javadoc. new A PR that maps mainly new names release A PR that targets a release version of Minecraft labels Jan 17, 2022
@Shnupbups Shnupbups requested a review from a team January 17, 2022 05:16
@liach
Copy link
Contributor

liach commented Jan 17, 2022

again, why wouldn't this be light constants (min level, max level, default block level) or chunk (size, offset, etc) constants?

@haykam821 haykam821 changed the base branch from 1.18.1 to 22w03a January 19, 2022 17:50
@haykam821 haykam821 added snapshot A PR that targets a snapshot version of Minecraft and removed release A PR that targets a release version of Minecraft labels Jan 19, 2022
@haykam821 haykam821 changed the base branch from 22w03a to 22w05a February 2, 2022 21:31
@haykam821 haykam821 changed the base branch from 22w05a to 22w07a February 18, 2022 14:44
@haykam821 haykam821 changed the base branch from 22w07a to 1.18.2-pre1 February 18, 2022 17:18
@@ -1,7 +0,0 @@
CLASS net/minecraft/class_6148
Copy link
Member

Choose a reason for hiding this comment

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

I think this should just be moved into the existing unused package next to where all of the package info's live.

Copy link
Contributor Author

@haykam821 haykam821 Feb 22, 2022

Choose a reason for hiding this comment

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

Should the class keep these new mappings as well?

Copy link
Contributor

@liach liach Mar 3, 2022

Choose a reason for hiding this comment

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

Unless there is definitive proof that these are and only are constants for redstone values (also two of the fields have same value, dunno how you can tell them apart), they should not keep the mappings and the class name you give it.

Maybe something like the original https://github.com/liach/yarn/blob/83ec0c0aff1f3a94693e4333eb4ba04e6e490878/mappings/net/minecraft/unused/UnknownWorldConstants6148.mapping

@haykam821 haykam821 changed the base branch from 1.18.2-pre1 to 1.18.2-pre3 February 23, 2022 20:29
@enbrain enbrain added the update-base Add this label to a pull request to automatically change the target branch to the default branch. label Mar 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2022

🚀 Target branch has been updated to 1.18.2

@github-actions github-actions bot changed the base branch from 1.18.2-pre3 to 1.18.2 March 3, 2022 03:24
@github-actions github-actions bot added release A PR that targets a release version of Minecraft and removed snapshot A PR that targets a snapshot version of Minecraft update-base Add this label to a pull request to automatically change the target branch to the default branch. labels Mar 3, 2022
@haykam821 haykam821 changed the base branch from 1.18.2 to 22w12a March 25, 2022 16:05
@haykam821 haykam821 changed the base branch from 22w12a to 22w15a April 20, 2022 04:18
@haykam821 haykam821 changed the base branch from 22w15a to 1.19-pre2 May 23, 2022 17:42
@haykam821 haykam821 added snapshot A PR that targets a snapshot version of Minecraft and removed release A PR that targets a release version of Minecraft labels May 23, 2022
@haykam821 haykam821 changed the base branch from 1.19-pre2 to 1.19-pre4 May 30, 2022 17:03
@haykam821 haykam821 changed the base branch from 1.19-pre4 to 1.19-pre5 June 1, 2022 18:17
@haykam821 haykam821 changed the base branch from 1.19-pre5 to 1.19 June 14, 2022 03:28
@haykam821 haykam821 added release A PR that targets a release version of Minecraft and removed snapshot A PR that targets a snapshot version of Minecraft labels Jun 14, 2022
@haykam821 haykam821 changed the base branch from 1.19 to 1.19.1 July 28, 2022 17:48
@haykam821 haykam821 changed the base branch from 1.19.1 to 1.19.3 December 28, 2022 03:13
@gudenau
Copy link
Contributor

gudenau commented Apr 30, 2023

I don't have an issue with this myself, it seems fairly logical and changing this stuff in the future will only break source compatibility due to constant inlining.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javadoc A PR that adds or refactors javadoc. new A PR that maps mainly new names release A PR that targets a release version of Minecraft
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants