mt-mods / pipeworks

Pipeworks is a mod for Minetest allowing the crafting and usage of pipes and tubes
Other
14 stars 19 forks source link

Get rid of random print() calls #127

Closed Emojigit closed 3 months ago

Emojigit commented 3 months ago

This PR gets rid of print() calls by either removing them or changing them into appropriate minetest.log calls. This PR is ready for review.

OgelGames commented 3 months ago

I think the prints in the Lua tube should stay, makes more sense than logging IMO.

The others are fine to be removed. (one of them was me forgetting to remove it :P)

Emojigit commented 3 months ago

I think the prints in the Lua tube should stay, makes more sense than logging IMO.

Done in ba00a49

S-S-X commented 3 months ago

I think the prints in the Lua tube should stay, makes more sense than logging IMO.

Shouldn't really, while print is easy on better platforms Linux it is way harder to utilize on Windows. And I'd guess there's now days a lot more Windows players than before.

What it ultimately should do is exactly same what mesecons does (besides mod name):

local function safe_print(param)
    if mesecon.setting("luacontroller_print_behavior", "log") == "log" then
        local string_meta = getmetatable("")
        local sandbox = string_meta.__index
        string_meta.__index = string -- Leave string sandbox temporarily
        minetest.log("action", string.format("[mesecons_luacontroller] print(%s)", dump(param)))
        string_meta.__index = sandbox -- Restore string sandbox
    end
end
Emojigit commented 3 months ago

What it ultimately should do is exactly same what mesecons does (besides mod name):

Done in 739ddc6 (BTW, perhaps it would be better if you can utilize GitHub's review function to give suggestions)

S-S-X commented 3 months ago

perhaps it would be better if you can utilize GitHub's review function to give suggestions

I would probably have used suggestion as I usually do but you can't add suggestions without changes and previous commit did revert changes so there were not enough changed lines for suggestion feature.

OgelGames commented 3 months ago

Oh, this wasn't merged yet.