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.

Sign up for GitHub

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

Jump to bottom

Change light banks meaning #11060

Closed
wants to merge 3 commits into from
Closed

Change light banks meaning #11060

wants to merge 3 commits into from

Conversation

numberZero
Copy link
Contributor

@numberZero numberZero commented Mar 13, 2021 •
edited

Fix #5155. Seems to work.
All the meshgen uses old-style light values (as they can always be reconstructed).

WARNING: converts the map to a new version. You will not be able to load the map in other Minetest versions. Making backup before testing is strongly recommended. (the conversion should always be reversible but MT can’t handle it. I wrote a tool for that but it is barely tested and supports sqlite only).

To do

This PR is Ready for Review. Let me know if I forgot anything.

How to test

Play with various sunlight&artificial light combinations. Try using client from this PR with server from master or vice versa. If it looks right it is probably right but light testing tool (devtest) may also be helpful (pay attention to param1, first digit is artificial light, second digit is sunlight).

@sfan5 sfan5 added @ Client / Audiovisuals Feature ✨ PRs that add or enhance a feature Non-trivial A large amount of work is required to address this (sometimes to the point of being infeasible). labels Mar 13, 2021
@rubenwardy rubenwardy added the Roadmap The change matches an item on the current roadmap. label Jun 16, 2021
@Zughy Zughy added the Rebase needed The PR needs to be rebased by its author. label May 28, 2022
@Zughy
Copy link
Member

Zughy commented May 28, 2022

@numberZero please rebase

@numberZero numberZero force-pushed the sunlight branch 2 times, most recently from 5dd2cd3 to 428be68 Compare June 5, 2022 17:50
@Zughy Zughy removed the Rebase needed The PR needs to be rebased by its author. label Jun 5, 2022
@TurkeyMcMac TurkeyMcMac added the Rebase needed The PR needs to be rebased by its author. label Oct 12, 2022
@TurkeyMcMac
Copy link
Contributor

Please rebase.

@TurkeyMcMac
Copy link
Contributor

I think LuaVoxelManip::l_set_lighting also needs to be changed.

@TurkeyMcMac
Copy link
Contributor

It seems the old light format is documented:

minetest/doc/lua_api.txt

Lines 1039 to 1041 in b3503e7

* `paramtype = "light"`
* The value stores light with and without sun in its lower and upper 4 bits
respectively.

How much of a problem would this compatibility break be?

@Zughy Zughy added the Adoption needed The pull request needs someone to adopt it. Adoption welcomed! label Nov 22, 2022
@Zughy Zughy closed this Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Adoption needed The pull request needs someone to adopt it. Adoption welcomed! @ Client / Audiovisuals Feature ✨ PRs that add or enhance a feature Non-trivial A large amount of work is required to address this (sometimes to the point of being infeasible). Rebase needed The PR needs to be rebased by its author. Roadmap The change matches an item on the current roadmap.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change light banks meaning to sunlight/artificial
5 participants
@numberZero @Zughy @TurkeyMcMac @sfan5 @rubenwardy