Skip to content

Soul lightning #1839

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

Closed
Closed

Conversation

IncredibleHolg
Copy link
Contributor

Add soul_fire, soul_lanterns and soul_torch

included PRs: #1835 and #1832 , I close this.

Related Issue: #1807

IncredibleHolg and others added 4 commits August 9, 2020 21:50
crying_obsidian, lodestone, respawn_anchor
ancient_debris part of the nether roof
Comment on lines +399 to +400
block_crimson_sign = 12505,
block_warped_sign = 12506,
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason for using high IDs for these and the wall signs? In general we should avoid high IDs because we create an array up to the highest block index, which increases memory usage if this is a new highest ID block.

Comment on lines 2026 to 2034
if blockid == 51:
firetextures = self.load_fire()
side1 = self.transform_image_side(firetextures[0])
side2 = self.transform_image_side(firetextures[1]).transpose(Image.FLIP_LEFT_RIGHT)
elif blockid == 1040:
soul_firetextures = self.load_soul_fire()
side1 = self.transform_image_side(soul_firetextures[0])
side2 = self.transform_image_side(soul_firetextures[1]).transpose(Image.FLIP_LEFT_RIGHT)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need to duplicate the transform code here.

firetextures = self.load_fire() if blockid == 51 else self.load_soul_fire()
side1 = self.transform_image_side(firetextures[0])
side2 = self.transform_image_side(firetextures[1]).transpose(Image.FLIP_LEFT_RIGHT)

should be enough to handle both cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @CounterPillow

Unless i completely missed something about the data parameter, this is always zero for both fire variants ( not 0 .. 15)
So this can be simplified to

# fire and soul_fire
@material(blockid=[51, 1040], transparent=True)
def fire(self, blockid, data):
    if blockid == 51:
        textureNS = self.load_image_texture("assets/minecraft/textures/block/fire_0.png")
        textureEW = self.load_image_texture("assets/minecraft/textures/block/fire_1.png")
    elif blockid == 1040:
        textureNS = self.load_image_texture("assets/minecraft/textures/block/soul_fire_0.png")
        textureEW = self.load_image_texture("assets/minecraft/textures/block/soul_fire_1.png")
    side1 = self.transform_image_side(textureNS)
    side2 = self.transform_image_side(textureEW).transpose(Image.FLIP_LEFT_RIGHT)
    img = Image.new("RGBA", (24,24), self.bgcolor)
    alpha_over(img, side1, (12,0), side1)
    alpha_over(img, side2, (0,0), side2)
    alpha_over(img, side1, (0,6), side1)
    alpha_over(img, side2, (12,6), side2)    
    return img

Or am I missing something?

Comment on lines +2085 to +2086
509: "assets/minecraft/textures/block/crimson_planks.png",
510: "assets/minecraft/textures/block/warped_planks.png",
Copy link
Member

Choose a reason for hiding this comment

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

why are these at 500 as opposed to 1000? not that it really matters, just wondering if there's something you were planning on doing with that

Choose a reason for hiding this comment

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

These are the block IDs that minecraft uses itself. Only in bedrock these days, but I thought it was best to just continue the existing pattern.

Copy link
Member

@CounterPillow CounterPillow left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this, I'd like to know more about some of the ID choices you've made though (see comments left on code)

@IncredibleHolg
Copy link
Contributor Author

Added the changes for soul fire texture loading
sha1 d23fdb1

@IncredibleHolg IncredibleHolg deleted the soul-lightning branch November 29, 2020 09:05
@IncredibleHolg IncredibleHolg restored the soul-lightning branch November 29, 2020 09:05
@IncredibleHolg IncredibleHolg deleted the soul-lightning branch May 24, 2022 06:46
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.

3 participants