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

Get rid of random print() calls #127

Merged

Conversation

Emojigit
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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

Done in ba00a49

@S-S-X
Copy link
Member

S-S-X commented Jun 17, 2024

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
Copy link
Contributor Author

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)

tubes/lua.lua Outdated Show resolved Hide resolved
@S-S-X
Copy link
Member

S-S-X commented Jun 17, 2024

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.

@BuckarooBanzay BuckarooBanzay added enhancement New feature or request cleanup code and other cleanups labels Jun 18, 2024
@Emojigit Emojigit requested a review from S-S-X June 18, 2024 06:19
tubes/lua.lua Outdated Show resolved Hide resolved
tubes/lua.lua Outdated Show resolved Hide resolved
Emojigit and others added 2 commits June 19, 2024 06:42
@Emojigit Emojigit requested review from TheEt1234 and S-S-X June 18, 2024 22:45
Copy link

@TheEt1234 TheEt1234 left a comment

Choose a reason for hiding this comment

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

lgtm

default_settings.lua Outdated Show resolved Hide resolved
tubes/lua.lua Outdated Show resolved Hide resolved
Co-authored-by: SX <50966843+S-S-X@users.noreply.github.com>
@Emojigit Emojigit requested a review from S-S-X June 22, 2024 06:50
Copy link
Member

@S-S-X S-S-X left a comment

Choose a reason for hiding this comment

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

Looks good to me, did not test it but seems it should work just fine.

In game lua tube print function now also works same as mesecons.

default_settings.lua Outdated Show resolved Hide resolved
Co-authored-by: SX <50966843+S-S-X@users.noreply.github.com>
@OgelGames
Copy link
Contributor

Oh, this wasn't merged yet.

@OgelGames OgelGames merged commit 5919f43 into mt-mods:master Jul 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup code and other cleanups enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants