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

Implement VoxelArea #84

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

TurkeyMcMac
Copy link
Contributor

Closes #34.

This is an independent VoxelArea implementation. There are tests.

I realized that that part of the code was similar to Minetest's
implementation, and not coincidentally so. I don't want to get into
trouble.
@S-S-X
Copy link
Owner

S-S-X commented May 29, 2022

Implementation has to be moved to root directory.

Reason why this cannot be in game directory is because it is not coming from core engine lua scripts.
game directory for different engine versions gets populated from git minetest engine git repo, there should not be anything but unmodified minetest core engine lua scripts.

@TurkeyMcMac
Copy link
Contributor Author

Perhaps you could just copy the implementation from Minetest and put it in the game/ directory.

@S-S-X
Copy link
Owner

S-S-X commented Jun 1, 2022

Perhaps you could just copy the implementation from Minetest and put it in the game/ directory.

Actually I think It'll be available if you run Mineunit with full Minetest core libraries using for example

mineunit --fetch-core 5.5.0 --engine-version 5.5.0

Everything around engine core libs is just badly messed up... that's mostly because it was not planned in any way and added on top of messed up directory structure.

See this crap for example:

mineunit/init.lua

Lines 52 to 71 in db70bcd

local function require_mineunit(name, root, tag)
local modulename = name:gsub("/", ".")
if root and tag and tag ~= "mineunit" then
local path = name:match("^([^/]+)/")
if path and tagged_paths[path] then
local oldpath = package.path
local module
package.path = root.."/"..tag.."/?.lua;"
mineunit:debug("Loading "..name.." from "..tag)
local success = pcall(function() module = require(modulename)end)
package.path = oldpath
if success then
return module
else
mineunit:error("Loading "..name.." from "..tag.." failed, trying to use builtin")
end
end
end
return require("mineunit." .. modulename)
end

Engine core library management requires some serious refactoring and I've planned to do that at some point in future but probably before that I have to refactor another completely messed up thing which is current Mineunit configuration system.

@S-S-X
Copy link
Owner

S-S-X commented Nov 18, 2022

Tested VoxelArea using --engine-version 5.4.1 mt-mods/technic@385ce5b
Seems to work fine.

However that's not really fixing whole issue as core library version management is still terrible.
I think it would be best to include some "default" VoxelArea with Mineunit, decision is mostly between including VoxelArea library direct from engine or going with this PR.

@TurkeyMcMac
Copy link
Contributor Author

I suppose it depends on the license of Minetest's implementation.

@S-S-X
Copy link
Owner

S-S-X commented Nov 18, 2022

LGPL 2.1 which is fine and already included here too https://github.com/S-S-X/mineunit/blob/master/LICENSE because there's few libraries included as is without changes directly from Minetest.
I did go through licensing when I was about to add core libs, determined that licenses are compatible and libraries can be included.

@S-S-X S-S-X added the in-latest-docker Included with latest docker images, might not yet be available elsewhere label Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-latest-docker Included with latest docker images, might not yet be available elsewhere
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add VoxelArea support
2 participants