Skip to content

Conversation

JurgenKuyper
Copy link
Collaborator

No description provided.

Copy link

@minimusubi minimusubi left a comment

Choose a reason for hiding this comment

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

just a few thoughts:

  • looks like your changes for #4064 are here too. however, this wont matter if 4064 is merged first
  • this change assumes all 1.17+ worlds are 363 height- the end and nether are not. though i don't think it will error, it might be cleaner to avoid reading the extra color values
  • this also wont work for any worlds with larger custom heights (achievable through data packs or mods)
    • if it works the way I assume it does, it may be better to programmatically read the height values instead of using magic numbers (for example, from MapIterator.getWorldHeight() and MapIterator.getWorldYMin()). this also future-proofs the code for any future world height changes

fillcolor = new Color[worldheight]; /* Color by Y, must be range of total world height, offset by +64*/
/* Load defined colors from parameters */
for(int i = 0; i < worldheight; i++) {
fillcolor[i] = configuration.getColor("color" + (i - 64), null); /* need to substract by 64 because Color does not accept <0 indexes*/

Choose a reason for hiding this comment

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

this wont work as expected on nether and end worlds, which do not have an expanded height (0-255)

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.

2 participants