Skip to content

Conversation

@Auseawesome
Copy link

Fixes #2117

Currently a draft and heavily WIP, feedback welcome

@BoySanic BoySanic moved this to Low Priority in PRs to review Nov 12, 2025
This is done to mirror the old tree top but could easily be changed to be open if preferred as a stylistic choice.
@Auseawesome Auseawesome marked this pull request as ready for review November 13, 2025 08:48
@Auseawesome
Copy link
Author

This should be enough for a basic PR replacing old logs with the new logs. I've kept it simple to avoid falling into the trap of the old PR by trying to do too much at once. Feedback welcome.

Copy link
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

Alright, so I think the problem of this PR is obvious: Too many conflicts.

I'd suggest to split the code changes from the bulk asset changes. Instead maybe pick a single tree for testing and leave the rest alone, instead they can be made in a follow-up, which (without code changes) will be faster to merge.

As for the code itself, I'd suggest to take inspiration from its predecessor which had an implmenetation that would switch depending on the block type (this would allow addons to use different blocks for the logs, best example for this currently is the glass fores), and if you keep supporting the old log rotation, then you would keep compatibility with the old assets, allowing you to split them to a future PR.

@IntegratedQuantum IntegratedQuantum moved this from Low Priority to In review in PRs to review Nov 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

Replace the old logs in structures and work towards removing them.

2 participants