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

Add lua errors to error dialog #1748

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@rubenwardy
Copy link
Member

commented Oct 19, 2014

lua_errors

@ShadowNinja

This comment has been minimized.

Copy link
Member

commented Oct 19, 2014

Spliting the error message at "Message:" is ugly, and I think that error should be an optional argument to loadScript/loadMod.
The concept is good though, I always have to check the console to debug a crash.

@rubenwardy

This comment has been minimized.

Copy link
Member Author

commented Oct 20, 2014

You can't make reference arguments optional. You would have to overload the function.

The way "message:" is done is pretty ugly, I agree.

  1. How about all of the message is added to the textlist? That way there is no need for the "message:". However, it won't look as nice - the mod error line won't stand out

  2. Or the first line could be the header and all the other lines in the text list.

@ShadowNinja

This comment has been minimized.

Copy link
Member

commented Oct 20, 2014

The first line could really be removed, it doesn't say much more than "Lua error".
You can make it a default parameter like so:

bool loadScript(const std::string &path, std::string *error = NULL);

And then add if (error) to the function.

@Zeno- Zeno- force-pushed the minetest:master branch to a1db83e Nov 25, 2014

@kwolekr kwolekr force-pushed the minetest:master branch to 3f83ca2 Dec 25, 2014

@nerzhul

This comment has been minimized.

Copy link
Member

commented Mar 2, 2015

@rubenwardy can you rebase and fix this commit ?

@rubenwardy rubenwardy force-pushed the rubenwardy:lua_errors branch Mar 2, 2015

@rubenwardy

This comment has been minimized.

Copy link
Member Author

commented Mar 2, 2015

Rebased and fixed.

lua_errors

@rubenwardy rubenwardy changed the title Add lua errors to error dialog (WIP) Add lua errors to error dialog Mar 2, 2015

@rubenwardy

View changes

server.conf Outdated
remote_port = 30000
address = 127.0.0.1
ctf_allocate_mode = 1
creative_mode = true

This comment has been minimized.

Copy link
@rubenwardy

rubenwardy Mar 2, 2015

Author Member

Oh dear. Will remove this file tomorrow morning and squash.

end
formspec = formspec .. core.formspec_escape(string.sub(text, 1, 79))
text = string.sub(text, 80, #text)
end

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

This is a horrible hack to implement word wrapping, and should -- at minimum -- be clearly marked as such.

@ShadowNinja

View changes

builtin/fstk/ui.lua Outdated
if string.find(gamedata.errormessage, "ModError") then
error_title = fgettext("An error occured in a Lua Script, such as a mod")
else
error_title = fgettext("An error occured")

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

These two are missing trailing punctuation (:).

@ShadowNinja

View changes

builtin/fstk/ui.lua Outdated
end
formspec = formspec .. core.formspec_escape(text)
end
local error_title = ""

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

This allocates an empty string but doesn't use it. Just do local error_title.

@ShadowNinja

View changes

builtin/fstk/ui.lua Outdated
formspec = formspec .. core.formspec_escape(text)
end
local error_title = ""
if string.find(gamedata.errormessage, "ModError") then

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

I don't like string searches to handle logic like this, but at least use a raw match instead of a regex match.

This comment has been minimized.

Copy link
@rubenwardy

rubenwardy Mar 3, 2015

Author Member

string.match also uses regex. So does gmatch. What do you mean?

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

gamedata.errormessage:find("ModError", 1, true)

@ShadowNinja

View changes

src/script/cpp_api/s_base.cpp Outdated
(*error) += scriptpath;
(*error) += ": modname does not follow naming conventions: ";
(*error) += "Only chararacters [a-z0-9_] are allowed.";
}

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

This is essentially duplicate code, find a way to only create the error message once.

It's also subtly different from what's logged (missing quotes).

@ShadowNinja

View changes

src/script/cpp_api/s_base.cpp Outdated
errorstream << lua_tostring(L, -1) << std::endl;
std::string errormsg = lua_tostring(L, -1);
if (error)
(*error) = errormsg;

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

Sticking this in the middle of a group of logging statements is ugly. While you're at it you can make it only refference the stream once (as in:

foostream << "abc" <<std::endl
    << "def" << std::endl;

).

@ShadowNinja

View changes

src/server.cpp Outdated
@@ -304,30 +300,32 @@ Server::Server(
m_script = new GameScripting(this);

std::string scriptpath = getBuiltinLuaPath() + DIR_DELIM "init.lua";
std::string errormsg = "";

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

= "" is unnecessary.

@ShadowNinja

View changes

src/server.cpp Outdated
if (!m_script->loadScript(scriptpath))
throw ModError("Failed to load and run " + scriptpath);
if (!m_script->loadScript(scriptpath, &errormsg))
throw ModError("Failed to load and run " + scriptpath + "\nError from Lua:\n" + errormsg);

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

Long line.

@ShadowNinja

View changes

src/server.cpp Outdated
const ModSpec &mod = *i;
std::string scriptpath = mod.path + DIR_DELIM + "init.lua";
infostream<<" ["<<padStringRight(mod.name, 12)<<"] [\""
<<scriptpath<<"\"]"<<std::endl;
bool success = m_script->loadMod(scriptpath, mod.name);
errormsg = "";

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

Unnecessary.

@ShadowNinja

View changes

src/server.cpp Outdated
@@ -3439,5 +3437,3 @@ void dedicated_server_loop(Server &server, bool &kill)
}
}
}

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Mar 3, 2015

Member

There should be one and only one trailing line.

@rubenwardy rubenwardy force-pushed the rubenwardy:lua_errors branch Mar 3, 2015

@rubenwardy

This comment has been minimized.

Copy link
Member Author

commented Mar 3, 2015

Fixed some issues, will fix the rest when I have access to a compiler.

@technomancy

This comment has been minimized.

Copy link

commented Jun 22, 2015

Is there anything I can help with to get this merged? I would love to see this happen.

@rubenwardy

This comment has been minimized.

Copy link
Member Author

commented Jun 22, 2015

@rubenwardy rubenwardy force-pushed the rubenwardy:lua_errors branch 3 times, most recently Jun 24, 2015

@rubenwardy

This comment has been minimized.

Copy link
Member Author

commented Jun 24, 2015

Fixed, rebased and ready for review.

@rubenwardy rubenwardy force-pushed the rubenwardy:lua_errors branch Jun 24, 2015

@est31

View changes

builtin/fstk/ui.lua Outdated
end
local error_title
if string.find(gamedata.errormessage, "ModError") then
error_title = fgettext("An error occured in a Lua Script, such as a mod:")

This comment has been minimized.

Copy link
@est31

est31 Jun 24, 2015

Contributor

Why capitalize "Script"?

This comment has been minimized.

Copy link
@rubenwardy

rubenwardy Jun 24, 2015

Author Member

No idea. Fixed.

@rubenwardy rubenwardy force-pushed the rubenwardy:lua_errors branch to faf4358 Jun 24, 2015

@est31

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2015

👍 looks good.

@est31

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2015

@est31 est31 closed this Jun 29, 2015

@rubenwardy rubenwardy deleted the rubenwardy:lua_errors branch Dec 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.