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

Consolidate horizontal rules getters #602

Merged
merged 1 commit into from Jun 5, 2022

Conversation

TurkeyMcMac
Copy link
Contributor

There are several different components that can be rotated around the Y axis. This PR consolidates all their rules getter functions using a new API function. This has the added benefit of making them all use lookup tables instead of repeated rotation, which is probably faster and creates less garbage. I think I have noticed a minor performance increase.

I have tested that the affected nodes still work, but it would be good if someone could verify this.

@sfan5
Copy link
Member

sfan5 commented Apr 22, 2022

I have tested that the affected nodes still work, but it would be good if someone could verify this.

I didn't get to doing this myself but what about exhaustively dumping all matching rules and then comparing it before and after this PR.

@TurkeyMcMac
Copy link
Contributor Author

That sounds good. The rules may not be exactly the same for the straight insulated wires, but it shouldn't matter since they're symmetrical.

@TurkeyMcMac
Copy link
Contributor Author

You can test with this patch. Most of the tests will print "OK" to the error log. mesecons_insulated will dump some rules to the error log that do not match each other. You can visually verify that these rules are actually equivalent.

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

Verified to work

@sfan5 sfan5 merged commit 0a4a88b into minetest-mods:master Jun 5, 2022
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.

None yet

2 participants