Skip to content

Change fallen log and simple trees to use new logs#1575

Closed
OneAvargeCoder193 wants to merge 3 commits into
PixelGuys:masterfrom
OneAvargeCoder193:log_fix
Closed

Change fallen log and simple trees to use new logs#1575
OneAvargeCoder193 wants to merge 3 commits into
PixelGuys:masterfrom
OneAvargeCoder193:log_fix

Conversation

@OneAvargeCoder193

Copy link
Copy Markdown
Contributor

No description provided.

.id = "cubyz:simple_tree",
.leaves = "cubyz:air",
.log = "cubyz:cactus",
.top = "cubyz:cactus_flower",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This breaks the behavior. Now cacti will no longer spawn with a flower on top.
Also should use log/cactus.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you should make a new structure for this, or add an optional argument to simple structure, abusing the tree was kind of hacky to begin with.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

They should use SBBs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I didn't see a fix for this.

Comment thread assets/cubyz/biomes/spawn.zig.zon
Comment thread mods/rotation.zig Outdated
Comment thread assets/cubyz/biomes/forest/thin_birch.zig.zon
Comment thread src/server/terrain/simple_structures/SimpleTreeModel.zig Outdated
Comment thread src/server/terrain/simple_structures/SimpleTreeModel.zig Outdated
return self;
}

fn getWoodBlock(self: *SimpleTreeModel, data: u16) main.blocks.Block {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't pass the branch data, instead it should just pass the Neighbor and calculate the data in here.
This would make the callsite cleaner and could allow expanding this with other rotation modes (e.g. if we ever need to support the old rotation again)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can't because there are cases where multiple sides need to be connected at once

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then pass multiple sides

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is what data is doing. Data right now is just the bitmask of which sides are enabled or not

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then name it accordingly, and not just data. Also should be at least a u6 if not a packed struct.

@IntegratedQuantum

Copy link
Copy Markdown
Member

please address the remaining change requests

@IntegratedQuantum

Copy link
Copy Markdown
Member

Since there is no sign of this progressing I have decided to move this into an issue, so that someone else may fix this. #2117

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