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

[CSM] add screenshot api lua #5674

Merged
merged 3 commits into from Apr 29, 2017
Merged

Conversation

Dumbeldor
Copy link
Contributor

@Dumbeldor Dumbeldor commented Apr 28, 2017

@@ -703,6 +703,8 @@ Call these functions only at load time!
* `minetest.get_protocol_version()`
* Returns the protocol version of the server.
* Might not be accurate at start up as the client might not be connected to the server yet, in that case it will return 0.
* `minetest.screenshot()`
Copy link
Member

Choose a reason for hiding this comment

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

take_screenshot

Copy link
Member

@nerzhul nerzhul left a comment

Choose a reason for hiding this comment

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

Okay for me

@@ -241,6 +241,13 @@ int ModApiClient::l_get_protocol_version(lua_State *L)
return 1;
}

int ModApiClient::l_take_screenshot(lua_State *L)
{
Client* client = getClient(L);
Copy link
Member

Choose a reason for hiding this comment

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

Client *client ...

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

LGTM

@sofar sofar added this to the 0.4.16 milestone Apr 29, 2017
@sofar sofar closed this Apr 29, 2017
@sofar sofar reopened this Apr 29, 2017
@sofar
Copy link
Contributor

sofar commented Apr 29, 2017

My bad, "back" button did something I didn't expect

@sofar
Copy link
Contributor

sofar commented Apr 29, 2017

Do we really want this? This will make it trivial to fill up someones drive with files.

@Ekdohibs
Copy link
Member

For that a few other changes (like maybe client API that will make predictions to the world state), I think what we really want is two Lua APIs: one that would be for locally-installed client mods, and one for client mods sent from server. Of course, they would share most of the API calls, but locally-installed mods do not need as much security and would be allowed API calls like this one, while mods sent from server would be much more limited with these API calls, but would probably however be allowed to make calls that predict changes to the world state or that modify the player physics. Since we only have the former for the moment, I don't think this should be an objection to this PR, as clients can have that code only if they install the mod themselves.

@nerzhul nerzhul merged commit 19960e2 into minetest:master Apr 29, 2017
Copy link
Member

@rubenwardy rubenwardy left a comment

Choose a reason for hiding this comment

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

This needed rate limiting

while 1 do
    minetest.take_screenshot()
end

@nerzhul
Copy link
Member

nerzhul commented Apr 29, 2017

ofc now we can do it, rate limiting seems lgtm to code

@nerzhul nerzhul added this to Done in Minetest 0.4.16 Apr 29, 2017
paramat referenced this pull request Jul 4, 2020
Reverted from commit 19960e2
(* [CSM] add screenshot api lua)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants